From f1ec7a5dde79c8a8c0d424a60755acb19efaca73 Mon Sep 17 00:00:00 2001 From: odersky Date: Sun, 15 Sep 2024 18:51:45 +0200 Subject: [PATCH 1/3] Three fixes for SAM type handling The first two fixes concern characterization of SAM types. One condition of a SAM type is that it can be instantiated with an empty argument list. This was implemented incorrectly. First, we missed the case where the SAM type is a trait with a parent class that takes arguments. In this case the SAM type _cannot_ be instantiated with an empty argument list. Second, we missed the case where the SAM type constructor has a single vararg parameter. In this case the SAM type _can_ be instantiated with an empty argument list. The second case was also translated incorrectly which led to illegal bytecodes. Fixes #15855 [Cherry-picked f0b6763327249f4198dcfe081a9183ed777bcdfe] --- compiler/src/dotty/tools/dotc/ast/tpd.scala | 75 ++++++++++++++----- .../src/dotty/tools/dotc/core/Phases.scala | 2 +- .../src/dotty/tools/dotc/core/Types.scala | 19 +++-- .../tools/dotc/transform/ExpandSAMs.scala | 10 ++- tests/neg/i15855.scala | 10 +++ tests/run/i15855.scala | 20 +++++ 6 files changed, 105 insertions(+), 31 deletions(-) create mode 100644 tests/neg/i15855.scala create mode 100644 tests/run/i15855.scala diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index 5adbef96ec89..9a52b51e68cf 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -305,24 +305,53 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { def TypeDef(sym: TypeSymbol)(using Context): TypeDef = ta.assignType(untpd.TypeDef(sym.name, TypeTree(sym.info)), sym) - def ClassDef(cls: ClassSymbol, constr: DefDef, body: List[Tree], superArgs: List[Tree] = Nil)(using Context): TypeDef = { + /** Create a class definition + * @param cls the class symbol of the created class + * @param constr its primary constructor + * @param body the statements in its template + * @param superArgs the arguments to pass to the superclass constructor + * @param adaptVarargs if true, allow matching a vararg superclass constructor + * with a missing argument in superArgs, and synthesize an + * empty repeated parameter in the supercall in this case + */ + def ClassDef(cls: ClassSymbol, constr: DefDef, body: List[Tree], + superArgs: List[Tree] = Nil, adaptVarargs: Boolean = false)(using Context): TypeDef = val firstParent :: otherParents = cls.info.parents: @unchecked + + def isApplicable(constr: Symbol): Boolean = + def recur(ctpe: Type): Boolean = ctpe match + case ctpe: PolyType => + recur(ctpe.instantiate(firstParent.argTypes)) + case ctpe: MethodType => + var paramInfos = ctpe.paramInfos + if adaptVarargs && paramInfos.length == superArgs.length + 1 + && atPhaseNoLater(Phases.elimRepeatedPhase)(constr.info.isVarArgsMethod) + then // accept missing argument for varargs parameter + paramInfos = paramInfos.init + superArgs.corresponds(paramInfos)(_.tpe <:< _) + case _ => + false + recur(constr.info) + + def adaptedSuperArgs(ctpe: Type): List[Tree] = ctpe match + case ctpe: PolyType => + adaptedSuperArgs(ctpe.instantiate(firstParent.argTypes)) + case ctpe: MethodType + if ctpe.paramInfos.length == superArgs.length + 1 => + // last argument must be a vararg, otherwise isApplicable would have failed + superArgs :+ + repeated(Nil, TypeTree(ctpe.paramInfos.last.argInfos.head, inferred = true)) + case _ => + superArgs + val superRef = - if (cls.is(Trait)) TypeTree(firstParent) - else { - def isApplicable(ctpe: Type): Boolean = ctpe match { - case ctpe: PolyType => - isApplicable(ctpe.instantiate(firstParent.argTypes)) - case ctpe: MethodType => - (superArgs corresponds ctpe.paramInfos)(_.tpe <:< _) - case _ => - false - } - val constr = firstParent.decl(nme.CONSTRUCTOR).suchThat(constr => isApplicable(constr.info)) - New(firstParent, constr.symbol.asTerm, superArgs) - } + if cls.is(Trait) then TypeTree(firstParent) + else + val constr = firstParent.decl(nme.CONSTRUCTOR).suchThat(isApplicable) + New(firstParent, constr.symbol.asTerm, adaptedSuperArgs(constr.info)) + ClassDefWithParents(cls, constr, superRef :: otherParents.map(TypeTree(_)), body) - } + end ClassDef def ClassDefWithParents(cls: ClassSymbol, constr: DefDef, parents: List[Tree], body: List[Tree])(using Context): TypeDef = { val selfType = @@ -349,13 +378,18 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { * @param parents a non-empty list of class types * @param termForwarders a non-empty list of forwarding definitions specified by their name and the definition they forward to. * @param typeMembers a possibly-empty list of type members specified by their name and their right hand side. + * @param adaptVarargs if true, allow matching a vararg superclass constructor + * with a missing argument in superArgs, and synthesize an + * empty repeated parameter in the supercall in this case * * The class has the same owner as the first function in `termForwarders`. * Its position is the union of all symbols in `termForwarders`. */ - def AnonClass(parents: List[Type], termForwarders: List[(TermName, TermSymbol)], - typeMembers: List[(TypeName, TypeBounds)] = Nil)(using Context): Block = { - AnonClass(termForwarders.head._2.owner, parents, termForwarders.map(_._2.span).reduceLeft(_ union _)) { cls => + def AnonClass(parents: List[Type], + termForwarders: List[(TermName, TermSymbol)], + typeMembers: List[(TypeName, TypeBounds)], + adaptVarargs: Boolean)(using Context): Block = { + AnonClass(termForwarders.head._2.owner, parents, termForwarders.map(_._2.span).reduceLeft(_ union _), adaptVarargs) { cls => def forwarder(name: TermName, fn: TermSymbol) = { val fwdMeth = fn.copy(cls, name, Synthetic | Method | Final).entered.asTerm for overridden <- fwdMeth.allOverriddenSymbols do @@ -375,6 +409,9 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { * with the specified owner and position. */ def AnonClass(owner: Symbol, parents: List[Type], coord: Coord)(body: ClassSymbol => List[Tree])(using Context): Block = + AnonClass(owner, parents, coord, adaptVarargs = false)(body) + + private def AnonClass(owner: Symbol, parents: List[Type], coord: Coord, adaptVarargs: Boolean)(body: ClassSymbol => List[Tree])(using Context): Block = val parents1 = if (parents.head.classSymbol.is(Trait)) { val head = parents.head.parents.head @@ -383,7 +420,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { else parents val cls = newNormalizedClassSymbol(owner, tpnme.ANON_CLASS, Synthetic | Final, parents1, coord = coord) val constr = newConstructor(cls, Synthetic, Nil, Nil).entered - val cdef = ClassDef(cls, DefDef(constr), body(cls)) + val cdef = ClassDef(cls, DefDef(constr), body(cls), Nil, adaptVarargs) Block(cdef :: Nil, New(cls.typeRef, Nil)) def Import(expr: Tree, selectors: List[untpd.ImportSelector])(using Context): Import = diff --git a/compiler/src/dotty/tools/dotc/core/Phases.scala b/compiler/src/dotty/tools/dotc/core/Phases.scala index 08827dfedce2..e3e28139a150 100644 --- a/compiler/src/dotty/tools/dotc/core/Phases.scala +++ b/compiler/src/dotty/tools/dotc/core/Phases.scala @@ -486,7 +486,7 @@ object Phases { def sbtExtractDependenciesPhase(using Context): Phase = ctx.base.sbtExtractDependenciesPhase def picklerPhase(using Context): Phase = ctx.base.picklerPhase def inliningPhase(using Context): Phase = ctx.base.inliningPhase - def stagingPhase(using Context): Phase = ctx.base.stagingPhase + def stagingPhase(using Context): Phase = ctx.base.stagingPhase def splicingPhase(using Context): Phase = ctx.base.splicingPhase def firstTransformPhase(using Context): Phase = ctx.base.firstTransformPhase def refchecksPhase(using Context): Phase = ctx.base.refchecksPhase diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index aafdd87a2a8d..c8670cba0ebe 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -5613,16 +5613,21 @@ object Types extends TypeUtils { def samClass(tp: Type)(using Context): Symbol = tp match case tp: ClassInfo => def zeroParams(tp: Type): Boolean = tp.stripPoly match - case mt: MethodType => mt.paramInfos.isEmpty && !mt.resultType.isInstanceOf[MethodType] + case mt: MethodType => + val noArgsNeeded = mt.paramInfos match + case Nil => true + case info :: Nil => info.isRepeatedParam + case _ => false + noArgsNeeded && !mt.resultType.isInstanceOf[MethodType] case et: ExprType => true case _ => false - val cls = tp.cls - val validCtor = + def validCtor(cls: Symbol): Boolean = val ctor = cls.primaryConstructor - // `ContextFunctionN` does not have constructors - !ctor.exists || zeroParams(ctor.info) - val isInstantiable = !cls.isOneOf(FinalOrSealed) && (tp.appliedRef <:< tp.selfType) - if validCtor && isInstantiable then tp.cls + (!ctor.exists || zeroParams(ctor.info)) // `ContextFunctionN` does not have constructors + && (!cls.is(Trait) || validCtor(cls.info.parents.head.classSymbol)) + def isInstantiable = + !tp.cls.isOneOf(FinalOrSealed) && (tp.appliedRef <:< tp.selfType) + if validCtor(tp.cls) && isInstantiable then tp.cls else NoSymbol case tp: AppliedType => samClass(tp.superType) diff --git a/compiler/src/dotty/tools/dotc/transform/ExpandSAMs.scala b/compiler/src/dotty/tools/dotc/transform/ExpandSAMs.scala index e9c16052894b..f62044eae7d2 100644 --- a/compiler/src/dotty/tools/dotc/transform/ExpandSAMs.scala +++ b/compiler/src/dotty/tools/dotc/transform/ExpandSAMs.scala @@ -69,10 +69,12 @@ class ExpandSAMs extends MiniPhase: val tpe1 = collectAndStripRefinements(tpe) val Seq(samDenot) = tpe1.possibleSamMethods cpy.Block(tree)(stats, - AnonClass(List(tpe1), - List(samDenot.symbol.asTerm.name -> fn.symbol.asTerm), - refinements.toList - ) + transformFollowingDeep: + AnonClass(List(tpe1), + List(samDenot.symbol.asTerm.name -> fn.symbol.asTerm), + refinements.toList, + adaptVarargs = true + ) ) } case _ => diff --git a/tests/neg/i15855.scala b/tests/neg/i15855.scala new file mode 100644 index 000000000000..ba9112032419 --- /dev/null +++ b/tests/neg/i15855.scala @@ -0,0 +1,10 @@ +// crash.scala +import scala.language.implicitConversions + +class MyFunction(args: String) + +trait MyFunction0[+R] extends MyFunction { + def apply(): R +} + +def fromFunction0[R](f: Function0[R]): MyFunction0[R] = () => f() // error diff --git a/tests/run/i15855.scala b/tests/run/i15855.scala new file mode 100644 index 000000000000..880d6d806132 --- /dev/null +++ b/tests/run/i15855.scala @@ -0,0 +1,20 @@ +// crash.scala +import scala.language.implicitConversions + +class MyFunction(args: String*) + +trait MyFunction0[+R] extends MyFunction { + def apply(): R +} + +abstract class MyFunction1[R](args: R*): + def apply(): R + +def fromFunction0[R](f: Function0[R]): MyFunction0[R] = () => f() +def fromFunction1[R](f: Function0[R]): MyFunction1[R] = () => f() + +@main def Test = + val m0: MyFunction0[Int] = fromFunction0(() => 1) + val m1: MyFunction1[Int] = fromFunction1(() => 2) + assert(m0() == 1) + assert(m1() == 2) From cbbb7e399952487f39548e7d609b8575bbab3e65 Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Tue, 3 Dec 2024 22:48:53 +0100 Subject: [PATCH 2/3] Align SAM test and expansion Fix SAM test to use the same scheme as SAM expansion to determine whether a type needs zero arguments for construction. [Cherry-picked 8aa59f834eaf26965ddc9cb16f7392db15445cf9][modified] --- compiler/src/dotty/tools/dotc/ast/tpd.scala | 22 +++--------- .../src/dotty/tools/dotc/core/TypeUtils.scala | 34 ++++++++++++++++--- .../src/dotty/tools/dotc/core/Types.scala | 24 ++++++------- 3 files changed, 45 insertions(+), 35 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index 9a52b51e68cf..d1f5f48ceb87 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -318,21 +318,6 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { superArgs: List[Tree] = Nil, adaptVarargs: Boolean = false)(using Context): TypeDef = val firstParent :: otherParents = cls.info.parents: @unchecked - def isApplicable(constr: Symbol): Boolean = - def recur(ctpe: Type): Boolean = ctpe match - case ctpe: PolyType => - recur(ctpe.instantiate(firstParent.argTypes)) - case ctpe: MethodType => - var paramInfos = ctpe.paramInfos - if adaptVarargs && paramInfos.length == superArgs.length + 1 - && atPhaseNoLater(Phases.elimRepeatedPhase)(constr.info.isVarArgsMethod) - then // accept missing argument for varargs parameter - paramInfos = paramInfos.init - superArgs.corresponds(paramInfos)(_.tpe <:< _) - case _ => - false - recur(constr.info) - def adaptedSuperArgs(ctpe: Type): List[Tree] = ctpe match case ctpe: PolyType => adaptedSuperArgs(ctpe.instantiate(firstParent.argTypes)) @@ -347,8 +332,11 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { val superRef = if cls.is(Trait) then TypeTree(firstParent) else - val constr = firstParent.decl(nme.CONSTRUCTOR).suchThat(isApplicable) - New(firstParent, constr.symbol.asTerm, adaptedSuperArgs(constr.info)) + val parentConstr = firstParent.applicableConstructors(superArgs.tpes, adaptVarargs) match + case Nil => assert(false, i"no applicable parent constructor of $firstParent for supercall arguments $superArgs") + case constr :: Nil => constr + case _ => assert(false, i"multiple applicable parent constructors of $firstParent for supercall arguments $superArgs") + New(firstParent, parentConstr.asTerm, adaptedSuperArgs(parentConstr.info)) ClassDefWithParents(cls, constr, superRef :: otherParents.map(TypeTree(_)), body) end ClassDef diff --git a/compiler/src/dotty/tools/dotc/core/TypeUtils.scala b/compiler/src/dotty/tools/dotc/core/TypeUtils.scala index 2a926903181a..cbe4a7f8fd9a 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeUtils.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeUtils.scala @@ -5,12 +5,13 @@ package core import TypeErasure.ErasedValueType import Types.*, Contexts.*, Symbols.*, Flags.*, Decorators.* import Names.Name +import StdNames.nme -class TypeUtils { +class TypeUtils: /** A decorator that provides methods on types * that are needed in the transformer pipeline. */ - extension (self: Type) { + extension (self: Type) def isErasedValueType(using Context): Boolean = self.isInstanceOf[ErasedValueType] @@ -125,5 +126,30 @@ class TypeUtils { def takesImplicitParams(using Context): Boolean = self.stripPoly match case mt: MethodType => mt.isImplicitMethod || mt.resType.takesImplicitParams case _ => false - } -} + + /** The constructors of this tyoe that that are applicable to `argTypes`, without needing + * an implicit conversion. + * @param adaptVarargs if true, allow a constructor with just a varargs argument to + * match an empty argument list. + */ + def applicableConstructors(argTypes: List[Type], adaptVarargs: Boolean)(using Context): List[Symbol] = + def isApplicable(constr: Symbol): Boolean = + def recur(ctpe: Type): Boolean = ctpe match + case ctpe: PolyType => + if argTypes.isEmpty then recur(ctpe.resultType) // no need to know instances + else recur(ctpe.instantiate(self.argTypes)) + case ctpe: MethodType => + var paramInfos = ctpe.paramInfos + if adaptVarargs && paramInfos.length == argTypes.length + 1 + && atPhaseNoLater(Phases.elimRepeatedPhase)(constr.info.isVarArgsMethod) + then // accept missing argument for varargs parameter + paramInfos = paramInfos.init + argTypes.corresponds(paramInfos)(_ <:< _) + case _ => + false + recur(constr.info) + + self.decl(nme.CONSTRUCTOR).altsWith(isApplicable).map(_.symbol) + +end TypeUtils + diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index c8670cba0ebe..5da6c31d241b 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -5612,22 +5612,18 @@ object Types extends TypeUtils { def samClass(tp: Type)(using Context): Symbol = tp match case tp: ClassInfo => - def zeroParams(tp: Type): Boolean = tp.stripPoly match - case mt: MethodType => - val noArgsNeeded = mt.paramInfos match - case Nil => true - case info :: Nil => info.isRepeatedParam - case _ => false - noArgsNeeded && !mt.resultType.isInstanceOf[MethodType] - case et: ExprType => true - case _ => false - def validCtor(cls: Symbol): Boolean = - val ctor = cls.primaryConstructor - (!ctor.exists || zeroParams(ctor.info)) // `ContextFunctionN` does not have constructors - && (!cls.is(Trait) || validCtor(cls.info.parents.head.classSymbol)) + val cls = tp.cls + def takesNoArgs(tp: Type) = + !tp.classSymbol.primaryConstructor.exists + // e.g. `ContextFunctionN` does not have constructors + || tp.applicableConstructors(Nil, adaptVarargs = true).lengthCompare(1) == 0 + // we require a unique constructor so that SAM expansion is deterministic + val noArgsNeeded: Boolean = + takesNoArgs(tp) + && (!tp.cls.is(Trait) || takesNoArgs(tp.parents.head)) def isInstantiable = !tp.cls.isOneOf(FinalOrSealed) && (tp.appliedRef <:< tp.selfType) - if validCtor(tp.cls) && isInstantiable then tp.cls + if noArgsNeeded && isInstantiable then tp.cls else NoSymbol case tp: AppliedType => samClass(tp.superType) From 81ff618de9efbdd256d5222380837c66c5d2b655 Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Tue, 3 Dec 2024 22:49:57 +0100 Subject: [PATCH 3/3] Address review comments [Cherry-picked 3af67baa337ac8791326fd7b94a752495bdd0f57][modified] --- compiler/src/dotty/tools/dotc/core/TypeUtils.scala | 6 +++--- tests/neg/i15855.scala | 11 ++++++++--- tests/run/i15855.scala | 3 --- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeUtils.scala b/compiler/src/dotty/tools/dotc/core/TypeUtils.scala index cbe4a7f8fd9a..8167bf532e1a 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeUtils.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeUtils.scala @@ -127,8 +127,8 @@ class TypeUtils: case mt: MethodType => mt.isImplicitMethod || mt.resType.takesImplicitParams case _ => false - /** The constructors of this tyoe that that are applicable to `argTypes`, without needing - * an implicit conversion. + /** The constructors of this type that are applicable to `argTypes`, without needing + * an implicit conversion. Curried constructors are always excluded. * @param adaptVarargs if true, allow a constructor with just a varargs argument to * match an empty argument list. */ @@ -144,7 +144,7 @@ class TypeUtils: && atPhaseNoLater(Phases.elimRepeatedPhase)(constr.info.isVarArgsMethod) then // accept missing argument for varargs parameter paramInfos = paramInfos.init - argTypes.corresponds(paramInfos)(_ <:< _) + argTypes.corresponds(paramInfos)(_ <:< _) && !ctpe.resultType.isInstanceOf[MethodType] case _ => false recur(constr.info) diff --git a/tests/neg/i15855.scala b/tests/neg/i15855.scala index ba9112032419..c1d316ccae81 100644 --- a/tests/neg/i15855.scala +++ b/tests/neg/i15855.scala @@ -1,6 +1,3 @@ -// crash.scala -import scala.language.implicitConversions - class MyFunction(args: String) trait MyFunction0[+R] extends MyFunction { @@ -8,3 +5,11 @@ trait MyFunction0[+R] extends MyFunction { } def fromFunction0[R](f: Function0[R]): MyFunction0[R] = () => f() // error + +class MyFunctionWithImplicit(implicit args: String) + +trait MyFunction0WithImplicit[+R] extends MyFunctionWithImplicit { + def apply(): R +} + +def fromFunction1[R](f: Function0[R]): MyFunction0WithImplicit[R] = () => f() // error diff --git a/tests/run/i15855.scala b/tests/run/i15855.scala index 880d6d806132..b67bcb11d18a 100644 --- a/tests/run/i15855.scala +++ b/tests/run/i15855.scala @@ -1,6 +1,3 @@ -// crash.scala -import scala.language.implicitConversions - class MyFunction(args: String*) trait MyFunction0[+R] extends MyFunction {