From 463448fed3436d0cceb00ed15f1e90db38f602cb Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 9 Feb 2021 15:32:02 +0100 Subject: [PATCH 1/5] Add MethodOrPoly#signature(isJava) overload This makes it possible to compute the Java or Scala signature of a method, regardless of whether it was defined as a JavaMethod or not. Because there's nothing left that is shareable, this required removing SignatureCachingType and duplicating the signature caching logic between MethodOrPoly and NamedType. This commit itself does not change any semantics, but this new method will be useful for the next commit. --- .../src/dotty/tools/dotc/core/Types.scala | 136 +++++++++++------- .../tools/dotc/transform/TreeChecker.scala | 19 +-- 2 files changed, 89 insertions(+), 66 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 6b12386d2e83..0f72b6724795 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -1999,7 +1999,7 @@ object Types { // --- NamedTypes ------------------------------------------------------------------ - abstract class NamedType extends CachedProxyType with ValueType with SignatureCachingType { self => + abstract class NamedType extends CachedProxyType with ValueType { self => type ThisType >: this.type <: NamedType type ThisName <: Name @@ -2015,11 +2015,13 @@ object Types { private var lastSymbol: Symbol = null private var checkedPeriod: Period = Nowhere private var myStableHash: Byte = 0 + private var mySignature: Signature = _ + private var mySignatureRunId: Int = NoRunId // Invariants: - // (1) checkedPeriod != Nowhere => lastDenotation != null - // (2) lastDenotation != null => lastSymbol != null - // (3) mySigRunId != NoRunId => mySig != null + // (1) checkedPeriod != Nowhere => lastDenotation != null + // (2) lastDenotation != null => lastSymbol != null + // (3) mySignatureRunId != NoRunId => mySignature != null def isType: Boolean = isInstanceOf[TypeRef] def isTerm: Boolean = isInstanceOf[TermRef] @@ -2037,15 +2039,22 @@ object Types { case sym: Symbol => sym.originDenotation.name } - /** The signature computed from the last known denotation with `sigFromDenot`, - * or if there is none, the signature of the symbol. Signatures are always - * computed before erasure, since some symbols change their signature at erasure. - */ - protected[dotc] def computeSignature(using Context): Signature = - val lastd = lastDenotation - if lastd != null then sigFromDenot(lastd) - else if ctx.erasedTypes then atPhase(erasurePhase)(computeSignature) - else symbol.asSeenFrom(prefix).signature + final override def signature(using Context): Signature = + /** The signature computed from the last known denotation with `sigFromDenot`, + * or if there is none, the signature of the symbol. Signatures are always + * computed before erasure, since some symbols change their signature at erasure. + */ + def computeSignature(using Context): Signature = + val lastd = lastDenotation + if lastd != null then sigFromDenot(lastd) + else if ctx.erasedTypes then atPhase(erasurePhase)(computeSignature) + else symbol.asSeenFrom(prefix).signature + + if ctx.runId != mySignatureRunId then + mySignature = computeSignature + if !mySignature.isUnderDefined then mySignatureRunId = ctx.runId + mySignature + end signature /** The signature computed from the current denotation with `sigFromDenot` if it is * known without forcing. @@ -3219,39 +3228,11 @@ object Types { // is that most poly types are cyclic via poly params, // and therefore two different poly types would never be equal. - /** A trait that mixes in functionality for signature caching */ - trait SignatureCachingType extends TermType { - protected var mySignature: Signature = _ - protected var mySignatureRunId: Int = NoRunId - - protected[dotc] def computeSignature(using Context): Signature - - final override def signature(using Context): Signature = { - if (ctx.runId != mySignatureRunId) { - mySignature = computeSignature - if (!mySignature.isUnderDefined) mySignatureRunId = ctx.runId - } - mySignature - } - } - - trait MethodicType extends TermType { - protected def resultSignature(using Context): Signature = try resultType match { - case rtp: MethodicType => rtp.signature - case tp => - if (tp.isRef(defn.UnitClass)) Signature(Nil, defn.UnitClass.fullName.asTypeName) - else Signature(tp, isJava = false) - } - catch { - case ex: AssertionError => - println(i"failure while taking result signature of $this: $resultType") - throw ex - } - } + trait MethodicType extends TermType /** A by-name parameter type of the form `=> T`, or the type of a method with no parameter list. */ abstract case class ExprType(resType: Type) - extends CachedProxyType with TermType with MethodicType { + extends CachedProxyType with MethodicType { override def resultType(using Context): Type = resType override def underlying(using Context): Type = resType @@ -3371,7 +3352,66 @@ object Types { final override def equals(that: Any): Boolean = equals(that, null) } - abstract class MethodOrPoly extends UncachedGroundType with LambdaType with MethodicType with SignatureCachingType { + /** The superclass of MethodType and PolyType. */ + sealed abstract class MethodOrPoly extends UncachedGroundType with LambdaType with MethodicType { + + // Invariants: + // (1) mySignatureRunId != NoRunId => mySignature != null + // (2) myJavaSignatureRunId != NoRunId => myJavaSignature != null + + private var mySignature: Signature = _ + private var mySignatureRunId: Int = NoRunId + private var myJavaSignature: Signature = _ + private var myJavaSignatureRunId: Int = NoRunId + + /** If `isJava` is false, the Scala signature of this method. Otherwise, its Java signature. + * + * This distinction is needed because the same method type + * might be part of both a Java and Scala class and each language has + * different type erasure rules. + * + * Invariants: + * - Two distinct method overloads defined in the same _Scala_ class will + * have distinct _Scala_ signatures. + * - Two distinct methods overloads defined in the same _Java_ class will + * have distinct _Java_ signatures. + * + * @see SingleDenotation#signature + */ + def signature(isJava: Boolean)(using Context): Signature = + def computeSignature(isJava: Boolean)(using Context): Signature = + val resultSignature = resultType match + case tp: MethodOrPoly => tp.signature(isJava) + case tp: ExprType => tp.signature + case tp => + if tp.isRef(defn.UnitClass) then Signature(Nil, defn.UnitClass.fullName.asTypeName) + else Signature(tp, isJava = false) + this match + case tp: MethodType => + val params = if (isErasedMethod) Nil else tp.paramInfos + resultSignature.prependTermParams(params, isJava) + case tp: PolyType => + resultSignature.prependTypeParams(tp.paramNames.length) + + if isJava then + if ctx.runId != myJavaSignatureRunId then + myJavaSignature = computeSignature(isJava) + if !myJavaSignature.isUnderDefined then myJavaSignatureRunId = ctx.runId + myJavaSignature + else + if ctx.runId != mySignatureRunId then + mySignature = computeSignature(isJava) + if !mySignature.isUnderDefined then mySignatureRunId = ctx.runId + mySignature + end signature + + final override def signature(using Context): Signature = + def isJava(tp: Type): Boolean = tp match + case tp: PolyType => isJava(tp.resultType) + case tp: MethodType => tp.isJavaMethod + case _ => false + signature(isJava = isJava(this)) + final override def hashCode: Int = System.identityHashCode(this) final override def equals(that: Any): Boolean = equals(that, null) @@ -3531,11 +3571,6 @@ object Types { companion.eq(ContextualMethodType) || companion.eq(ErasedContextualMethodType) - protected[dotc] def computeSignature(using Context): Signature = { - val params = if (isErasedMethod) Nil else paramInfos - resultSignature.prependTermParams(params, isJavaMethod) - } - protected def prefixString: String = companion.prefixString } @@ -3766,9 +3801,6 @@ object Types { assert(resType.isInstanceOf[TermType], this) assert(paramNames.nonEmpty) - protected[dotc] def computeSignature(using Context): Signature = - resultSignature.prependTypeParams(paramNames.length) - override def isContextualMethod = resType.isContextualMethod override def isImplicitMethod = resType.isImplicitMethod diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 4ebe2cf41fad..10dddd12a90a 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -90,22 +90,13 @@ class TreeChecker extends Phase with SymTransformer { // Signatures are used to disambiguate overloads and need to stay stable // until erasure, see the comment above `Compiler#phases`. if (ctx.phaseId <= erasurePhase.id) { - val cur = symd.info - val initial = symd.initial.info - val curSig = cur match { - case cur: SignatureCachingType => - // Bypass the signature cache, it might hide a signature change - cur.computeSignature - case _ => - cur.signature - } - assert(curSig == initial.signature, + val initial = symd.initial + assert(symd.signature == initial.signature, i"""Signature of ${sym.showLocated} changed at phase ${ctx.base.fusedContaining(ctx.phase.prev)} - |Initial info: ${initial} + |Initial info: ${initial.info} |Initial sig : ${initial.signature} - |Current info: ${cur} - |Current sig : ${curSig} - |Current cached sig: ${cur.signature}""") + |Current info: ${symd.info} + |Current sig : ${symd.signature}""") } symd From 0705749ba974185c0da404780ceb46d9dfa29f65 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 9 Feb 2021 15:58:27 +0100 Subject: [PATCH 2/5] Don't rely on JavaMethodType to use Java signatures Before this commit, SingleDenotation#signature simply delegated to MethodOrPoly#signature which used Java signatures for JavaMethodType and non-Java signatures otherwise. As a first step towards getting rid of JavaMethodType, this commit changes this to make SingleDenotation#signature itself responsible for using Java signatures. --- .../dotty/tools/dotc/core/Denotations.scala | 18 +++++++++++++++--- compiler/src/dotty/tools/dotc/core/Types.scala | 10 +++++----- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index 1bf13c80fb85..c60f5bdbad4f 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -582,11 +582,23 @@ object Denotations { */ def prefix: Type = NoPrefix + /** Either the Scala or Java signature of the info, depending on where the + * symbol is defined. + * + * Invariants: + * - Before erasure, the signature of a denotation is always equal to the + * signature of its corresponding initial denotation. + * - Two distinct overloads will have SymDenotations with distinct + * signatures (the SELECTin tag in Tasty relies on this to refer to an + * overload unambiguously). Note that this only applies to + * SymDenotations, in general we cannot assume that distinct + * SingleDenotations will have distinct signatures (cf #9050). + */ final def signature(using Context): Signature = - if (isType) Signature.NotAMethod // don't force info if this is a type SymDenotation + if (isType) Signature.NotAMethod // don't force info if this is a type denotation else info match { - case info: MethodicType => - try info.signature + case info: MethodOrPoly => + try info.signature(isJava = symbol.is(JavaDefined)) catch { // !!! DEBUG case scala.util.control.NonFatal(ex) => report.echo(s"cannot take signature of $info") diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 0f72b6724795..e53b47f737e5 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -3405,12 +3405,12 @@ object Types { mySignature end signature + /** The Scala signature of this method. Note that two distinct Java method + * overloads may have the same Scala signature, the other overload of + * `signature` can be used to avoid ambiguity if necessary. + */ final override def signature(using Context): Signature = - def isJava(tp: Type): Boolean = tp match - case tp: PolyType => isJava(tp.resultType) - case tp: MethodType => tp.isJavaMethod - case _ => false - signature(isJava = isJava(this)) + signature(isJava = false) final override def hashCode: Int = System.identityHashCode(this) From c91d5bf262a6b2b2d8f389084fa06d3d93e28d71 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 2 Feb 2021 18:04:46 +0100 Subject: [PATCH 3/5] Get rid of JavaMethodType This is possible thanks to the previous commit, in all remaining places where we called `isJavaMethod` we can rely on contextual informations to obtain the same information, so we can completely get rid of it which is a nice simplification. Also get rid of ElimRepeated#transformTypeOfTree which turned out to be a no-op: the denotation transformer already transforms method types and there's nothing to do on top of that. --- .../dotty/tools/dotc/core/Definitions.scala | 1 - .../src/dotty/tools/dotc/core/NamerOps.scala | 2 +- .../dotty/tools/dotc/core/TypeErasure.scala | 5 +-- .../src/dotty/tools/dotc/core/Types.scala | 38 +++---------------- .../dotc/core/classfile/ClassfileParser.scala | 2 +- .../tools/dotc/transform/ElimRepeated.scala | 23 +++-------- .../tools/dotc/transform/PickleQuotes.scala | 2 +- .../dotty/tools/dotc/typer/Applications.scala | 2 +- .../tools/dotc/typer/ErrorReporting.scala | 7 +--- .../src/dotty/tools/dotc/typer/Namer.scala | 2 +- .../dotty/tools/dotc/typer/TypeAssigner.scala | 2 +- .../src/dotty/tools/dotc/typer/Typer.scala | 2 +- .../quoted/runtime/impl/QuotesImpl.scala | 2 +- 13 files changed, 22 insertions(+), 68 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index ef4005f6c1f1..c4fa803bd007 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -122,7 +122,6 @@ class Definitions { } val resParamRef = enterTypeParam(cls, paramNamePrefix ++ "R", Covariant, decls).typeRef val methodType = MethodType.companion( - isJava = false, isContextual = name.isContextFunction, isImplicit = false, isErased = name.isErasedFunction) diff --git a/compiler/src/dotty/tools/dotc/core/NamerOps.scala b/compiler/src/dotty/tools/dotc/core/NamerOps.scala index 8997d5ed1e1c..c45e820c0e90 100644 --- a/compiler/src/dotty/tools/dotc/core/NamerOps.scala +++ b/compiler/src/dotty/tools/dotc/core/NamerOps.scala @@ -39,7 +39,7 @@ object NamerOps: val (isContextual, isImplicit, isErased) = if params.isEmpty then (false, false, false) else (params.head.is(Given), params.head.is(Implicit), params.head.is(Erased)) - val make = MethodType.companion(isJava = isJava, isContextual = isContextual, isImplicit = isImplicit, isErased = isErased) + val make = MethodType.companion(isContextual = isContextual, isImplicit = isImplicit, isErased = isErased) if isJava then for param <- params do if param.info.isDirectRef(defn.ObjectClass) then param.info = defn.AnyType diff --git a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala index b5bd1cfb92bc..31c6982696cc 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala @@ -249,8 +249,7 @@ object TypeErasure { !classify(tp).derivesFrom(defn.ObjectClass) && !tp.symbol.is(JavaDefined) case tp: TypeParamRef => - !classify(tp).derivesFrom(defn.ObjectClass) && - !tp.binder.resultType.isJavaMethod + !classify(tp).derivesFrom(defn.ObjectClass) case tp: TypeAlias => isUnboundedGeneric(tp.alias) case tp: TypeBounds => val upper = classify(tp.hi) @@ -474,7 +473,7 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean TypeComparer.orType(this(tp1), this(tp2), isErased = true) case tp: MethodType => def paramErasure(tpToErase: Type) = - erasureFn(tp.isJavaMethod, semiEraseVCs, isConstructor, wildcardOK)(tpToErase) + erasureFn(isJava, semiEraseVCs, isConstructor, wildcardOK)(tpToErase) val (names, formals0) = if (tp.isErasedMethod) (Nil, Nil) else (tp.paramNames, tp.paramInfos) val formals = formals0.mapConserve(paramErasure) eraseResult(tp.resultType) match { diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index e53b47f737e5..2ed39e04dc1f 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -77,9 +77,7 @@ object Types { * +- GroundType -+- AndType * +- OrType * +- MethodOrPoly ---+-- PolyType - * +-- MethodType ---+- ImplicitMethodType - * +- ContextualMethodType - * | +- JavaMethodType + * | +-- MethodType * +- ClassInfo * | * +- NoType @@ -400,9 +398,6 @@ object Types { /** Is this an alias TypeBounds? */ final def isTypeAlias: Boolean = this.isInstanceOf[TypeAlias] - /** Is this a MethodType which is from Java? */ - def isJavaMethod: Boolean = false - /** Is this a Method or PolyType which has implicit or contextual parameters? */ def isImplicitMethod: Boolean = false @@ -1725,35 +1720,22 @@ object Types { * @param dropLast The number of trailing parameters that should be dropped * when forming the function type. */ - def toFunctionType(dropLast: Int = 0)(using Context): Type = this match { + def toFunctionType(isJava: Boolean, dropLast: Int = 0)(using Context): Type = this match { case mt: MethodType if !mt.isParamDependent => val formals1 = if (dropLast == 0) mt.paramInfos else mt.paramInfos dropRight dropLast val isContextual = mt.isContextualMethod && !ctx.erasedTypes val isErased = mt.isErasedMethod && !ctx.erasedTypes val result1 = mt.nonDependentResultApprox match { - case res: MethodType => res.toFunctionType() + case res: MethodType => res.toFunctionType(isJava) case res => res } val funType = defn.FunctionOf( - formals1 mapConserve (_.translateFromRepeated(toArray = mt.isJavaMethod)), + formals1 mapConserve (_.translateFromRepeated(toArray = isJava)), result1, isContextual, isErased) if (mt.isResultDependent) RefinedType(funType, nme.apply, mt) else funType } - final def dropJavaMethod(using Context): Type = this match - case pt: PolyType => pt.derivedLambdaType(resType = pt.resType.dropJavaMethod) - - case mt: MethodType => - if mt.isJavaMethod then - MethodType.apply(mt.paramNames, mt.paramInfos, mt.resType.dropJavaMethod) - else - mt.derivedLambdaType(resType = mt.resType.dropJavaMethod) - - case _ => this - - end dropJavaMethod - /** The signature of this type. This is by default NotAMethod, * but is overridden for PolyTypes, MethodTypes, and TermRef types. * (the reason why we deviate from the "final-method-with-pattern-match-in-base-class" @@ -3558,7 +3540,6 @@ object Types { def companion: MethodTypeCompanion - final override def isJavaMethod: Boolean = companion eq JavaMethodType final override def isImplicitMethod: Boolean = companion.eq(ImplicitMethodType) || companion.eq(ErasedImplicitMethodType) || @@ -3654,21 +3635,14 @@ object Types { } object MethodType extends MethodTypeCompanion("MethodType") { - def companion(isJava: Boolean = false, isContextual: Boolean = false, isImplicit: Boolean = false, isErased: Boolean = false): MethodTypeCompanion = - if (isJava) { - assert(!isImplicit) - assert(!isErased) - assert(!isContextual) - JavaMethodType - } - else if (isContextual) + def companion(isContextual: Boolean = false, isImplicit: Boolean = false, isErased: Boolean = false): MethodTypeCompanion = + if (isContextual) if (isErased) ErasedContextualMethodType else ContextualMethodType else if (isImplicit) if (isErased) ErasedImplicitMethodType else ImplicitMethodType else if (isErased) ErasedMethodType else MethodType } - object JavaMethodType extends MethodTypeCompanion("JavaMethodType") object ErasedMethodType extends MethodTypeCompanion("ErasedMethodType") object ContextualMethodType extends MethodTypeCompanion("ContextualMethodType") object ErasedContextualMethodType extends MethodTypeCompanion("ErasedContextualMethodType") diff --git a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala index 8c5fb61c9da7..ebaebd3a7937 100644 --- a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala +++ b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala @@ -447,7 +447,7 @@ class ClassfileParser( index += 1 val restype = sig2type(tparams, skiptvs = false) - JavaMethodType(paramnames.toList, paramtypes.toList, restype) + MethodType(paramnames.toList, paramtypes.toList, restype) case 'T' => val n = subName(';'.==).toTypeName index += 1 diff --git a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala index c1be520cf0d5..f4d11f5a51a3 100644 --- a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala +++ b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala @@ -31,7 +31,7 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => override def changesMembers: Boolean = true // the phase adds vararg forwarders def transformInfo(tp: Type, sym: Symbol)(using Context): Type = - elimRepeated(tp) + elimRepeated(tp, sym.is(JavaDefined)) /** Create forwarder symbols for the methods that are annotated * with `@varargs` or that override java varargs. @@ -88,15 +88,14 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => (sym.allOverriddenSymbols.exists(s => s.is(JavaDefined) || hasVarargsAnnotation(s))) /** Eliminate repeated parameters from method types. */ - private def elimRepeated(tp: Type)(using Context): Type = tp.stripTypeVar match + private def elimRepeated(tp: Type, isJava: Boolean)(using Context): Type = tp.stripTypeVar match case tp @ MethodTpe(paramNames, paramTypes, resultType) => - val resultType1 = elimRepeated(resultType) + val resultType1 = elimRepeated(resultType, isJava) val paramTypes1 = val lastIdx = paramTypes.length - 1 if lastIdx >= 0 then val last = paramTypes(lastIdx) if last.isRepeatedParam then - val isJava = tp.isJavaMethod // We need to be careful when handling Java repeated parameters // of the form `Object...` or `T...` where `T` is unbounded: // in both cases, `Object` will have been translated to `FromJavaObject` @@ -117,22 +116,10 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => else paramTypes tp.derivedLambdaType(paramNames, paramTypes1, resultType1) case tp: PolyType => - tp.derivedLambdaType(tp.paramNames, tp.paramInfos, elimRepeated(tp.resultType)) + tp.derivedLambdaType(tp.paramNames, tp.paramInfos, elimRepeated(tp.resultType, isJava)) case tp => tp - def transformTypeOfTree(tree: Tree)(using Context): Tree = - tree.withType(elimRepeated(tree.tpe)) - - override def transformTypeApply(tree: TypeApply)(using Context): Tree = - transformTypeOfTree(tree) - - override def transformIdent(tree: Ident)(using Context): Tree = - transformTypeOfTree(tree) - - override def transformSelect(tree: Select)(using Context): Tree = - transformTypeOfTree(tree) - override def transformApply(tree: Apply)(using Context): Tree = val args = tree.args.mapConserve { case arg: Typed if isWildcardStarArg(arg) => @@ -147,7 +134,7 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => arg.expr case arg => arg } - transformTypeOfTree(cpy.Apply(tree)(tree.fun, args)) + cpy.Apply(tree)(tree.fun, args) /** Convert sequence argument to Java array */ private def seqToArray(tree: Tree)(using Context): Tree = tree match diff --git a/compiler/src/dotty/tools/dotc/transform/PickleQuotes.scala b/compiler/src/dotty/tools/dotc/transform/PickleQuotes.scala index e0f0858652cf..4ab263bb66a8 100644 --- a/compiler/src/dotty/tools/dotc/transform/PickleQuotes.scala +++ b/compiler/src/dotty/tools/dotc/transform/PickleQuotes.scala @@ -377,7 +377,7 @@ class PickleQuotes extends MacroTransform { (tree: Tree) => { def newCapture = { val tpw = tree.tpe.widen match { - case tpw: MethodicType => tpw.toFunctionType() + case tpw: MethodicType => tpw.toFunctionType(isJava = false) case tpw => tpw } assert(tpw.isInstanceOf[ValueType]) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 1d1122e65f16..12444d8239d6 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -628,7 +628,7 @@ trait Applications extends Compatibility { false case argtpe => def SAMargOK = formal match { - case SAMType(sam) => argtpe <:< sam.toFunctionType() + case SAMType(sam) => argtpe <:< sam.toFunctionType(isJava = formal.classSymbol.is(JavaDefined)) case _ => false } isCompatible(argtpe, formal) || ctx.mode.is(Mode.ImplicitsEnabled) && SAMargOK diff --git a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala index be512af9a7c6..ffac54960578 100644 --- a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala +++ b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala @@ -118,12 +118,7 @@ object ErrorReporting { /** A subtype log explaining why `found` does not conform to `expected` */ def whyNoMatchStr(found: Type, expected: Type): String = { - val found1 = found.dropJavaMethod - val expected1 = expected.dropJavaMethod - if ((found1 eq found) != (expected eq expected1) && (found1 <:< expected1)) - i""" - |(Note that Scala's and Java's representation of this type differs)""" - else if (ctx.settings.explainTypes.value) + if (ctx.settings.explainTypes.value) i""" |${ctx.typerState.constraint} |${TypeComparer.explained(_.isSubType(found, expected))}""" diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 674c4ea065f5..f975e1852b25 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -1049,7 +1049,7 @@ class Namer { typer: Typer => if sym.isStableMember && sym.isPublic && !refersToPrivate(path.tpe) then (StableRealizable, ExprType(path.tpe.select(sym))) else - (EmptyFlags, mbr.info.ensureMethodic.dropJavaMethod) + (EmptyFlags, mbr.info.ensureMethodic) var mbrFlags = Exported | Method | Final | maybeStable | sym.flags & RetainedExportFlags if sym.is(ExtensionMethod) then mbrFlags |= ExtensionMethod val forwarderName = checkNoConflict(alias, isPrivate = false, span) diff --git a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala index 36adc62487c4..8bc16f1aa738 100644 --- a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -375,7 +375,7 @@ trait TypeAssigner { def assignType(tree: untpd.Closure, meth: Tree, target: Tree)(using Context): Closure = tree.withType( - if (target.isEmpty) meth.tpe.widen.toFunctionType(tree.env.length) + if (target.isEmpty) meth.tpe.widen.toFunctionType(isJava = meth.symbol.is(JavaDefined), tree.env.length) else target.tpe) def assignType(tree: untpd.CaseDef, pat: Tree, body: Tree)(using Context): CaseDef = { diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 6093e5cb3c29..e2b15c96c0d0 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -3603,7 +3603,7 @@ class Typer extends Namer if defn.isFunctionType(wtp) && !defn.isFunctionType(pt) => pt match { case SAMType(sam) - if wtp <:< sam.toFunctionType() => + if wtp <:< sam.toFunctionType(isJava = pt.classSymbol.is(JavaDefined)) => // was ... && isFullyDefined(pt, ForceDegree.flipBottom) // but this prevents case blocks from implementing polymorphic partial functions, // since we do not know the result parameter a priori. Have to wait until the diff --git a/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala b/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala index 134f7e8507b8..746ea78e0ba9 100644 --- a/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala +++ b/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala @@ -385,7 +385,7 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler def etaExpand(owner: Symbol): Term = self.tpe.widen match { case mtpe: Types.MethodType if !mtpe.isParamDependent => val closureResType = mtpe.resType match { - case t: Types.MethodType => t.toFunctionType() + case t: Types.MethodType => t.toFunctionType(isJava = self.symbol.is(JavaDefined)) case t => t } val closureTpe = Types.MethodType(mtpe.paramNames, mtpe.paramInfos, closureResType) From 3fcc12f3ea7a2e3222ca3f5225bf3f4d24bd1ee8 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 11 Feb 2021 17:37:03 +0100 Subject: [PATCH 4/5] Consistent erasure for Java method signatures Use Jave erasure to compute the result type part of Java method signatures too. I don't think that two distinct non-bridge Java overloads can only differ in their result type so this shouldn't fix anything, but it's more consistent. --- compiler/src/dotty/tools/dotc/core/Types.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 2ed39e04dc1f..c5749640a758 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -3367,7 +3367,7 @@ object Types { case tp: ExprType => tp.signature case tp => if tp.isRef(defn.UnitClass) then Signature(Nil, defn.UnitClass.fullName.asTypeName) - else Signature(tp, isJava = false) + else Signature(tp, isJava) this match case tp: MethodType => val params = if (isErasedMethod) Nil else tp.paramInfos From 590d587d639d4aa270c879a863c3ac75c56a06e9 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 9 Feb 2021 15:58:27 +0100 Subject: [PATCH 5/5] Fix overriding checks involving Scala and Java methods To check which methods are in an overriding relationship, we rely on denotation matching which checks that the methods have matching signatures, but because Scala and Java have different erasure rules, seemingly valid overrides might not be seen as such because they end up with different signatures. To compensate for this we change SingleDenotation#matchesLoosely to use the Scala signatures of both methods when comparing a Scala and a Java method. This works even though the methods end up with different erased types because Erasure will insert bridges. Note that even after this change, we cannot simply override a Java generic `Array[T]` with a Scala `Array[T]`, because we intentionally parse these Java types as `Array[T & Object]`, see `TypeApplications.translateJavaArrayElementType` for details and tests/{neg,pos}/i1747.scala for examples. --- .../dotty/tools/dotc/core/Denotations.scala | 74 ++++++++++++------- tests/pos-java-interop/i9109b/A.java | 3 + tests/pos-java-interop/i9109b/B.scala | 3 + tests/pos-java-interop/i9109b/Intf.java | 1 + tests/pos/i8615b.scala | 26 +++++++ 5 files changed, 79 insertions(+), 28 deletions(-) create mode 100644 tests/pos-java-interop/i9109b/A.java create mode 100644 tests/pos-java-interop/i9109b/B.scala create mode 100644 tests/pos-java-interop/i9109b/Intf.java create mode 100644 tests/pos/i8615b.scala diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index c60f5bdbad4f..24aa48213f31 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -595,10 +595,17 @@ object Denotations { * SingleDenotations will have distinct signatures (cf #9050). */ final def signature(using Context): Signature = + signature(isJava = !isType && symbol.is(JavaDefined)) + + /** Overload of `signature` which lets the caller pick between the Java and + * Scala signature of the info. Useful to match denotations defined in + * different classes (see `matchesLoosely`). + */ + def signature(isJava: Boolean)(using Context): Signature = if (isType) Signature.NotAMethod // don't force info if this is a type denotation else info match { case info: MethodOrPoly => - try info.signature(isJava = symbol.is(JavaDefined)) + try info.signature(isJava) catch { // !!! DEBUG case scala.util.control.NonFatal(ex) => report.echo(s"cannot take signature of $info") @@ -1004,34 +1011,45 @@ object Denotations { symbol.hasTargetName(other.symbol.targetName) && matchesLoosely(other) - /** matches without a target name check */ + /** `matches` without a target name check. + * + * We consider a Scala method and a Java method to match if they have + * matching Scala signatures. This allows us to override some Java + * definitions even if they have a different erasure (see i8615b, + * i9109b), Erasure takes care of adding any necessary bridge to make + * this work at runtime. + */ def matchesLoosely(other: SingleDenotation)(using Context): Boolean = - val d = signature.matchDegree(other.signature) - d match - case FullMatch => - true - case MethodNotAMethodMatch => - !ctx.erasedTypes && { - val isJava = symbol.is(JavaDefined) - val otherIsJava = other.symbol.is(JavaDefined) - // A Scala zero-parameter method and a Scala non-method always match. - if !isJava && !otherIsJava then - true - // Java allows defining both a field and a zero-parameter method with the same name, - // so they must not match. - else if isJava && otherIsJava then - false - // A Java field never matches a Scala method. - else if isJava then - symbol.is(Method) - else // otherIsJava - other.symbol.is(Method) - } - case ParamMatch => - // The signatures do not tell us enough to be sure about matching - !ctx.erasedTypes && info.matches(other.info) - case noMatch => - false + if isType then true + else + val isJava = symbol.is(JavaDefined) + val otherIsJava = other.symbol.is(JavaDefined) + val useJavaSig = isJava && otherIsJava + val sig = signature(isJava = useJavaSig) + val otherSig = other.signature(isJava = useJavaSig) + sig.matchDegree(otherSig) match + case FullMatch => + true + case MethodNotAMethodMatch => + !ctx.erasedTypes && { + // A Scala zero-parameter method and a Scala non-method always match. + if !isJava && !otherIsJava then + true + // Java allows defining both a field and a zero-parameter method with the same name, + // so they must not match. + else if isJava && otherIsJava then + false + // A Java field never matches a Scala method. + else if isJava then + symbol.is(Method) + else // otherIsJava + other.symbol.is(Method) + } + case ParamMatch => + // The signatures do not tell us enough to be sure about matching + !ctx.erasedTypes && info.matches(other.info) + case noMatch => + false def mapInherited(ownDenots: PreDenotation, prevDenots: PreDenotation, pre: Type)(using Context): SingleDenotation = if hasUniqueSym && prevDenots.containsSym(symbol) then NoDenotation diff --git a/tests/pos-java-interop/i9109b/A.java b/tests/pos-java-interop/i9109b/A.java new file mode 100644 index 000000000000..9a0e63f83077 --- /dev/null +++ b/tests/pos-java-interop/i9109b/A.java @@ -0,0 +1,3 @@ +public class A { + public void foo(T x) {} +} diff --git a/tests/pos-java-interop/i9109b/B.scala b/tests/pos-java-interop/i9109b/B.scala new file mode 100644 index 000000000000..8e05b329ec9f --- /dev/null +++ b/tests/pos-java-interop/i9109b/B.scala @@ -0,0 +1,3 @@ +class B extends A { + override def foo[T <: Object with Intf](x: T): Unit = {} +} diff --git a/tests/pos-java-interop/i9109b/Intf.java b/tests/pos-java-interop/i9109b/Intf.java new file mode 100644 index 000000000000..13b93a9241ac --- /dev/null +++ b/tests/pos-java-interop/i9109b/Intf.java @@ -0,0 +1 @@ +public interface Intf {} diff --git a/tests/pos/i8615b.scala b/tests/pos/i8615b.scala new file mode 100644 index 000000000000..559c1cb5648f --- /dev/null +++ b/tests/pos/i8615b.scala @@ -0,0 +1,26 @@ +class ArrayOrdering[N] extends Comparable[Array[N]] { + override def compareTo(x: Array[N]): Int = 0 + + compareTo(???) +} + +class ArrayIntOrdering extends Comparable[Array[Int]] { + override def compareTo(x: Array[Int]): Int = 0 + + compareTo(???) +} + +class ArrayOrdering2[N] extends Comparable[Array[N]] { + self: Serializable => + override def compareTo(x: Array[N]): Int = 0 + + compareTo(???) +} + +class ArrayIntOrdering2 extends Comparable[Array[Int]] { + self: Serializable => + + override def compareTo(x: Array[Int]): Int = 0 + + compareTo(???) +}