Merge pull request #420 from scala/backport-lts-3.3-23178

Backport "Avoid creating constructors where not warranted" to 3.3 LTS

Author: tgodzik

compiler/src/dotty/tools/backend/jvm/Primitives.scala

@@ -150,13 +150,12 @@ object Primitives {
   case object XOR extends LogicalOp
 
   /** Signals the beginning of a series of concatenations.
-    *  On the JVM platform, it should create a new StringBuffer
-    */
+   *  On the JVM platform, it should create a new StringBuilder.
+   */
   case object StartConcat extends Primitive
 
-  /**
-   * type: (buf) => STR
-   * jvm : It should turn the StringBuffer into a String.
+  /** type: (buf) => STR
+   *  jvm : It should turn the StringBuilder into a String.
    */
   case object EndConcat extends Primitive
 

compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala

@@ -194,27 +194,21 @@ class ClassfileParser(
      *  Updates the read pointer of 'in'. */
     def parseParents: List[Type] = {
       val superType =
-        if (classRoot.symbol == defn.ComparableClass ||
-                 classRoot.symbol == defn.JavaCloneableClass ||
-                 classRoot.symbol == defn.JavaSerializableClass) {
-          // Treat these interfaces as universal traits
-          in.nextChar
+        val superClass = in.nextChar
+        // Treat these interfaces as universal traits
+        if classRoot.symbol == defn.ComparableClass
+        || classRoot.symbol == defn.JavaCloneableClass
+        || classRoot.symbol == defn.JavaSerializableClass
+        then
           defn.AnyType
-        }
         else
-          pool.getSuperClass(in.nextChar).typeRef
+          pool.getSuperClass(superClass).typeRef
       val ifaceCount = in.nextChar
-      var ifaces = for (i <- (0 until ifaceCount).toList) yield pool.getSuperClass(in.nextChar).typeRef
-        // Dotty deviation: was
-        //    var ifaces = for (i <- List.range(0, ifaceCount)) ...
-        // This does not typecheck because the type parameter of List is now lower-bounded by Int | Char.
-        // Consequently, no best implicit for the "Integral" evidence parameter of "range"
-        // is found. Previously, this worked because of weak conformance, which has been dropped.
-
+      val ifaces = List.fill(ifaceCount.toInt):
+        pool.getSuperClass(in.nextChar).typeRef
       superType :: ifaces
     }
 
-
     val result = unpickleOrParseInnerClasses()
     if (!result.isDefined) {
       var classInfo: Type = TempClassInfoType(parseParents, instanceScope, classRoot.symbol)
@@ -233,8 +227,8 @@ class ClassfileParser(
         moduleRoot.setPrivateWithin(privateWithin)
         moduleRoot.sourceModule.setPrivateWithin(privateWithin)
 
-      for (i <- 0 until in.nextChar) parseMember(method = false)
-      for (i <- 0 until in.nextChar) parseMember(method = true)
+      for (_ <- 0 until in.nextChar) parseMember(method = false)
+      for (_ <- 0 until in.nextChar) parseMember(method = true)
 
       classRoot.registerCompanion(moduleRoot.symbol)
       moduleRoot.registerCompanion(classRoot.symbol)

compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala

@@ -64,18 +64,22 @@ object Scala2Unpickler {
     denot.info = PolyType.fromParams(denot.owner.typeParams, denot.info)
   }
 
-  def ensureConstructor(cls: ClassSymbol, clsDenot: ClassDenotation, scope: Scope)(using Context): Unit = {
-    if (scope.lookup(nme.CONSTRUCTOR) == NoSymbol) {
-      val constr = newDefaultConstructor(cls)
+  def ensureConstructor(cls: ClassSymbol, clsDenot: ClassDenotation, scope: Scope)(using Context): Unit =
+    doEnsureConstructor(cls, clsDenot, scope, fromScala2 = true)
+
+  private def doEnsureConstructor(cls: ClassSymbol, clsDenot: ClassDenotation, scope: Scope, fromScala2: Boolean)
+      (using Context): Unit =
+    if scope.lookup(nme.CONSTRUCTOR) == NoSymbol then
+      val constr =
+        if fromScala2 || cls.isAllOf(Trait | JavaDefined) then newDefaultConstructor(cls)
+        else newConstructor(cls, Private, paramNames = Nil, paramTypes = Nil)
       // Scala 2 traits have a constructor iff they have initialization code
       // In dotc we represent that as !StableRealizable, which is also owner.is(NoInits)
       if clsDenot.flagsUNSAFE.is(Trait) then
         constr.setFlag(StableRealizable)
         clsDenot.setFlag(NoInits)
       addConstructorTypeParams(constr)
       cls.enter(constr, scope)
-    }
-  }
 
   def setClassInfo(denot: ClassDenotation, info: Type, fromScala2: Boolean, selfInfo: Type = NoType)(using Context): Unit = {
     val cls = denot.classSymbol
@@ -109,7 +113,7 @@ object Scala2Unpickler {
       if (tsym.exists) tsym.setFlag(TypeParam)
       else denot.enter(tparam, decls)
     }
-    if (!denot.flagsUNSAFE.isAllOf(JavaModule)) ensureConstructor(cls, denot, decls)
+    if (!denot.flagsUNSAFE.isAllOf(JavaModule)) doEnsureConstructor(cls, denot, decls, fromScala2)
 
     val scalacCompanion = denot.classSymbol.scalacLinkedClass
 

compiler/src/dotty/tools/dotc/printing/Formatting.scala

@@ -129,6 +129,7 @@ object Formatting {
       given Show[Class[?]]                            = ShowAny
       given Show[Throwable]                           = ShowAny
       given Show[StringBuffer]                        = ShowAny
+      given Show[StringBuilder]                       = ShowAny
       given Show[CompilationUnit]                     = ShowAny
       given Show[Phases.Phase]                        = ShowAny
       given Show[TyperState]                          = ShowAny

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -3098,18 +3098,18 @@ extends ReferenceMsg(CannotBeAccessedID):
       case _ =>
         i"none of the overloaded alternatives named $name can"
     val where = if (ctx.owner.exists) i" from ${ctx.owner.enclosingClass}" else ""
-    val whyNot = new StringBuffer
+    val whyNot = new StringBuilder
     for alt <- alts do
       val cls = alt.owner.enclosingSubClass
       val owner = if cls.exists then cls else alt.owner
       val location: String =
         if alt.is(Protected) then
           if alt.privateWithin.exists && alt.privateWithin != owner then
             if owner.is(Final) then alt.privateWithin.showLocated
-            else alt.privateWithin.showLocated + ", or " + owner.showLocated + " or one of its subclasses"
+            else s"${alt.privateWithin.showLocated}, or ${owner.showLocated} or one of its subclasses"
           else
             if owner.is(Final) then owner.showLocated
-            else owner.showLocated + " or one of its subclasses"
+            else s"${owner.showLocated} or one of its subclasses"
         else
           alt.privateWithin.orElse(owner).showLocated
       val accessMod = if alt.is(Protected) then "protected" else "private"

tests/neg/i15144.scala

@@ -0,0 +1,16 @@
+//> using options --release 11
+// test: -jvm 17+
+// Must be tested where release target < current JVM.
+// Maybe a bug that ct.sym is not used if release == JVM version.
+
+import java.time.Instant
+
+class C:
+  def f: Instant = new Instant // error
+  def g: Instant = Instant() // error
+  def p: P = new P // error
+
+class P private ()
+
+@main def test() = println:
+  C().f