From 302990835dc5b358e8f187e3e0549d7903cc182f Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 14 Apr 2021 00:45:11 +0200 Subject: [PATCH 1/3] SAM types cannot be sealed or final Previously we caught this in Typer, but doing it in the `SAMType` extractor is more robust because it is used in overloading resolution, and this will be needed in a subsequent commit of this PR to avoid selecting the wrong overload in some cases. This required dropping the final flag from the compiler-generated ContextFunctionN, instead we explicitly prevent subclasses in RefChecks. --- compiler/src/dotty/tools/dotc/core/Definitions.scala | 3 +-- compiler/src/dotty/tools/dotc/core/Types.scala | 2 +- .../src/dotty/tools/dotc/reporting/ErrorMessageID.scala | 3 ++- compiler/src/dotty/tools/dotc/reporting/messages.scala | 6 ++++++ compiler/src/dotty/tools/dotc/typer/RefChecks.scala | 4 +++- compiler/src/dotty/tools/dotc/typer/Typer.scala | 4 ---- 6 files changed, 13 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 70eebc750521..73e5d147a100 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -133,8 +133,7 @@ class Definitions { ClassInfo(ScalaPackageClass.thisType, cls, ObjectType :: Nil, decls) } } - val flags0 = Trait | NoInits - val flags = if (name.isContextFunction) flags0 | Final else flags0 + val flags = Trait | NoInits newPermanentClassSymbol(ScalaPackageClass, name, flags, completer) } diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index f9cbf932f9c8..878f469d207b 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -5043,7 +5043,7 @@ object Types { NoType } def isInstantiatable(tp: Type)(using Context): Boolean = zeroParamClass(tp) match { - case cinfo: ClassInfo => + case cinfo: ClassInfo if !cinfo.cls.isOneOf(FinalOrSealed) => val selfType = cinfo.selfType.asSeenFrom(tp, cinfo.cls) tp <:< selfType case _ => diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala index d73e799ddf3c..23e1450f9225 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala @@ -173,7 +173,8 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] { CaseClassInInlinedCodeID, OverrideTypeMismatchErrorID, OverrideErrorID, - MatchableWarningID + MatchableWarningID, + CannotExtendFunctionID def errorNumber = ordinal - 2 } diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index b32f00dde083..d2d43b60ec6f 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -1558,6 +1558,12 @@ import transform.SymUtils._ def explain = "" } + class CannotExtendContextFunction(sym: Symbol)(using Context) + extends SyntaxMsg(CannotExtendFunctionID) { + def msg = em"""$sym cannot extend a context function class""" + def explain = "" + } + class JavaEnumParentArgs(parent: Type)(using Context) extends TypeMsg(JavaEnumParentArgsID) { def msg = em"""not enough arguments for constructor Enum: ${hl("(name: String, ordinal: Int)")}: ${hl(parent.show)}""" diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 7d9e25c33b2d..342e0bf3a3c3 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -96,7 +96,7 @@ object RefChecks { /** Check that self type of this class conforms to self types of parents * and required classes. Also check that only `enum` constructs extend - * `java.lang.Enum`. + * `java.lang.Enum` and no user-written class extends ContextFunctionN. */ private def checkParents(cls: Symbol, parentTrees: List[Tree])(using Context): Unit = cls.info match { case cinfo: ClassInfo => @@ -132,6 +132,8 @@ object RefChecks { case _ => false } + if psyms.exists(defn.isContextFunctionClass) then + report.error(CannotExtendContextFunction(cls), cls.sourcePos) /** Check that arguments passed to trait parameters conform to the parameter types * in the current class. This is necessary since parameter types might be narrowed diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 4433ac7dee90..e3b685fb1beb 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1399,10 +1399,6 @@ class Typer extends Namer else report.error(ex"result type of lambda is an underspecified SAM type $pt", tree.srcPos) pt - if (pt.classSymbol.isOneOf(FinalOrSealed)) { - val offendingFlag = pt.classSymbol.flags & FinalOrSealed - report.error(ex"lambda cannot implement $offendingFlag ${pt.classSymbol}", tree.srcPos) - } TypeTree(targetTpe) case _ => if (mt.isParamDependent) From 281d6f6f2d9554cbed07d65f527c74d0d949c418 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 15 Apr 2021 10:06:19 +0200 Subject: [PATCH 2/3] Drop first pass in narrowByTrees The two pass approach was originally introduced in 295c981f719d09a690076af9d87d48679b1ceb42 to lower the priority of implicit conversions, but just a month later in 62531258003dba513a21847d58395823d6840363 a similar two pass approach was used to call resolveOverloaded which itself calls narrowByTrees, rendering the first commit moot. Therefore, we can just remove this first pass and all associated code without affecting anything. --- .../dotty/tools/dotc/typer/Applications.scala | 33 +++++++------------ 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 48a5738b4074..a67ea603186b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -614,7 +614,6 @@ trait Applications extends Compatibility { /** The degree to which an argument has to match a formal parameter */ enum ArgMatch: - case SubType // argument is a relaxed subtype of formal case Compatible // argument is compatible with formal case CompatibleCAP // capture-converted argument is compatible with formal @@ -639,17 +638,14 @@ trait Applications extends Compatibility { case SAMType(sam) => argtpe <:< sam.toFunctionType(isJava = formal.classSymbol.is(JavaDefined)) case _ => false } - if argMatch == ArgMatch.SubType then - argtpe relaxed_<:< formal.widenExpr - else - isCompatible(argtpe, formal) - || ctx.mode.is(Mode.ImplicitsEnabled) && SAMargOK - || argMatch == ArgMatch.CompatibleCAP - && { - val argtpe1 = argtpe.widen - val captured = captureWildcards(argtpe1) - (captured ne argtpe1) && isCompatible(captured, formal.widenExpr) - } + isCompatible(argtpe, formal) + || ctx.mode.is(Mode.ImplicitsEnabled) && SAMargOK + || argMatch == ArgMatch.CompatibleCAP + && { + val argtpe1 = argtpe.widen + val captured = captureWildcards(argtpe1) + (captured ne argtpe1) && isCompatible(captured, formal.widenExpr) + } /** The type of the given argument */ protected def argType(arg: Arg, formal: Type): Type @@ -1863,17 +1859,10 @@ trait Applications extends Compatibility { else alts - def narrowByTrees(alts: List[TermRef], args: List[Tree], resultType: Type): List[TermRef] = { - val alts2 = alts.filterConserve(alt => - isApplicableMethodRef(alt, args, resultType, keepConstraint = false, ArgMatch.SubType) + def narrowByTrees(alts: List[TermRef], args: List[Tree], resultType: Type): List[TermRef] = + alts.filterConserve(alt => + isApplicableMethodRef(alt, args, resultType, keepConstraint = false, ArgMatch.CompatibleCAP) ) - if (alts2.isEmpty && !ctx.isAfterTyper) - alts.filterConserve(alt => - isApplicableMethodRef(alt, args, resultType, keepConstraint = false, ArgMatch.CompatibleCAP) - ) - else - alts2 - } record("resolveOverloaded.FunProto", alts.length) val alts1 = narrowBySize(alts) From 0470bfc3aad342e13028ecf04ff0d9e058339bbe Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 15 Apr 2021 10:59:03 +0200 Subject: [PATCH 3/3] Overloading resolution: Handle SAM types more like Java and Scala 2 `resolveOverloaded` is called twice, first with implicit conversions off, then on. Before this commit, turning off implicit conversions also turned off SAM conversions, this behavior does not match Java or Scala 2 which means we could end up picking a different overload than they do (cf #11938). This commit enables SAM conversions in the first pass, _except_ for conversions to PartialFunction as that would break a lot of existing code and wouldn't match how either Java or Scala 2 pick overloads. This is an alternative to #11945 (which special-cased SAM types with an explicit `@FunctionalInterfaces` annotation) and #11990 (which special-cased SAM types that subtype a scala Function type). Special-casing PartialFunction instead seems more defensible since it's already special-cased in Scala 2 and is not considered a SAM type by Java. Fixes #11938. --- .../dotty/tools/dotc/typer/Applications.scala | 30 +++++++++++++++---- tests/neg/i11938.scala | 16 ++++++++++ tests/run/i11938.scala | 22 ++++++++++++++ 3 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 tests/neg/i11938.scala create mode 100644 tests/run/i11938.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index a67ea603186b..126892a8f132 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -634,12 +634,32 @@ trait Applications extends Compatibility { // matches expected type false case argtpe => - def SAMargOK = formal match { - case SAMType(sam) => argtpe <:< sam.toFunctionType(isJava = formal.classSymbol.is(JavaDefined)) - case _ => false - } + val argtpe1 = argtpe.widen + + def SAMargOK = + defn.isFunctionType(argtpe1) && formal.match + case SAMType(sam) => argtpe <:< sam.toFunctionType(isJava = formal.classSymbol.is(JavaDefined)) + case _ => false + isCompatible(argtpe, formal) - || ctx.mode.is(Mode.ImplicitsEnabled) && SAMargOK + // Only allow SAM-conversion to PartialFunction if implicit conversions + // are enabled. This is necessary to avoid ambiguity between an overload + // taking a PartialFunction and one taking a Function1 because + // PartialFunction extends Function1 but Function1 is SAM-convertible to + // PartialFunction. Concretely, given: + // + // def foo(a: Int => Int): Unit = println("1") + // def foo(a: PartialFunction[Int, Int]): Unit = println("2") + // + // - `foo(x => x)` will print 1, because the PartialFunction overload + // won't be seen as applicable in the first call to + // `resolveOverloaded`, this behavior happens to match what Java does + // since PartialFunction is not a SAM type according to Java + // (`isDefined` is abstract). + // - `foo { case x if x % 2 == 0 => x }` will print 2, because both + // overloads are applicable, but PartialFunction is a subtype of + // Function1 so it's more specific. + || (!formal.isRef(defn.PartialFunctionClass) || ctx.mode.is(Mode.ImplicitsEnabled)) && SAMargOK || argMatch == ArgMatch.CompatibleCAP && { val argtpe1 = argtpe.widen diff --git a/tests/neg/i11938.scala b/tests/neg/i11938.scala new file mode 100644 index 000000000000..e8f4e747915b --- /dev/null +++ b/tests/neg/i11938.scala @@ -0,0 +1,16 @@ +import java.util.function.Function + +object Test { + def foo[V](v: V): Int = 1 + def foo[U](fn: Function[Int, U]): Int = 2 + + def main(args: Array[String]): Unit = { + val f: Int => Int = x => x + foo(f) // error + // Like Scala 2, we emit an error here because the Function1 argument was + // deemed SAM-convertible to Function, even though it's not a lambda + // expression and therefore not convertible. If we wanted to support this, + // we would have to tweak TestApplication#argOK to look at the shape of + // `arg` and turn off SAM conversions when it's a non-closure tree. + } +} diff --git a/tests/run/i11938.scala b/tests/run/i11938.scala new file mode 100644 index 000000000000..ac39ecfc7f61 --- /dev/null +++ b/tests/run/i11938.scala @@ -0,0 +1,22 @@ +import java.util.function.Function + +object Test { + def foo[V](v: V): Int = 1 + def foo[U](fn: Function[Int, U]): Int = 2 + + def foo2(a: Int => Int): Int = 1 + def foo2(a: PartialFunction[Int, Int]): Int = 2 + + def main(args: Array[String]): Unit = { + assert(foo((x: Int) => x) == 2) + val jf: Function[Int, Int] = x => x + assert(foo(jf) == 2) + + assert(foo2(x => x) == 1) + val f: Int => Int = x => x + assert(foo2(f) == 1) + assert(foo2({ case x if x % 2 == 0 => x }) == 2) + val pf: PartialFunction[Int, Int] = { case x if x % 2 == 0 => x } + assert(foo2(pf) == 2) + } +}