From fe40c594a1694e8de7c1d797ed22769c3a61164b Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 4 Apr 2022 12:23:59 +0200 Subject: [PATCH 1/7] Fix Scala Wart about implicit () class parameters Fixes #2576 As the discussion in #2576 shows, we still have some problems with the implicitly inserted empty parameter lists for class constructors. We do need that empty list to support syntax like `C()` and `new C()`. But it gets in the way if a class has using clauses. Example from the issue: ```scala class Bar(using x: Int)(y: String) given Int = ??? def test = new Bar("") ``` Here, an implicitly inserted `()` in front makes the last line fail. We'd need `new Bar()("")`. If a class has only using clauses as parameters we now insert a `()` at the end instead of at the start. That makes the example compile. For old-style implicit parameters we don't have a choice. We still need the `()` at the start since otherwise we'd change the meaning of calls with explicit arguments for the implicit parameters. --- .../src/dotty/tools/dotc/ast/Desugar.scala | 14 ++++--- .../tools/dotc/config/PathResolver.scala | 8 +++- .../src/dotty/tools/dotc/core/NamerOps.scala | 17 +++++++-- .../dotty/tools/dotc/transform/Bridges.scala | 2 +- .../src/dotty/tools/dotc/typer/Typer.scala | 37 +++++++++++-------- tests/neg/i12344.scala | 20 +++++----- tests/pos/given-constrapps.scala | 1 - tests/pos/i2576.scala | 3 ++ tests/run-macros/i12021/Macro_1.scala | 1 + tests/run/i2567.scala | 4 +- 10 files changed, 67 insertions(+), 40 deletions(-) create mode 100644 tests/pos/i2576.scala diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 9cd30b94d692..28c1214762fb 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -585,12 +585,16 @@ object desugar { // new C[Ts](paramss) lazy val creatorExpr = { - val vparamss = constrVparamss match { - case (vparam :: _) :: _ if vparam.mods.isOneOf(GivenOrImplicit) => // add a leading () to match class parameters + val vparamss = constrVparamss match + case (vparam :: _) :: _ if vparam.mods.is(Implicit) => // add a leading () to match class parameters Nil :: constrVparamss case _ => - constrVparamss - } + if constrVparamss.nonEmpty && constrVparamss.forall { + case vparam :: _ => vparam.mods.is(Given) + case _ => false + } + then constrVparamss :+ Nil // add a trailing () to match class parameters + else constrVparamss val nu = vparamss.foldLeft(makeNew(classTypeRef)) { (nu, vparams) => val app = Apply(nu, vparams.map(refOfDef)) vparams match { @@ -818,7 +822,7 @@ object desugar { } flatTree(cdef1 :: companions ::: implicitWrappers ::: enumScaffolding) - }.showing(i"desugared: $result", Printers.desugar) + }.showing(i"desugared: $cdef --> $result", Printers.desugar) /** Expand * diff --git a/compiler/src/dotty/tools/dotc/config/PathResolver.scala b/compiler/src/dotty/tools/dotc/config/PathResolver.scala index f9c49c1a7723..afa30e38dc2a 100644 --- a/compiler/src/dotty/tools/dotc/config/PathResolver.scala +++ b/compiler/src/dotty/tools/dotc/config/PathResolver.scala @@ -131,7 +131,9 @@ object PathResolver { def fromPathString(path: String)(using Context): ClassPath = { val settings = ctx.settings.classpath.update(path) - new PathResolver()(using ctx.fresh.setSettings(settings)).result + inContext(ctx.fresh.setSettings(settings)) { + new PathResolver().result + } } /** Show values in Environment and Defaults when no argument is provided. @@ -147,7 +149,9 @@ object PathResolver { val ArgsSummary(sstate, rest, errors, warnings) = ctx.settings.processArguments(args.toList, true, ctx.settingsState) errors.foreach(println) - val pr = new PathResolver()(using ctx.fresh.setSettings(sstate)) + val pr = inContext(ctx.fresh.setSettings(sstate)) { + new PathResolver() + } println(" COMMAND: 'scala %s'".format(args.mkString(" "))) println("RESIDUAL: 'scala %s'\n".format(rest.mkString(" "))) diff --git a/compiler/src/dotty/tools/dotc/core/NamerOps.scala b/compiler/src/dotty/tools/dotc/core/NamerOps.scala index 56022e9fcaf2..1a878c9547b1 100644 --- a/compiler/src/dotty/tools/dotc/core/NamerOps.scala +++ b/compiler/src/dotty/tools/dotc/core/NamerOps.scala @@ -18,14 +18,23 @@ object NamerOps: case TypeSymbols(tparams) :: _ => ctor.owner.typeRef.appliedTo(tparams.map(_.typeRef)) case _ => ctor.owner.typeRef - /** if isConstructor, make sure it has one leading non-implicit parameter list */ + /** If isConstructor, make sure it has at least one non-implicit parameter list + * This is done by adding a () in front of a leading old style implicit parameter, + * or by adding a () as last -- or only -- parameter list if the constructor has + * only using clauses as parameters. + */ def normalizeIfConstructor(paramss: List[List[Symbol]], isConstructor: Boolean)(using Context): List[List[Symbol]] = if !isConstructor then paramss else paramss match - case Nil :: _ => paramss - case TermSymbols(vparam :: _) :: _ if !vparam.isOneOf(GivenOrImplicit) => paramss case TypeSymbols(tparams) :: paramss1 => tparams :: normalizeIfConstructor(paramss1, isConstructor) - case _ => Nil :: paramss + case TermSymbols(vparam :: _) :: _ if vparam.is(Implicit) => Nil :: paramss + case _ => + if paramss.forall { + case TermSymbols(vparams) => vparams.nonEmpty && vparams.head.is(Given) + case _ => true + } + then paramss :+ Nil + else paramss /** The method type corresponding to given parameters and result type */ def methodType(paramss: List[List[Symbol]], resultType: Type, isJava: Boolean = false)(using Context): Type = diff --git a/compiler/src/dotty/tools/dotc/transform/Bridges.scala b/compiler/src/dotty/tools/dotc/transform/Bridges.scala index 912b8ca69131..071109147b45 100644 --- a/compiler/src/dotty/tools/dotc/transform/Bridges.scala +++ b/compiler/src/dotty/tools/dotc/transform/Bridges.scala @@ -170,7 +170,7 @@ class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(using Context) { * time deferred methods in `stats` that are replaced by a bridge with the same signature. */ def add(stats: List[untpd.Tree]): List[untpd.Tree] = - val opc = new BridgesCursor()(using preErasureCtx) + val opc = inContext(preErasureCtx) { new BridgesCursor } while opc.hasNext do if !opc.overriding.is(Deferred) then addBridgeIfNeeded(opc.overriding, opc.overridden) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 2ae55032d7ae..bdbf1066377d 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2356,25 +2356,32 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer /** If `ref` is an implicitly parameterized trait, pass an implicit argument list. * Otherwise, if `ref` is a parameterized trait, error. - * Note: Traits and classes currently always have at least an empty parameter list () - * before the implicit parameters (this is inserted if not given in source). - * We skip this parameter list when deciding whether a trait is parameterless or not. + * Note: Traits and classes have sometimes a synthesized empty parameter list () + * in front or after the implicit parameter(s). See NamerOps.normalizeIfConstructor. + * We synthesize a () argument at the correct place in this case. * @param ref The tree referring to the (parent) trait * @param psym Its type symbol - * @param cinfo The info of its constructor */ - def maybeCall(ref: Tree, psym: Symbol): Tree = psym.primaryConstructor.info.stripPoly match - case cinfo @ MethodType(Nil) if cinfo.resultType.isImplicitMethod => + def maybeCall(ref: Tree, psym: Symbol): Tree = + def appliedRef = typedExpr(untpd.New(untpd.TypedSplice(ref)(using superCtx), Nil))(using superCtx) - case cinfo @ MethodType(Nil) if !cinfo.resultType.isInstanceOf[MethodType] => - ref - case cinfo: MethodType => - if !ctx.erasedTypes then // after constructors arguments are passed in super call. - typr.println(i"constr type: $cinfo") - report.error(ParameterizedTypeLacksArguments(psym), ref.srcPos) - ref - case _ => - ref + def dropContextual(tp: Type): Type = tp.stripPoly match + case mt: MethodType if mt.isContextualMethod => dropContextual(mt.resType) + case _ => tp + psym.primaryConstructor.info.stripPoly match + case cinfo @ MethodType(Nil) + if cinfo.resultType.isImplicitMethod && !cinfo.resultType.isContextualMethod => + appliedRef + case cinfo => + val cinfo1 = dropContextual(cinfo) + cinfo1 match + case cinfo1 @ MethodType(Nil) if !cinfo1.resultType.isInstanceOf[MethodType] => + if cinfo1 ne cinfo then appliedRef else ref + case cinfo1: MethodType if !ctx.erasedTypes => + report.error(ParameterizedTypeLacksArguments(psym), ref.srcPos) + ref + case _ => + ref val seenParents = mutable.Set[Symbol]() diff --git a/tests/neg/i12344.scala b/tests/neg/i12344.scala index 88daf535e699..b2fd6d27c37c 100644 --- a/tests/neg/i12344.scala +++ b/tests/neg/i12344.scala @@ -2,15 +2,15 @@ import scala.quoted.* class C(using q: Quotes)(i: Int = 1, f: q.reflect.Flags = q.reflect.Flags.EmptyFlags) -def test1a(using q: Quotes) = new C() // error -def test2a(using q: Quotes) = new C(1) // error -def test3a(using q: Quotes) = new C(1, q.reflect.Flags.Lazy) // error -def test4a(using q: Quotes) = new C(f = q.reflect.Flags.Lazy) // error +def test1a(using q: Quotes) = new C() +def test2a(using q: Quotes) = new C(1) +def test3a(using q: Quotes) = new C(1, q.reflect.Flags.Lazy) +def test4a(using q: Quotes) = new C(f = q.reflect.Flags.Lazy) -def test1b(using q: Quotes) = C() // error -def test2b(using q: Quotes) = C(1) // error -def test3b(using q: Quotes) = C(1, q.reflect.Flags.Lazy) // error -def test4b(using q: Quotes) = C(f = q.reflect.Flags.Lazy) // error +def test1b(using q: Quotes) = C() +def test2b(using q: Quotes) = C(1) +def test3b(using q: Quotes) = C(1, q.reflect.Flags.Lazy) +def test4b(using q: Quotes) = C(f = q.reflect.Flags.Lazy) def test1c(using q: Quotes) = new C(using q)() def test2c(using q: Quotes) = new C(using q)(1) @@ -22,5 +22,5 @@ def test2d(using q: Quotes) = C(using q)(1) def test3d(using q: Quotes) = C(using q)(1, q.reflect.Flags.Lazy) def test4d(using q: Quotes) = C(using q)(f = q.reflect.Flags.Lazy) -def test1e(using q: Quotes) = new C()() -def test2e(using q: Quotes) = C()() +def test1e(using q: Quotes) = new C()() // error +def test2e(using q: Quotes) = C()() // error diff --git a/tests/pos/given-constrapps.scala b/tests/pos/given-constrapps.scala index f92653542861..cdea7967f15e 100644 --- a/tests/pos/given-constrapps.scala +++ b/tests/pos/given-constrapps.scala @@ -19,7 +19,6 @@ class Foo(using TC) { object Test extends App { new C(using tc) - new C()(using tc) new C(using tc) {} new C2(1)(using tc)(using List(tc)) new C2(1)(using tc)(using List(tc)) {} diff --git a/tests/pos/i2576.scala b/tests/pos/i2576.scala new file mode 100644 index 000000000000..ac8cdefb1b30 --- /dev/null +++ b/tests/pos/i2576.scala @@ -0,0 +1,3 @@ +class Bar(using x: Int)(y: String) +given Int = ??? +def test = new Bar("") diff --git a/tests/run-macros/i12021/Macro_1.scala b/tests/run-macros/i12021/Macro_1.scala index 4c10cf2ca17d..81703dfbab3d 100644 --- a/tests/run-macros/i12021/Macro_1.scala +++ b/tests/run-macros/i12021/Macro_1.scala @@ -9,6 +9,7 @@ def inspect2[A: Type](using Quotes): Expr[String] = { val ps = TypeRepr.of[A].typeSymbol.primaryConstructor.tree match case DefDef(_, List(Nil, ps: TermParamClause), _, _) => ps + case DefDef(_, List(ps: TermParamClause, Nil), _, _) => ps case DefDef(_, List(ps: TermParamClause), _, _) => ps val names = ps.params.map(p => s"${p.name}: ${p.tpt.show}").mkString("(", ", ", ")") diff --git a/tests/run/i2567.scala b/tests/run/i2567.scala index c9625aeb33d1..4cd60080faa7 100644 --- a/tests/run/i2567.scala +++ b/tests/run/i2567.scala @@ -10,7 +10,7 @@ object Test extends App { new Foo new Foo(using tc) new Foo() - new Foo()(using tc) + new Foo(using tc) Foo() - Foo()(using tc) + Foo(using tc) } \ No newline at end of file From 3763e6be1d1585564ed16f6c74953ec50730a1f6 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 4 Apr 2022 14:00:27 +0200 Subject: [PATCH 2/7] Update semanticDB expect file --- tests/semanticdb/metac.expect | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/semanticdb/metac.expect b/tests/semanticdb/metac.expect index 732e9b07aa85..436eebfe27b5 100644 --- a/tests/semanticdb/metac.expect +++ b/tests/semanticdb/metac.expect @@ -1808,7 +1808,7 @@ givens/InventedNames$package.given_String. => final implicit lazy val given meth givens/InventedNames$package.given_X. => final implicit given object given_X extends Object with X { self: given_X.type => +2 decls } givens/InventedNames$package.given_X.doX(). => method doX => Int <: givens/X#doX(). givens/InventedNames$package.given_Y# => implicit given class given_Y extends Object with Y { self: given_Y => +3 decls } -givens/InventedNames$package.given_Y#``(). => primary ctor ()(implicit val given param x$1: X): given_Y +givens/InventedNames$package.given_Y#``(). => primary ctor (implicit val given param x$1: X)(): given_Y givens/InventedNames$package.given_Y#``().(x$1) => implicit val given param x$1: X givens/InventedNames$package.given_Y#doY(). => method doY => String <: givens/Y#doY(). givens/InventedNames$package.given_Y#x$1. => protected implicit val given method x$1 X From 2fc2aa2881ed0bf4a20ac6f337f6aedc39fc2648 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 4 Apr 2022 14:15:26 +0200 Subject: [PATCH 3/7] Harden patch logic --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index bdbf1066377d..fe5290c7ab2d 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -3408,7 +3408,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer def isContextBoundParams = wtp.stripPoly match case MethodType(EvidenceParamName(_) :: _) => true case _ => false - if sourceVersion == `future-migration` && isContextBoundParams + if sourceVersion == `future-migration` && isContextBoundParams && pt.args.nonEmpty then // Under future-migration, don't infer implicit arguments yet for parameters // coming from context bounds. Issue a warning instead and offer a patch. report.migrationWarning( From 00a839e3cd5db3e4b68c8f0790f8923bc2029b4a Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 4 Apr 2022 22:48:30 +0200 Subject: [PATCH 4/7] Make TreeUnpickler read pre-3.2 Tasty files correctly This is needed to ensure backwards Tasty compatibility --- .../tools/dotc/core/tasty/TreeUnpickler.scala | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index 06dc4872c7f3..cd45baf26b72 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -27,7 +27,7 @@ import typer.ConstFold import typer.Checking.checkNonCyclic import typer.Nullables._ import util.Spans._ -import util.SourceFile +import util.{SourceFile, Property} import ast.{Trees, tpd, untpd} import Trees._ import Decorators._ @@ -1135,7 +1135,23 @@ class TreeUnpickler(reader: TastyReader, tpd.Super(qual, mixId, mixTpe.typeSymbol) case APPLY => val fn = readTerm() - tpd.Apply(fn, until(end)(readTerm())) + val args = until(end)(readTerm()) + // Adapt constructor calls where class has only using clauses from old to new scheme. + // Old: leading (), new: trailing (). + // This is neccessary so that we can read pre-3.2 Tasty correctly. There, + // constructor calls use the old scheme, but constructor definitions already + // use the new scheme, since they are reconstituted with normalizeIfConstructor. + if fn.symbol.isConstructor && fn.tpe.widen.isContextualMethod && args.isEmpty then + fn.withAttachment(SuppressedApplyToNone, ()) + else + val res = tpd.Apply(fn, args) + if fn.removeAttachment(SuppressedApplyToNone).isEmpty then + res + else res.tpe.widen match + case MethodType(Nil) => + res.appliedToNone + case mt: MethodType if mt.isContextualMethod => + res.withAttachment(SuppressedApplyToNone, ()) case TYPEAPPLY => tpd.TypeApply(readTerm(), until(end)(readTpt())) case TYPED => @@ -1523,4 +1539,9 @@ object TreeUnpickler { inline val AllDefs = 2 // add everything class TreeWithoutOwner extends Exception + + /** An attachment key indicating that an old-style leading () in a constructor + * call that has otherwise only using clauses was suppressed. + */ + val SuppressedApplyToNone: Property.Key[Unit] = Property.Key() } From df9f466ac4963f1cd36322b451aa4ff23ec9d856 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 5 Apr 2022 13:32:29 +0200 Subject: [PATCH 5/7] Test tasty backwards compatibility This currently fail with: MethodType(List(y), List(TypeRef(TermRef(TermRef(ThisType(TypeRef(NoPrefix,module class )),object scala),Predef),String)), TypeRef(ThisType(TypeRef(NoPrefix,module class )),class Bar)) (of class dotty.tools.dotc.core.Types$CachedMethodType) at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readLengthTerm$1(TreeUnpickler.scala:1154) at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readTerm(TreeUnpickler.scala:1313) at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readLengthTerm$1(TreeUnpickler.scala:1137) at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readTerm(TreeUnpickler.scala:1313) at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.$anonfun$24(TreeUnpickler.scala:1138) at dotty.tools.tasty.TastyReader.until(TastyReader.scala:125) at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readLengthTerm$1(TreeUnpickler.scala:1138) at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readTerm(TreeUnpickler.scala:1313) at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readIndexedStat(TreeUnpickler.scala:998) at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readIndexedStats$$anonfun$1(TreeUnpickler.scala:1036) at dotty.tools.tasty.TastyReader.until(TastyReader.scala:125) at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readIndexedStats(TreeUnpickler.scala:1036) at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readStats(TreeUnpickler.scala:1040) at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readLengthTerm$1(TreeUnpickler.scala:1166) at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readTerm(TreeUnpickler.scala:1313) at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readLengthTerm$1(TreeUnpickler.scala:1158) at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readTerm(TreeUnpickler.scala:1313) at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.createMemberSymbol$$anonfun$1(TreeUnpickler.scala:614) at dotty.tools.dotc.core.Annotations$.dotty$tools$dotc$core$Annotations$$anon$2$$_$$lessinit$greater$$anonfun$1(Annotations.scala:162) at dotty.tools.dotc.core.Annotations$LazyBodyAnnotation.tree(Annotations.scala:150) at dotty.tools.dotc.typer.Inliner$.bodyToInline(Inliner.scala:53) at dotty.tools.dotc.typer.Inliner$.inlineCall(Inliner.scala:157) at dotty.tools.dotc.transform.Inlining$InliningTreeMap.transform(Inlining.scala:70) at dotty.tools.dotc.ast.Trees$Instance$TreeMap.transform$$anonfun$1(Trees.scala:1494) at scala.collection.immutable.List.mapConserve(List.scala:472) at dotty.tools.dotc.ast.Trees$Instance$TreeMap.transform(Trees.scala:1494) at dotty.tools.dotc.ast.Trees$Instance$TreeMap.transform(Trees.scala:1388) at dotty.tools.dotc.ast.TreeMapWithImplicits.transform(TreeMapWithImplicits.scala:73) at dotty.tools.dotc.transform.Inlining$InliningTreeMap.transform(Inlining.scala:78) at dotty.tools.dotc.ast.tpd$TreeMapWithPreciseStatContexts.transformBlock$$anonfun$1$$anonfun$1(tpd.scala:1211) at dotty.tools.dotc.ast.tpd$TreeMapWithPreciseStatContexts.loop$2(tpd.scala:1193) at dotty.tools.dotc.ast.tpd$TreeMapWithPreciseStatContexts.transformStats(tpd.scala:1206) at dotty.tools.dotc.ast.tpd$TreeMapWithPreciseStatContexts.transformBlock(tpd.scala:1211) at dotty.tools.dotc.ast.Trees$Instance$TreeMap.transform(Trees.scala:1402) at dotty.tools.dotc.ast.TreeMapWithImplicits.transform(TreeMapWithImplicits.scala:49) at dotty.tools.dotc.transform.Inlining$InliningTreeMap.transform(Inlining.scala:66) at dotty.tools.dotc.ast.TreeMapWithImplicits.transform(TreeMapWithImplicits.scala:56) at dotty.tools.dotc.transform.Inlining$InliningTreeMap.transform(Inlining.scala:64) at dotty.tools.dotc.ast.tpd$TreeMapWithPreciseStatContexts.loop$2(tpd.scala:1206) at dotty.tools.dotc.ast.tpd$TreeMapWithPreciseStatContexts.transformStats(tpd.scala:1206) at dotty.tools.dotc.ast.tpd$TreeMapWithPreciseStatContexts.transformStats(tpd.scala:1208) at dotty.tools.dotc.ast.TreeMapWithImplicits.transform(TreeMapWithImplicits.scala:64) at dotty.tools.dotc.transform.Inlining$InliningTreeMap.transform(Inlining.scala:64) at dotty.tools.dotc.ast.Trees$Instance$TreeMap.transform(Trees.scala:1465) at dotty.tools.dotc.ast.TreeMapWithImplicits.transform(TreeMapWithImplicits.scala:73) at dotty.tools.dotc.transform.Inlining$InliningTreeMap.transform(Inlining.scala:64) at dotty.tools.dotc.ast.tpd$TreeMapWithPreciseStatContexts.loop$2(tpd.scala:1206) at dotty.tools.dotc.ast.tpd$TreeMapWithPreciseStatContexts.transformStats(tpd.scala:1206) at dotty.tools.dotc.ast.tpd$TreeMapWithPreciseStatContexts.transformStats(tpd.scala:1208) at dotty.tools.dotc.ast.Trees$Instance$TreeMap.transform(Trees.scala:1476) at dotty.tools.dotc.ast.TreeMapWithImplicits.transform(TreeMapWithImplicits.scala:73) at dotty.tools.dotc.transform.Inlining$InliningTreeMap.transform(Inlining.scala:78) at dotty.tools.dotc.transform.Inlining$$anon$2.transform(Inlining.scala:56) at dotty.tools.dotc.transform.MacroTransform.run(MacroTransform.scala:18) at dotty.tools.dotc.transform.Inlining.run(Inlining.scala:28) --- tests/run/backwardsCompat-implicitParens/A_1_c3.0.2.scala | 6 ++++++ tests/run/backwardsCompat-implicitParens/Test_2.scala | 5 +++++ 2 files changed, 11 insertions(+) create mode 100644 tests/run/backwardsCompat-implicitParens/A_1_c3.0.2.scala create mode 100644 tests/run/backwardsCompat-implicitParens/Test_2.scala diff --git a/tests/run/backwardsCompat-implicitParens/A_1_c3.0.2.scala b/tests/run/backwardsCompat-implicitParens/A_1_c3.0.2.scala new file mode 100644 index 000000000000..7d8462ccad04 --- /dev/null +++ b/tests/run/backwardsCompat-implicitParens/A_1_c3.0.2.scala @@ -0,0 +1,6 @@ +class Bar(using x: Int)(y: String) +object Bar: + given Int = 1 + inline def foo = + println(new Bar()("")) + println(Bar()("")) diff --git a/tests/run/backwardsCompat-implicitParens/Test_2.scala b/tests/run/backwardsCompat-implicitParens/Test_2.scala new file mode 100644 index 000000000000..ae2acba2835b --- /dev/null +++ b/tests/run/backwardsCompat-implicitParens/Test_2.scala @@ -0,0 +1,5 @@ +@main def Test = + given Int = 1 + println(new Bar("")) + println(Bar("")) + println(Bar.foo) From 6e9be57fe3ef68a6e740f739eff50d24382f2eef Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 5 Apr 2022 15:49:14 +0200 Subject: [PATCH 6/7] Fix unpickler logic and beef up test --- .../tools/dotc/core/tasty/TreeUnpickler.scala | 53 +++++++++++++------ .../run/backwardsCompat-implicitParens.check | 20 +++++++ .../A_1_c3.0.2.scala | 28 +++++++++- .../Test_2.scala | 14 +++++ 4 files changed, 98 insertions(+), 17 deletions(-) create mode 100644 tests/run/backwardsCompat-implicitParens.check diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index cd45baf26b72..15cfc656f222 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -1125,6 +1125,36 @@ class TreeUnpickler(reader: TastyReader, readPathTerm() } + /** Adapt constructor calls where class has only using clauses from old to new scheme. + * or class has mixed using clauses and other clauses. + * Old: leading (), new: nothing, or trailing () if all clauses are using clauses. + * This is neccessary so that we can read pre-3.2 Tasty correctly. There, + * constructor calls use the old scheme, but constructor definitions already + * use the new scheme, since they are reconstituted with normalizeIfConstructor. + */ + def constructorApply(fn: Tree, args: List[Tree]): Tree = + if fn.tpe.widen.isContextualMethod && args.isEmpty then + fn.withAttachment(SuppressedApplyToNone, ()) + else + val fn1 = fn match + case Apply(fn1, Nil) if fn.removeAttachment(InsertedApplyToNone).isDefined => + // We thought we inserted a final `()` but hit a user-written `()` instead. + // Remove the inserted `()`. + fn1 + case _ => + fn + val res = tpd.Apply(fn1, args) + if fn.removeAttachment(SuppressedApplyToNone).isEmpty then + res + else res.tpe.widen match + case mt @ MethodType(params) => + if params.isEmpty then + // Assume it's the final synthesized `()` parameter + res.appliedToNone.withAttachment(InsertedApplyToNone, ()) + else if mt.isContextualMethod then + res.withAttachment(SuppressedApplyToNone, ()) + else res + def readLengthTerm(): Tree = { val end = readEnd() val result = @@ -1136,22 +1166,8 @@ class TreeUnpickler(reader: TastyReader, case APPLY => val fn = readTerm() val args = until(end)(readTerm()) - // Adapt constructor calls where class has only using clauses from old to new scheme. - // Old: leading (), new: trailing (). - // This is neccessary so that we can read pre-3.2 Tasty correctly. There, - // constructor calls use the old scheme, but constructor definitions already - // use the new scheme, since they are reconstituted with normalizeIfConstructor. - if fn.symbol.isConstructor && fn.tpe.widen.isContextualMethod && args.isEmpty then - fn.withAttachment(SuppressedApplyToNone, ()) - else - val res = tpd.Apply(fn, args) - if fn.removeAttachment(SuppressedApplyToNone).isEmpty then - res - else res.tpe.widen match - case MethodType(Nil) => - res.appliedToNone - case mt: MethodType if mt.isContextualMethod => - res.withAttachment(SuppressedApplyToNone, ()) + if fn.symbol.isConstructor then constructorApply(fn, args) + else tpd.Apply(fn, args) case TYPEAPPLY => tpd.TypeApply(readTerm(), until(end)(readTpt())) case TYPED => @@ -1544,4 +1560,9 @@ object TreeUnpickler { * call that has otherwise only using clauses was suppressed. */ val SuppressedApplyToNone: Property.Key[Unit] = Property.Key() + + /** An attachment key indicating that an trailing () in a constructor + * call that has otherwise only using clauses was inserted. + */ + val InsertedApplyToNone: Property.Key[Unit] = Property.Key() } diff --git a/tests/run/backwardsCompat-implicitParens.check b/tests/run/backwardsCompat-implicitParens.check new file mode 100644 index 000000000000..1a471019f925 --- /dev/null +++ b/tests/run/backwardsCompat-implicitParens.check @@ -0,0 +1,20 @@ +Bar +Bar +Bar +Bar +() +Bat +Bat +Bat +Bat +() +Bax +Bax +Bax +Bax +() +Baz +Baz +Baz +Baz +() diff --git a/tests/run/backwardsCompat-implicitParens/A_1_c3.0.2.scala b/tests/run/backwardsCompat-implicitParens/A_1_c3.0.2.scala index 7d8462ccad04..77a77baf86e6 100644 --- a/tests/run/backwardsCompat-implicitParens/A_1_c3.0.2.scala +++ b/tests/run/backwardsCompat-implicitParens/A_1_c3.0.2.scala @@ -1,6 +1,32 @@ -class Bar(using x: Int)(y: String) +class Bar(using x: Int)(y: String): + override def toString = "Bar" object Bar: given Int = 1 inline def foo = println(new Bar()("")) println(Bar()("")) + +class Bat(using x: Int): + override def toString = "Bat" +object Bat: + given Int = 1 + inline def foo = + println(new Bat()) + println(Bat()) + +class Bax(using x: Int)(): + override def toString = "Bax" +object Bax: + given Int = 1 + inline def foo = + println(new Bax()) + println(Bax()) + +class Baz(using x: Int)(using y: String): + override def toString = "Baz" +object Baz: + given Int = 1 + given String = "x" + inline def foo = + println(new Baz()) + println(Baz()) diff --git a/tests/run/backwardsCompat-implicitParens/Test_2.scala b/tests/run/backwardsCompat-implicitParens/Test_2.scala index ae2acba2835b..b22b5b411b0c 100644 --- a/tests/run/backwardsCompat-implicitParens/Test_2.scala +++ b/tests/run/backwardsCompat-implicitParens/Test_2.scala @@ -1,5 +1,19 @@ @main def Test = given Int = 1 + given String = "x" + println(new Bar("")) println(Bar("")) println(Bar.foo) + + println(new Bat()) + println(Bat()) + println(Bat.foo) + + println(new Bax()) + println(Bax()) + println(Bax.foo) + + println(new Baz()) + println(Baz()) + println(Baz.foo) From 0ab1bb639312f4046151c5eb9a8a2686ed3bf7bd Mon Sep 17 00:00:00 2001 From: odersky Date: Fri, 15 Apr 2022 11:21:04 +0200 Subject: [PATCH 7/7] Apply suggestions from code review --- compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index 15cfc656f222..1cff4dfe4b8b 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -1557,11 +1557,11 @@ object TreeUnpickler { class TreeWithoutOwner extends Exception /** An attachment key indicating that an old-style leading () in a constructor - * call that has otherwise only using clauses was suppressed. + * call that is followed by a using clause was suppressed. */ val SuppressedApplyToNone: Property.Key[Unit] = Property.Key() - /** An attachment key indicating that an trailing () in a constructor + /** An attachment key indicating that a trailing () in a constructor * call that has otherwise only using clauses was inserted. */ val InsertedApplyToNone: Property.Key[Unit] = Property.Key()