include constructor in unroll clash, loosen final check for objects

Author: bishabosha

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

@@ -137,7 +137,7 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
         if method.is(Deferred) then
           report.error("Unrolled method must be final and concrete", method.srcPos)
           res = false
-        if !method.isConstructor && !method.is(Final) then
+        if !(method.isConstructor || method.is(Final) || method.owner.is(ModuleClass)) then
           report.error("Unrolled method must be final", method.srcPos)
           res = false
         res

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

@@ -25,6 +25,8 @@ import dotty.tools.unreachable
 
 /**Implementation of SIP-61.
  * Runs when `@unroll` annotations are found in a compilation unit, installing new definitions
+ *
+ * Note that it only generates `Invisible` methods, so no interactions with Zinc/SemanticDB
  */
 class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
   self =>
@@ -63,17 +65,18 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
 
     def computeIndices(annotated: Symbol)(using Context): ComputedIndicies =
       unrolledDefs.getOrElseUpdate(annotated, {
-        annotated
+        val indices = annotated
           .paramSymss
           .zipWithIndex
-          .flatMap { (paramClause, paramClauseIndex) =>
+          .flatMap: (paramClause, paramClauseIndex) =>
             val annotationIndices = findUnrollAnnotations(paramClause)
             if (annotationIndices.isEmpty) None
-            else
-              require(annotated.is(Final, butNot = Deferred) || annotated.isConstructor,
-                i"${annotated} is not final&concrete, or a constructor")
-              Some((paramClauseIndex, annotationIndices))
-          }
+            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
       })
     end computeIndices
 
@@ -320,7 +323,7 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
       decl.matches(other) && !staticNonStaticPair
 
     if allGenerated.nonEmpty then
-      val byName = otherDecls.groupMap(_.symbol.name.toString)(_.symbol)
+      val byName = (tmpl.constr :: otherDecls).groupMap(_.symbol.name.toString)(_.symbol)
       for
         (src, _, dcls) <- allGenerated
         dcl <- dcls

tests/neg/unroll-clash.check

@@ -4,3 +4,9 @@
   |      Unrolled method foo clashes with existing declaration.
   |      Please remove the clashing definition, or the @unroll annotation.
   |      Unrolled from final def foo(x: Int, y: Int): Int in class Foo at line 6
+-- Error: tests/neg/unroll-clash.scala:13:20 ---------------------------------------------------------------------------
+13 |class UnrolledClass2(s: String) { // error
+   |                    ^
+   |              Unrolled constructor UnrolledClass2 clashes with existing declaration.
+   |              Please remove the clashing definition, or the @unroll annotation.
+   |              Unrolled from def <init>(s: String, y: Boolean): UnrolledClass2 in class UnrolledClass2 at line 15

tests/neg/unroll-clash.scala

@@ -9,3 +9,10 @@ class Foo {
     -1
   }
 }
+
+class UnrolledClass2(s: String) { // error
+
+  def this(s: String, @unroll y: Boolean = true) = this(s + y)
+
+  override def toString = s"UnrolledClass2($s)"
+}

tests/run/unroll-inferredFinal.scala

@@ -0,0 +1,17 @@
+//> using options -experimental
+
+import scala.annotation.unroll
+
+object UnrolledObj {
+  // final is not needed because objects can't be extended
+  def foo(s: String, @unroll y: Boolean = true): String = s + y
+}
+
+// final inferred for constructor
+class UnrolledClass(s: String, @unroll y: Boolean = true):
+  override def toString = s"UnrolledClass($s,$y)"
+
+
+@main def Test: Unit =
+  assert(UnrolledObj.foo("foo") == "footrue")
+  assert(new UnrolledClass("foo").toString == "UnrolledClass(foo,true)")