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

Two fixes for mixing in Java interfaces that refine a member's type #9615

Merged
merged 3 commits into from
May 6, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/transform/Erasure.scala
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ abstract class Erasure extends InfoTransform
val rhs = member.tpe match {
case MethodType(Nil, FoldableConstantType(c)) => Literal(c)
case _ =>
val sel: Tree = Select(This(root), member)
val sel: Tree = gen.mkAttributedSelect(gen.mkAttributedThis(root), member)
val bridgingCall = bridge.paramss.foldLeft(sel)((fun, vparams) => Apply(fun, vparams map Ident))

maybeWrap(bridgingCall)
Expand Down
3 changes: 0 additions & 3 deletions src/compiler/scala/tools/nsc/transform/OverridingPairs.scala
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ abstract class OverridingPairs extends SymbolPairs {
&& !exclude(low) // this admits private, as one can't have a private member that matches a less-private member.
&& (lowMemberType matches (self memberType high))
) // TODO we don't call exclude(high), should we?

override def skipOwnerPair(lowClass: Symbol, highClass: Symbol): Boolean =
lowClass.isJavaDefined && highClass.isJavaDefined // javac is already checking this better than we could
Copy link
Member

Choose a reason for hiding this comment

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

In Dotty I ended up with a similar check but restricted to the case where both symbols are actual classes and not interfaces: https://github.com/lampepfl/dotty/blob/69c800b22c0ae6c5eb6fa6e7c2142c18a8278ff1/compiler/src/dotty/tools/dotc/typer/RefChecks.scala#L506 since one can't extend two Java classes without one extending the other, but I'm not sure if it's completely correct either.

}

private def bothJavaOwnedAndEitherIsField(low: Symbol, high: Symbol): Boolean = {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ abstract class RefChecks extends Transform {
def isOverrideAccessOK = member.isPublic || { // member is public, definitely same or relaxed access
(!other.isProtected || member.isProtected) && // if o is protected, so is m
((!isRootOrNone(ob) && ob.hasTransOwner(mb)) || // m relaxes o's access boundary
(other.isJavaDefined && other.isProtected)) // overriding a protected java member, see #3946 #12349
(other.isJavaDefined && (member.isJavaDefined || other.isProtected))) // overriding a protected java member, see #3946 #12349
}
if (!isOverrideAccessOK) {
overrideAccessError()
Expand Down
8 changes: 8 additions & 0 deletions test/files/neg/t12380.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Test.scala:1: error: incompatible type in overriding
def m(): String (defined in trait I)
with def m(): Object (defined in class C);
found : (): Object
required: (): String
object Test extends p.J.C with p.J.I {
^
1 error
14 changes: 14 additions & 0 deletions test/files/neg/t12380/J.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package p;

public class J {
public static class C {
public Object m() { return new Object(); }
}
public interface I {
public String m();
}

public static class Test extends C implements I {
@Override public String m() { return ""; }
}
}
5 changes: 5 additions & 0 deletions test/files/neg/t12380/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object Test extends p.J.C with p.J.I {
def main(args: Array[String]): Unit = {
println((this: p.J.I).m.trim)
}
}
7 changes: 7 additions & 0 deletions test/files/pos/t12349b/A.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package p;

public class A {
public static class R { }

/* package-protected */ R foo() { return null; }
}
7 changes: 7 additions & 0 deletions test/files/pos/t12349b/B.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package q;

public class B extends p.A {
public static class RR extends p.A.R { }

/* package-protected */ RR foo() { return null; }
}
1 change: 1 addition & 0 deletions test/files/pos/t12349b/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
class Test extends q.B
28 changes: 28 additions & 0 deletions test/files/run/t12380/A.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// filter: unchecked

package p;

public class A {
public static interface I {
public I w();
}

public static interface J<R extends J<R>> extends I {
@Override public R w();
}

public static interface K extends I {
@Override public K w();

public default String mK() { return "K"; }
}

/* package-private */ static class B<R extends J<R>> implements J<R> {
@Override public R w() { return (R) this; }
}

public static class C<R extends J<R>> extends B<R> implements J<R> { }

// OK in Java, also OK in Scala
public static class Test extends C<Test> implements K { }
}
7 changes: 7 additions & 0 deletions test/files/run/t12380/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class Test extends p.A.C[Test] with p.A.K
object Test {
def main(args: Array[String]): Unit = {
assert((new Test).w.mK == "K")
assert((new p.A.Test).w.mK == "K")
}
}