Merge pull request #14840 from dotty-staging/fix-2576

Fix Scala Wart about implicit () class parameters

Author: odersky

compiler/src/dotty/tools/dotc/ast/Desugar.scala

@@ -589,12 +589,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 {
@@ -822,7 +826,7 @@ object desugar {
     }
 
     flatTree(cdef1 :: companions ::: implicitWrappers ::: enumScaffolding)
-  }.showing(i"desugared: $result", Printers.desugar)
+  }.showing(i"desugared: $cdef --> $result", Printers.desugar)
 
   /** Expand
    *

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(" ")))
 

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 =

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._
@@ -1163,6 +1163,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 =
@@ -1173,7 +1203,9 @@ 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())
+              if fn.symbol.isConstructor then constructorApply(fn, args)
+              else tpd.Apply(fn, args)
             case TYPEAPPLY =>
               tpd.TypeApply(readTerm(), until(end)(readTpt()))
             case TYPED =>
@@ -1560,4 +1592,14 @@ 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 is followed by a using clause was suppressed.
+   */
+  val SuppressedApplyToNone: Property.Key[Unit] = Property.Key()
+
+  /** 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()
 }

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)

compiler/src/dotty/tools/dotc/typer/Typer.scala

@@ -2381,25 +2381,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]()
 
@@ -3429,7 +3436,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(

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

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)) {}

tests/pos/i2576.scala

@@ -0,0 +1,3 @@
+class Bar(using x: Int)(y: String)
+given Int = ???
+def test = new Bar("")

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("(", ", ", ")")

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
+()

tests/run/backwardsCompat-implicitParens/A_1_c3.0.2.scala

@@ -0,0 +1,32 @@
+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())

tests/run/backwardsCompat-implicitParens/Test_2.scala

@@ -0,0 +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)

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)
 }

tests/semanticdb/metac.expect

@@ -1848,7 +1848,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#`<init>`(). => primary ctor <init> ()(implicit val given param x$1: X): given_Y
+givens/InventedNames$package.given_Y#`<init>`(). => primary ctor <init> (implicit val given param x$1: X)(): given_Y
 givens/InventedNames$package.given_Y#`<init>`().(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