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

Java interface methods always non-concrete in lookup in Java classes #10580

Draft
wants to merge 2 commits into
base: 2.13.x
Choose a base branch
from

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Oct 19, 2023

When selecting from a Java type, looking up the member takes interface members as non-concrete.

Fixes scala/bug#12892

@scala-jenkins scala-jenkins added this to the 2.13.13 milestone Oct 19, 2023
@som-snytt
Copy link
Contributor Author

At least my failure is "interesting"...

[error] Test scala.lang.traits.BytecodeTest.traitMethodForwardersForJavaDefaultMethods failed: java.lang.AssertionError: assertion failed: The compiler issued non-allowed warnings or errors:
[error] pos: RangePosition(unitTestSource.scala, 142, 142, 144) class K3 needs to be abstract.
[error] Missing implementation for member of trait J1:
[error]   def f(): Int = ???
[error]  ERROR, took 0.051 sec
[error]     at scala.tools.testkit.Compiler.checkReport(BytecodeTesting.scala:84)
[error]     at scala.tools.testkit.Compiler.compileToBytes(BytecodeTesting.scala:91)
[error]     at scala.tools.testkit.Compiler.compileClasses(BytecodeTesting.scala:96)
[error]     at scala.lang.traits.BytecodeTest.traitMethodForwardersForJavaDefaultMethods(BytecodeTest.scala:149)
[error]     ...

@lrytz
Copy link
Member

lrytz commented Oct 20, 2023

Your idea looks good! Makes sense to me, after looking at the JLS.

@lrytz
Copy link
Member

lrytz commented Oct 20, 2023

Test passes after narrowing the change down to selectorClass.isJavaDefined (vs currentBaseClass.isJavaDefined).

So

interface J1 { int f(); }
interface J2 { default int f() { return 1; } }

Then new J2 with J1 {} and new J1 with J2 {} in Scala both work, the Scala rules apply. The Java rules apply when selecting from Java-defined class.

@som-snytt
Copy link
Contributor Author

som-snytt commented Oct 20, 2023

Thanks, I got side-tracked reading Mixin. If testing selectorClass works, then I'll have to revisit the change. I must chauffeur this morning, so you will be enjoying your weekend by the time I get back to study it.

I also noticed there is a test for .isInterface.

@lrytz
Copy link
Member

lrytz commented Oct 20, 2023

you will be enjoying your weekend

Yes! 4️⃣0️⃣ 🎂

@som-snytt som-snytt changed the title Review/t12892 java override [ci: last-only] Java interface methods always non-concrete in lookup in Java classes Oct 21, 2023
@som-snytt som-snytt marked this pull request as ready for review October 21, 2023 18:48
@som-snytt som-snytt marked this pull request as draft October 22, 2023 00:20
@lrytz lrytz marked this pull request as ready for review October 23, 2023 08:50
@lrytz lrytz force-pushed the review/t12892-java-override branch from b28f79c to 12d6677 Compare October 23, 2023 08:50
@som-snytt
Copy link
Contributor Author

It should work also with abstract methods. I haven't given it more thought/time yet.

@lrytz
Copy link
Member

lrytz commented Oct 23, 2023

It should work also with abstract methods

🤔 what should work?

@som-snytt
Copy link
Contributor Author

Your words are ripe with wisdom, my friend!

@lrytz
Copy link
Member

lrytz commented Oct 23, 2023

👍 I see..

@lrytz lrytz force-pushed the review/t12892-java-override branch 2 times, most recently from 4169823 to d3b6164 Compare October 30, 2023 16:27
@lrytz
Copy link
Member

lrytz commented Oct 30, 2023

So there are two competing factors: refining the result type and being a SAM type.

Turns out that scalac permits too many types as SAM:

public interface A {
    int sam(int x);
    default A m() { return x -> x; }
}

public interface B extends A {
    B m();
}
Welcome to Scala 2.13.12 (OpenJDK 64-Bit Server VM, Java 17.0.6).
Type in expressions for evaluation. Or try :help.

scala> val b: B = x => x
val b: B = $Lambda$1037/0x00000008005f5428@698ac187

scala> b.m
java.lang.AbstractMethodError: Receiver class $Lambda$1037/0x00000008005f5428 does not define or inherit an implementation of the resolved method 'abstract B m()' of interface B.
  at B.m(B.java:1)

On the flipside, we need to keep treating the following as SAM:

interface B {
  int sam(int x);
  @Override boolean equals(Object other);
}

B is like java.util.Comparator. Probably we can fix that by adjusting samOf in Definitions.

I'll keep digging :)

@lrytz lrytz force-pushed the review/t12892-java-override branch from d3b6164 to bb218b8 Compare October 31, 2023 15:55
@lrytz lrytz marked this pull request as draft November 29, 2023 17:16
@lrytz lrytz self-assigned this Nov 29, 2023
@lrytz lrytz modified the milestones: 2.13.13, 2.13.14 Nov 29, 2023
@SethTisue SethTisue modified the milestones: 2.13.14, 2.13.15 Mar 13, 2024
@som-snytt som-snytt closed this Mar 20, 2024
@SethTisue SethTisue removed this from the 2.13.15 milestone Mar 21, 2024
@som-snytt
Copy link
Contributor Author

I forgot that this has lrytz assistance in its history, what we call either rytzassist or rytztory.

I'll redraft for 2.13.15 and try to understand it, then ping for a rytzassist.

@som-snytt som-snytt reopened this Apr 15, 2024
@scala-jenkins scala-jenkins added this to the 2.13.15 milestone Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants