Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(#17255): cannot find Scala companion module from Java #19773

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions compiler/src/dotty/tools/dotc/core/ContextOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ object ContextOps:
else pre.findMember(name, pre, required, excluded)
}
else // we are in the outermost context belonging to a class; self is invisible here. See inClassContext.
ctx.owner.findMember(name, ctx.owner.thisType, required, excluded)
if ctx.isJava then
javaFindMember(name, ctx.owner.thisType, lookInCompanion = true,required, excluded)
else
ctx.owner.findMember(name, ctx.owner.thisType, required, excluded)
else
ctx.scope.denotsNamed(name).filterWithFlags(required, excluded).toDenot(NoPrefix)
}
Expand All @@ -55,11 +58,20 @@ object ContextOps:
final def javaFindMember(name: Name, pre: Type, lookInCompanion: Boolean, required: FlagSet = EmptyFlags, excluded: FlagSet = EmptyFlags): Denotation =
assert(ctx.isJava)
inContext(ctx) {

import dotty.tools.dotc.core.NameOps.*
val preSym = pre.typeSymbol

// 1. Try to search in current type and parents.
val directSearch = pre.findMember(name, pre, required, excluded)
val directSearch =
def asModule =
if name.isTypeName && name.endsWith(StdNames.str.MODULE_SUFFIX) then
pre.findMember(name.stripModuleClassSuffix.moduleClassName, pre, required, excluded) match
case NoDenotation => NoDenotation
case symDenot: SymDenotation =>
symDenot.companionModule.denot
else NoDenotation
pre.findMember(name, pre, required, excluded) match
case NoDenotation => asModule
case denot => denot

// 2. Try to search in companion class if current is an object.
def searchCompanionClass = if lookInCompanion && preSym.is(Flags.Module) then
Expand Down
1 change: 1 addition & 0 deletions compiler/test/dotc/pos-test-pickling.blacklist
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ i16649-irrefutable.scala
strict-pattern-bindings-3.0-migration.scala
i17186b.scala
i11982a.scala
i17255

# Tree is huge and blows stack for printing Text
i7034.scala
Expand Down
2 changes: 2 additions & 0 deletions compiler/test/dotc/run-test-pickling.blacklist
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,5 @@ t6138
t6138-2
i12656.scala
trait-static-forwarder
i17255

6 changes: 6 additions & 0 deletions tests/pos/i17255/Bar.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package example;


public class Bar {
private static final example.Foo$ MOD = example.Foo$.MODULE$;
}
7 changes: 7 additions & 0 deletions tests/pos/i17255/Baz.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package example;

import example.Foo$;

public class Baz {
private static final Foo$ MOD = Foo$.MODULE$;
}
5 changes: 5 additions & 0 deletions tests/pos/i17255/Foo.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package example

case class Foo(i: Int)

object Foo
13 changes: 13 additions & 0 deletions tests/run/i17255/J.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package p;

public class J {
public static J j = new J();
public static p.J f() {
return p.J.j;
}
public static p.Module$ module() {

This comment was marked as outdated.

Copy link
Member

@bishabosha bishabosha Feb 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the tree reaches TASTy, this looks like

    87:       DEFDEF(12) 16 [module]
    90:         EMPTYCLAUSE
    91:         SELECTtpt 17 [Module$]
    93:           SHAREDtype 3
    95:         ELIDED
    96:           TYPEREF 19 [Module[ModuleClass]]
    98:             SHAREDtype 3
   100:         STATIC

really, tree 91 should be SELECTtpt 19 [Module[ModuleClass]].

It would be very nice to perhaps also replace typeName("Module$") with typeName("Module").moduleClassName in typedSelect in Typer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can leave this to follow-up, as it is still better than status quo. However this does rely on the compiler (aka all TASTy clients) intrinsically knowing how to resolve Module$ when the definition is actually Module[ModuleClass]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new issue #19806

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for reviewing. I will investigate #19806 too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with TASTy, so it may take a while...

Copy link
Member

@bishabosha bishabosha Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way to solve this is in typedSelect replacing the name in the Select tree, in the same way you did in javaFindMember

Copy link
Contributor Author

@i10416 i10416 Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can I test TASTy?(or make sure SELECTtpt 17 [Module$] is replaced with
SELECTtpt 19 [Module[ModuleClass]] because testp/pos does not seem to detect the difference...)

Copy link
Member

@bishabosha bishabosha Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can do it manually with the -Yprint-tasty flag which will print to the console any generated TASTy after pickler phase, make sure you use the required -Yjava-tasty also so that you generate tasty for java sources

i10416 marked this conversation as resolved.
Show resolved Hide resolved
return p.Module$.MODULE$;
}

public String toString() { return "J"; }
}
12 changes: 12 additions & 0 deletions tests/run/i17255/Module.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// scalajs: --skip

package p {
object Module {
override def toString = "Module"
}
}

object Test extends App {
assert(p.J.f().toString == "J")
assert(p.J.module().toString == "Module")
i10416 marked this conversation as resolved.
Show resolved Hide resolved
}
Loading