Skip to content

Allow overloads between fields and methods for Java code #9116

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

Merged
merged 1 commit into from
Jun 9, 2020
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
47 changes: 33 additions & 14 deletions compiler/src/dotty/tools/dotc/core/Denotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import Periods._
import Flags._
import DenotTransformers._
import Decorators._
import Signature.MatchDegree._
import printing.Texts._
import printing.Printer
import io.AbstractFile
Expand Down Expand Up @@ -612,12 +613,19 @@ object Denotations {
def accessibleFrom(pre: Type, superAccess: Boolean)(implicit ctx: Context): Denotation =
if (!symbol.exists || symbol.isAccessibleFrom(pre, superAccess)) this else NoDenotation

def atSignature(sig: Signature, site: Type, relaxed: Boolean)(implicit ctx: Context): SingleDenotation = {
val situated = if (site == NoPrefix) this else asSeenFrom(site)
val matches = sig.matchDegree(situated.signature).ordinal >=
(if (relaxed) Signature.ParamMatch else Signature.FullMatch).ordinal
if (matches) this else NoDenotation
}
def atSignature(sig: Signature, site: Type, relaxed: Boolean)(implicit ctx: Context): SingleDenotation =
val situated = if site == NoPrefix then this else asSeenFrom(site)
val matches = sig.matchDegree(situated.signature) match
case FullMatch =>
true
case MethodNotAMethodMatch =>
// See comment in `matches`
relaxed && !symbol.is(JavaDefined)
case ParamMatch =>
relaxed
case noMatch =>
false
if matches then this else NoDenotation

def matchesImportBound(bound: Type)(implicit ctx: Context): Boolean =
if bound.isRef(defn.NothingClass) then false
Expand Down Expand Up @@ -961,15 +969,26 @@ object Denotations {
final def first: SingleDenotation = this
final def last: SingleDenotation = this

final def matches(other: SingleDenotation)(implicit ctx: Context): Boolean = {
final def matches(other: SingleDenotation)(implicit ctx: Context): Boolean =
val d = signature.matchDegree(other.signature)
(// fast path: signatures are the same and neither denotation is a PolyType
// For polytypes, signatures alone do not tell us enough to be sure about matching.
d == Signature.FullMatch &&
!infoOrCompleter.isInstanceOf[PolyType] && !other.infoOrCompleter.isInstanceOf[PolyType]
||
d != Signature.NoMatch && info.matches(other.info))
}

/** Slower check used if the signatures alone do not tell us enough to be sure about matching */
def slowCheck = info.matches(other.info)

d match
case FullMatch =>
if infoOrCompleter.isInstanceOf[PolyType] || other.infoOrCompleter.isInstanceOf[PolyType] then
slowCheck
else
true
case MethodNotAMethodMatch =>
// Java allows defining both a field and a zero-parameter method with the same name
!ctx.erasedTypes && !(symbol.is(JavaDefined) && other.symbol.is(JavaDefined))
case ParamMatch =>
!ctx.erasedTypes && slowCheck
case noMatch =>
false
end matches

def mapInherited(ownDenots: PreDenotation, prevDenots: PreDenotation, pre: Type)(implicit ctx: Context): SingleDenotation =
if (hasUniqueSym && prevDenots.containsSym(symbol)) NoDenotation
Expand Down
41 changes: 25 additions & 16 deletions compiler/src/dotty/tools/dotc/core/Signature.scala
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,7 @@ case class Signature(paramsSig: List[ParamSig], resSig: TypeName) {
@tailrec def loop(names1: List[ParamSig], names2: List[ParamSig]): Boolean =
if (names1.isEmpty) names2.isEmpty
else !names2.isEmpty && consistent(names1.head, names2.head) && loop(names1.tail, names2.tail)
if (ctx.erasedTypes && (this == NotAMethod) != (that == NotAMethod))
false // After erasure, we allow fields and parameterless methods with the same name.
// This is needed to allow both a module field and a bridge method for an abstract val.
// Test case is patmatch-classtag.scala
else
loop(this.paramsSig, that.paramsSig)
loop(this.paramsSig, that.paramsSig)
}

/** `that` signature, but keeping all corresponding parts of `this` signature. */
Expand All @@ -87,23 +82,27 @@ case class Signature(paramsSig: List[ParamSig], resSig: TypeName) {
/** The degree to which this signature matches `that`.
* If parameter signatures are consistent and result types names match (i.e. they are the same
* or one is a wildcard), the result is `FullMatch`.
* If only the parameter signatures are consistent, the result is `ParamMatch` before erasure and
* `NoMatch` otherwise.
* If only the parameter signatures are consistent, the result is either
* `MethodNotAMethodMatch` (if one side is a method signature and the other isn't),
* or `ParamMatch`.
* If the parameters are inconsistent, the result is always `NoMatch`.
*/
final def matchDegree(that: Signature)(implicit ctx: Context): MatchDegree =
if (consistentParams(that))
if (resSig == that.resSig || isWildcard(resSig) || isWildcard(that.resSig)) FullMatch
else if (!ctx.erasedTypes) ParamMatch
else NoMatch
else NoMatch
if consistentParams(that) then
if resSig == that.resSig || isWildcard(resSig) || isWildcard(that.resSig) then
FullMatch
else if (this == NotAMethod) != (that == NotAMethod) then
MethodNotAMethodMatch
else
ParamMatch
else
NoMatch

/** Does this signature potentially clash with `that` ? */
def clashes(that: Signature)(implicit ctx: Context): Boolean =
matchDegree(that) == FullMatch

/** name.toString == "" or name.toString == "_" */
private def isWildcard(name: TypeName) = name.isEmpty || name == tpnme.WILDCARD
private def isWildcard(name: TypeName) = name == tpnme.WILDCARD
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename to isWildcardOrEmpty

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually that commit removed the name.isEmpty check, so the current name of that method should be accurate.


/** Construct a signature by prepending the signature names of the given `params`
* to the parameter part of this signature.
Expand Down Expand Up @@ -136,7 +135,17 @@ object Signature {
// small values, so the performance hit should be minimal.

enum MatchDegree {
case NoMatch, ParamMatch, FullMatch
/** The signatures are unrelated. */
case NoMatch
/** The parameter signatures are equivalent. */
case ParamMatch
/** Both signatures have no parameters, one is a method and the other isn't.
*
* @see NotAMethod
*/
case MethodNotAMethodMatch
/** The parameter and result type signatures are equivalent. */
case FullMatch
}
export MatchDegree._

Expand Down
10 changes: 7 additions & 3 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2015,19 +2015,23 @@ object messages {
def nameAnd = if (decl.name != previousDecl.name) " name and" else ""
def details(implicit ctx: Context): String =
if (decl.isRealMethod && previousDecl.isRealMethod) {
import Signature.MatchDegree._

// compare the signatures when both symbols represent methods
decl.signature.matchDegree(previousDecl.signature) match {
case Signature.MatchDegree.NoMatch =>
case NoMatch =>
// If the signatures don't match at all at the current phase, then
// they might match after erasure.
val elimErasedCtx = ctx.withPhaseNoEarlier(ctx.elimErasedValueTypePhase.next)
if (elimErasedCtx != ctx)
details(elimErasedCtx)
else
"" // shouldn't be reachable
case Signature.MatchDegree.ParamMatch =>
case ParamMatch =>
"have matching parameter types."
case Signature.MatchDegree.FullMatch =>
case MethodNotAMethodMatch =>
"neither has parameters."
case FullMatch =>
i"have the same$nameAnd type after erasure."
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import dotty.tools.dotc.ast.{TreeTypeMap, Trees, tpd, untpd}
import dotty.tools.dotc.typer.{Implicits, Typer}
import dotty.tools.dotc.core._
import dotty.tools.dotc.core.Flags._
import dotty.tools.dotc.core.StdNames.nme
import dotty.tools.dotc.core.StdNames._
import dotty.tools.dotc.core.quoted.PickledQuotes
import dotty.tools.dotc.core.Symbols._
import dotty.tools.dotc.core.Decorators._
Expand Down Expand Up @@ -2100,7 +2100,7 @@ class ReflectionCompilerInterface(val rootContext: core.Contexts.Context) extend
val tpe = tpt.tpe.dropDependentRefinement
// we checked that this is a plain Function closure, so there will be an apply method with a MethodType
// and the expected signature based on param types
val expectedSig = Signature.NotAMethod.prependTermParams(paramTypes, false)
val expectedSig = Signature(Nil, tpnme.WILDCARD).prependTermParams(paramTypes, false)
val method = tpt.tpe.member(nme.apply).atSignature(expectedSig)
if method.symbol.is(Deferred) then
val methodType = method.info.asInstanceOf[MethodType]
Expand Down
2 changes: 1 addition & 1 deletion project/Build.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1056,7 +1056,7 @@ object Build {

++ (dir / "shared/src/test/scala/org/scalajs/testsuite/utils" ** "*.scala").get
++ (dir / "shared/src/test/scala/org/scalajs/testsuite/junit" ** "*.scala").get
++ (dir / "shared/src/test/scala/org/scalajs/testsuite/niobuffer" ** (("*.scala": FileFilter) -- "ByteBufferTest.scala")).get
++ (dir / "shared/src/test/scala/org/scalajs/testsuite/niobuffer" ** "*.scala").get
++ (dir / "shared/src/test/scala/org/scalajs/testsuite/niocharset" ** (("*.scala": FileFilter) -- "BaseCharsetTest.scala" -- "Latin1Test.scala" -- "USASCIITest.scala" -- "UTF16Test.scala" -- "UTF8Test.scala")).get
++ (dir / "shared/src/test/scala/org/scalajs/testsuite/scalalib" ** (("*.scala": FileFilter) -- "ArrayBuilderTest.scala" -- "ClassTagTest.scala" -- "EnumerationTest.scala" -- "SymbolTest.scala")).get
++ (dir / "shared/src/test/require-sam" ** "*.scala").get
Expand Down
4 changes: 4 additions & 0 deletions tests/pos-java-interop/java-overload-field-method/A.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
public class A {
public String foo = "";
public int foo() { return 1; }
}
7 changes: 7 additions & 0 deletions tests/pos-java-interop/java-overload-field-method/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
object Test {
val a = new A
val b = a.foo
val bs: String = b
val c = a.foo()
val ci: Int = c
}
3 changes: 3 additions & 0 deletions tests/pos/i7132.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
object Test {
java.nio.ByteBuffer.allocate(1).isReadOnly
}