better error when multiple parameter lists with unroll

scala · Oct 3, 2024 · fba2445 · fba2445
1 parent 1f98384
commit fba2445
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 26 deletions.
diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/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"
@@ -133,18 +134,21 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
         else
           local
       seenMethods.getOrElseUpdate(method, {
-        var res = true
-        if method.is(Deferred) then
-          report.error("Unrolled method must be final and concrete", method.srcPos)
-          res = false
-        val isCtor = method.isConstructor
-        if isCtor && method.owner.is(Trait) then
-          report.error("implementation restriction: Unrolled method cannot be a trait constructor", method.srcPos)
-          res = false
-        if !(isCtor || method.is(Final) || method.owner.is(ModuleClass)) then
-          report.error("Unrolled method must be final", method.srcPos)
-          res = false
-        res
+        if method.name.is(DefaultGetterName) then
+          false // not an error, but not an expandable unrolled method
+        else
+          var res = true
+          if method.is(Deferred) then
+            report.error("Unrolled method must be final and concrete", method.srcPos)
+            res = false
+          val isCtor = method.isConstructor
+          if isCtor && method.owner.is(Trait) then
+            report.error("implementation restriction: Unrolled method cannot be a trait constructor", method.srcPos)
+            res = false
+          if !(isCtor || method.is(Final) || method.owner.is(ModuleClass)) then
+            report.error(s"Unrolled method ${method} must be final", method.srcPos)
+            res = false
+          res
       })
 
     def withNoCheckNews[T](ts: List[New])(op: => T): T = {

diff --git a/compiler/src/dotty/tools/dotc/transform/UnrollDefinitions.scala b/compiler/src/dotty/tools/dotc/transform/UnrollDefinitions.scala
@@ -65,18 +65,21 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
 
     def computeIndices(annotated: Symbol)(using Context): ComputedIndicies =
       unrolledDefs.getOrElseUpdate(annotated, {
-        val indices = annotated
-          .paramSymss
-          .zipWithIndex
-          .flatMap: (paramClause, paramClauseIndex) =>
-            val annotationIndices = findUnrollAnnotations(paramClause)
-            if (annotationIndices.isEmpty) None
-            else Some((paramClauseIndex, annotationIndices))
-        if indices.nonEmpty then
-          // pre-validation should have occurred in posttyper
-          assert(annotated.is(Final, butNot = Deferred) || annotated.isConstructor || annotated.owner.is(ModuleClass),
-            i"$annotated is not final&concrete, or a constructor")
-        indices
+        if annotated.name.is(DefaultGetterName) then
+          Nil // happens in curried methods where more than one parameter list has @unroll
+        else
+          val indices = annotated
+            .paramSymss
+            .zipWithIndex
+            .flatMap: (paramClause, paramClauseIndex) =>
+              val annotationIndices = findUnrollAnnotations(paramClause)
+              if (annotationIndices.isEmpty) None
+              else Some((paramClauseIndex, annotationIndices))
+          if indices.nonEmpty then
+            // pre-validation should have occurred in posttyper
+            assert(annotated.is(Final, butNot = Deferred) || annotated.isConstructor || annotated.owner.is(ModuleClass) || annotated.name.is(DefaultGetterName),
+              i"$annotated is not final&concrete, or a constructor")
+          indices
       })
     end computeIndices
 
@@ -103,7 +106,7 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
     params
       .zipWithIndex
       .collect {
-        case (v, i) if v.annotations.exists(_.symbol.fullName.toString == "scala.annotation.unroll") =>
+        case (v, i) if v.hasAnnotation(defn.UnrollAnnot) =>
           i
       }
   }
@@ -303,7 +306,9 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
                 case _ => unreachable("sliding with at least 2 elements")
             Some((defdef.symbol, None, generatedDefs))
 
-        case multiple => sys.error("Cannot have multiple parameter lists containing `@unroll` annotation")
+        case multiple =>
+          report.error("Cannot have multiple parameter lists containing `@unroll` annotation", defdef.srcPos)
+          None
       }
 
     case _ => None

diff --git a/tests/neg/unroll-multipleParams.check b/tests/neg/unroll-multipleParams.check
@@ -0,0 +1,4 @@
+-- Error: tests/neg/unroll-multipleParams.scala:6:12 -------------------------------------------------------------------
+6 |  final def foo(x: Int, @unroll y: Int = 0)(@unroll z: Int = 0) = x + y + z // error
+  |  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  |  Cannot have multiple parameter lists containing `@unroll` annotation
diff --git a/tests/neg/unroll-multipleParams.scala b/tests/neg/unroll-multipleParams.scala
@@ -0,0 +1,7 @@
+//> using options -experimental
+
+import scala.annotation.unroll
+
+class Foo {
+  final def foo(x: Int, @unroll y: Int = 0)(@unroll z: Int = 0) = x + y + z // error
+}