SIP 61 - Improve checks and type substitution for forwarders

- add documentation page
- move before pickling, fix errors due to unpickling select of Invisible definitions and incorrect spans
- forwarders now only call the original method.
- detect in posttyper which compilation units have unrolled definitions
- detect clashes with forwarders and existing definitions
- check for illegal uses of @unroll
- implementation restriction: no unroll for trait parameters
- unlink replaced definitions
- check for default parameter
- fix invalid pattern in generateFromProduct
- sbt-test/scripted: fork when running with unmanaged classpaths
- update stdlibExperimentalDefinitions.scala
- skip reflection test on scala.js
- require final methods, remove special treatment of abstract methods
- fix order of printing in test
- better error when multiple parameter lists with unroll
- Move sbt-scripted tests to vulpix suites

Author: bishabosha

compiler/src/dotty/tools/dotc/CompilationUnit.scala

@@ -59,6 +59,9 @@ class CompilationUnit protected (val source: SourceFile, val info: CompilationUn
 
   var hasMacroAnnotations: Boolean = false
 
+  def hasUnrollDefs: Boolean = unrolledClasses != null
+  var unrolledClasses: Set[Symbol] | Null = null
+
   /** Set to `true` if inliner added anonymous mirrors that need to be completed */
   var needsMirrorSupport: Boolean = false
 

compiler/src/dotty/tools/dotc/Compiler.scala

@@ -40,6 +40,7 @@ class Compiler {
     List(new sbt.ExtractDependencies) :: // Sends information on classes' dependencies to sbt via callbacks
     List(new semanticdb.ExtractSemanticDB.ExtractSemanticInfo) :: // Extract info into .semanticdb files
     List(new PostTyper) ::          // Additional checks and cleanups after type checking
+    List(new UnrollDefinitions) ::  // Unroll annotated methods if detected in PostTyper
     List(new sjs.PrepJSInterop) ::  // Additional checks and transformations for Scala.js (Scala.js only)
     List(new SetRootTree) ::        // Set the `rootTreeOrProvider` on class symbols
     Nil
@@ -61,7 +62,6 @@ class Compiler {
     List(new InstrumentCoverage) ::  // Perform instrumentation for code coverage (if -coverage-out is set)
     List(new CrossVersionChecks,     // Check issues related to deprecated and experimental
          new FirstTransform,         // Some transformations to put trees into a canonical form
-         new UnrollDefs,             // Unroll annotated methods
          new CheckReentrant,         // Internal use only: Check that compiled program has no data races involving global vars
          new ElimPackagePrefixes,    // Eliminate references to package prefixes in Select nodes
          new CookComments,           // Cook the comments: expand variables, doc, etc.

compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala

@@ -20,6 +20,7 @@ import collection.mutable
 import reporting.{Profile, NoProfile}
 import dotty.tools.tasty.TastyFormat.ASTsSection
 import quoted.QuotePatterns
+import dotty.tools.dotc.config.Feature
 
 object TreePickler:
   class StackSizeExceeded(val mdef: tpd.MemberDef) extends Exception
@@ -474,15 +475,16 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) {
             case _ =>
               if passesConditionForErroringBestEffortCode(tree.hasType) then
                 // #19951 The signature of a constructor of a Java annotation is irrelevant
+                val sym = tree.symbol
                 val sig =
-                  if name == nme.CONSTRUCTOR && tree.symbol.exists && tree.symbol.owner.is(JavaAnnotation) then Signature.NotAMethod
+                  if name == nme.CONSTRUCTOR && sym.exists && sym.owner.is(JavaAnnotation) then Signature.NotAMethod
                   else tree.tpe.signature
-                var ename = tree.symbol.targetName
+                var ename = sym.targetName
                 val selectFromQualifier =
                   name.isTypeName
                   || qual.isInstanceOf[Hole] // holes have no symbol
                   || sig == Signature.NotAMethod // no overload resolution necessary
-                  || !tree.denot.symbol.exists // polymorphic function type
+                  || !sym.exists // polymorphic function type
                   || tree.denot.asSingleDenotation.isRefinedMethod // refined methods have no defining class symbol
                 if selectFromQualifier then
                   writeByte(if name.isTypeName then SELECTtpt else SELECT)
@@ -491,9 +493,9 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) {
                 else // select from owner
                   writeByte(SELECTin)
                   withLength {
-                    pickleNameAndSig(name, tree.symbol.signature, ename)
+                    pickleNameAndSig(name, sym.signature, ename)
                     pickleTree(qual)
-                    pickleType(tree.symbol.owner.typeRef)
+                    pickleType(sym.owner.typeRef)
                   }
               else
                 writeByte(if name.isTypeName then SELECTtpt else SELECT)

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

@@ -220,6 +220,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case DeprecatedInfixNamedArgumentSyntaxID // errorNumber: 204
   case GivenSearchPriorityID // errorNumber: 205
   case EnumMayNotBeValueClassesID // errorNumber: 206
+  case IllegalUnrollPlacementID // errorNumber: 207
 
   def errorNumber = ordinal - 1
 

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

@@ -3334,14 +3334,14 @@ final class QuotedTypeMissing(tpe: Type)(using Context) extends StagingMessage(Q
 
   private def witness = defn.QuotedTypeClass.typeRef.appliedTo(tpe)
 
-  override protected def msg(using Context): String = 
+  override protected def msg(using Context): String =
     i"Reference to $tpe within quotes requires a given ${witness} in scope"
 
   override protected def explain(using Context): String =
-    i"""Referencing `$tpe` inside a quoted expression requires a `${witness}` to be in scope. 
+    i"""Referencing `$tpe` inside a quoted expression requires a `${witness}` to be in scope.
         |Since Scala is subject to erasure at runtime, the type information will be missing during the execution of the code.
-        |`${witness}` is therefore needed to carry `$tpe`'s type information into the quoted code. 
-        |Without an implicit `${witness}`, the type `$tpe` cannot be properly referenced within the expression. 
+        |`${witness}` is therefore needed to carry `$tpe`'s type information into the quoted code.
+        |Without an implicit `${witness}`, the type `$tpe` cannot be properly referenced within the expression.
         |To resolve this, ensure that a `${witness}` is available, either through a context-bound or explicitly.
         |"""
 
@@ -3408,3 +3408,26 @@ final class EnumMayNotBeValueClasses(sym: Symbol)(using Context) extends SyntaxM
 
     def explain(using Context) = ""
 end EnumMayNotBeValueClasses
+
+class IllegalUnrollPlacement(origin: Option[Symbol])(using Context)
+extends DeclarationMsg(IllegalUnrollPlacementID):
+  def msg(using Context) = origin match
+    case None => "@unroll is only allowed on a method parameter"
+    case Some(method) =>
+      val isCtor = method.isConstructor
+      def what = if isCtor then i"a ${if method.owner.is(Trait) then "trait" else "class"} constructor" else i"method ${method.name}"
+      val prefix = s"Can not unroll parameters of $what"
+      if method.is(Deferred) then
+        i"$prefix: it must not be abstract"
+      else if isCtor && method.owner.is(Trait) then
+        i"implementation restriction: $prefix"
+      else if !(isCtor || method.is(Final) || method.owner.is(ModuleClass)) then
+        i"$prefix: it is not final"
+      else if method.owner.companionClass.is(CaseClass) then
+        i"$prefix of a case class companion object: please annotate the class constructor instead"
+      else
+        assert(method.owner.is(CaseClass))
+        i"$prefix of a case class: please annotate the class constructor instead"
+
+  def explain(using Context) = ""
+end IllegalUnrollPlacement

compiler/src/dotty/tools/dotc/transform/PostTyper.scala

@@ -21,6 +21,7 @@ import reporting.*
 import NameKinds.WildcardParamName
 import cc.*
 import dotty.tools.dotc.transform.MacroAnnotations.hasMacroAnnotation
+import dotty.tools.dotc.core.NameKinds.DefaultGetterName
 
 object PostTyper {
   val name: String = "posttyper"
@@ -119,8 +120,39 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
 
     private var inJavaAnnot: Boolean = false
 
+    private var seenUnrolledMethods: util.EqHashMap[Symbol, Boolean] | Null = null
+
     private var noCheckNews: Set[New] = Set()
 
+    def isValidUnrolledMethod(method: Symbol, origin: SrcPos)(using Context): Boolean =
+      val seenMethods =
+        val local = seenUnrolledMethods
+        if local == null then
+          val map = new util.EqHashMap[Symbol, Boolean]
+          seenUnrolledMethods = map
+          map
+        else
+          local
+      seenMethods.getOrElseUpdate(method, {
+        val isCtor = method.isConstructor
+        if
+          method.name.is(DefaultGetterName)
+        then
+          false // not an error, but not an expandable unrolled method
+        else if
+          method.is(Deferred)
+          || isCtor && method.owner.is(Trait)
+          || !(isCtor || method.is(Final) || method.owner.is(ModuleClass))
+          || method.owner.companionClass.is(CaseClass)
+            && (method.name == nme.apply || method.name == nme.fromProduct)
+          || method.owner.is(CaseClass) && method.name == nme.copy
+        then
+          report.error(IllegalUnrollPlacement(Some(method)), origin)
+          false
+        else
+          true
+      })
+
     def withNoCheckNews[T](ts: List[New])(op: => T): T = {
       val saved = noCheckNews
       noCheckNews ++= ts
@@ -199,6 +231,16 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
       tree
     }
 
+    private def registerIfUnrolledParam(sym: Symbol)(using Context): Unit =
+      if sym.hasAnnotation(defn.UnrollAnnot) && isValidUnrolledMethod(sym.owner, sym.sourcePos) then
+        val cls = sym.enclosingClass
+        val classes = ctx.compilationUnit.unrolledClasses
+        val additions = Array(cls, cls.linkedClass).filter(_ != NoSymbol)
+        if classes == null then
+          ctx.compilationUnit.unrolledClasses = Set.from(additions)
+        else
+          ctx.compilationUnit.unrolledClasses = classes ++ additions
+
     private def processValOrDefDef(tree: Tree)(using Context): tree.type =
       val sym = tree.symbol
       tree match
@@ -215,6 +257,7 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
                     ++ sym.annotations)
           else
             if sym.is(Param) then
+              registerIfUnrolledParam(sym)
               sym.keepAnnotationsCarrying(thisPhase, Set(defn.ParamMetaAnnot), orNoneOf = defn.NonBeanMetaAnnots)
             else if sym.is(ParamAccessor) then
               // @publicInBinary is not a meta-annotation and therefore not kept by `keepAnnotationsCarrying`