-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Handle overloaded members when generating Java varargs bridges #13066
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
package dotty.tools.dotc | ||
package dotty.tools | ||
package dotc | ||
package transform | ||
|
||
import core._ | ||
|
@@ -76,7 +77,23 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => | |
|
||
override def infoMayChange(sym: Symbol)(using Context): Boolean = sym.is(Method) | ||
|
||
private def overridesJava(sym: Symbol)(using Context) = sym.allOverriddenSymbols.exists(_.is(JavaDefined)) | ||
/** Does `sym` override a symbol defined in a Java class? One might think that | ||
* this can be expressed as | ||
* | ||
* sym.allOverriddenSymbols.exists(_.is(JavaDefined)) | ||
* | ||
* but that does not work, since `allOverriddenSymbols` gets confused because the | ||
* signatures of a Java varargs method and a Scala varargs override are not the same. | ||
*/ | ||
private def overridesJava(sym: Symbol)(using Context) = | ||
sym.owner.info.baseClasses.drop(1).exists { bc => | ||
bc.is(JavaDefined) && { | ||
val other = bc.info.nonPrivateDecl(sym.name) | ||
other.hasAltWith { alt => | ||
sym.owner.thisType.memberInfo(alt.symbol).matchesLoosely(sym.info) | ||
} | ||
} | ||
} | ||
|
||
private def hasVarargsAnnotation(sym: Symbol)(using Context) = sym.hasAnnotation(defn.VarargsAnnot) | ||
|
||
|
@@ -85,7 +102,8 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => | |
private def isVarargsMethod(sym: Symbol)(using Context) = | ||
hasVarargsAnnotation(sym) || | ||
hasRepeatedParams(sym) && | ||
(sym.allOverriddenSymbols.exists(s => s.is(JavaDefined) || hasVarargsAnnotation(s))) | ||
overridesJava(sym) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The previous version skipped the checks on overriden symbols when there were no repeated params (hence the parens). Is it intentional to change that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, this is missing a pair of parens |
||
|| sym.allOverriddenSymbols.exists(hasVarargsAnnotation) | ||
|
||
/** Eliminate repeated parameters from method types. */ | ||
private def elimRepeated(tp: Type, isJava: Boolean)(using Context): Type = tp.stripTypeVar match | ||
|
@@ -292,6 +310,7 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => | |
report.error(s"$src produces a forwarder method that conflicts with ${conflict.showDcl}", original.srcPos) | ||
case Nil => | ||
forwarder.enteredAfter(thisPhase) | ||
end addVarArgsForwarder | ||
|
||
/** Convert type from Scala to Java varargs method */ | ||
private def toJavaVarArgs(tp: Type)(using Context): Type = tp match | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
class Impl extends Intf { | ||
override def thing(x: Int) = ??? | ||
override def thing(y: String*) = ??? | ||
override def thing2(y: String*) = ??? | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
interface Intf { | ||
public void thing(int x); | ||
public void thing(String... y); | ||
public void thing2(String... y); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is that
allOverriddenSymbols
delegates toDenotation#matchingDenotation
which is missing the logic present inDenotation#matchesLoosely
to handle matching denotations across languages (java/scala2/scala3), I think this needs to be fixed but this is a good stop-gap meanwhile.