From 675d44fce583776c977d663ba8fbaec175bbe35d 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 c2762427539e..f0a41e2947fa 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 10f1996001e9..4544b2c5ef4e 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 23ad69fd0a61..bc1b3f9f583f 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala @@ -174,7 +174,8 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] { OverrideTypeMismatchErrorID, OverrideErrorID, MatchableWarningID, - IllegalParameterInitID + IllegalParameterInitID, + 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 4e2b254186cc..534922b3d0b3 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 49b91a37c43f..d7cccc1b5309 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1398,10 +1398,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 ccfbf15999d4174ed8eb45ffbf5691b723543f83 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 990a01f966fe29458aa21d9ef1d8b0d098aaaf46 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) + } +}