Add checks for the consistency of the parents in TreeChecker (#18935)

This PR adds a check for the parents in the TreeChecker. In the context of macro annotations, this PR doesn't allow the modification of the parents. This restriction will probably be partially lifted as we come up with the final specification. This PR is related to #18677 put does not close it.
scala · Feb 12, 2024 · a0ea484 · a0ea484
2 parents 6efcdba + aa5492f
commit a0ea484
Show file tree

Hide file tree

Showing 9 changed files with 134 additions and 14 deletions.
diff --git a/compiler/src/dotty/tools/dotc/transform/Erasure.scala b/compiler/src/dotty/tools/dotc/transform/Erasure.scala
@@ -2,6 +2,7 @@ package dotty.tools
 package dotc
 package transform
 
+import ast.Trees
 import core.Phases.*
 import core.DenotTransformers.*
 import core.Denotations.*
@@ -1044,7 +1045,21 @@ object Erasure {
 
     override def typedClassDef(cdef: untpd.TypeDef, cls: ClassSymbol)(using Context): Tree =
       if cls.is(Flags.Erased) then erasedDef(cls)
-      else super.typedClassDef(cdef, cls)
+      else
+        val typedTree@TypeDef(name, impl @ Template(constr, _, self, _)) = super.typedClassDef(cdef, cls): @unchecked
+        // In the case where a trait extends a class, we need to strip any non trait class from the signature
+        // and accept the first one (see tests/run/mixins.scala)
+        val newTraits = impl.parents.tail.filterConserve: tree =>
+          def isTraitConstructor = tree match
+            case Trees.Block(_, expr) => // Specific management for trait constructors (see tests/pos/i9213.scala)
+              expr.symbol.isConstructor && expr.symbol.owner.is(Flags.Trait)
+            case _ => tree.symbol.isConstructor && tree.symbol.owner.is(Flags.Trait)
+          tree.symbol.is(Flags.Trait) || isTraitConstructor
+
+        val newParents =
+          if impl.parents.tail eq newTraits then impl.parents
+          else impl.parents.head :: newTraits
+        cpy.TypeDef(typedTree)(rhs = cpy.Template(impl)(parents = newParents))
 
     override def typedAnnotated(tree: untpd.Annotated, pt: Type)(using Context): Tree =
       typed(tree.arg, pt)

diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala
@@ -547,8 +547,24 @@ object TreeChecker {
         i"owner chain = ${tree.symbol.ownersIterator.toList}%, %, ctxOwners = ${ctx.outersIterator.map(_.owner).toList}%, %")
     }
 
+    private def checkParents(tree: untpd.TypeDef)(using Context): Unit = {
+      val TypeDef(_, impl: Template) = tree: @unchecked
+      assert(ctx.owner.isClass)
+      val sym = ctx.owner.asClass
+      if !sym.isPrimitiveValueClass then
+        val symbolParents = sym.classInfo.parents.map(_.dealias.typeSymbol)
+        val treeParents = impl.parents.map(_.tpe.dealias.typeSymbol)
+        assert(symbolParents == treeParents,
+        i"""Parents of class symbol differs from the parents in the tree for $sym
+            |
+            |Parents in symbol: $symbolParents
+            |Parents in tree: $treeParents
+            |""".stripMargin)
+    }
+
     override def typedTypeDef(tdef: untpd.TypeDef, sym: Symbol)(using Context): Tree = {
       assert(sym.info.isInstanceOf[ClassInfo | TypeBounds], i"wrong type, expect a template or type bounds for ${sym.fullName}, but found: ${sym.info}")
+      if sym.isClass then checkParents(tdef)
       super.typedTypeDef(tdef, sym)
     }
 
@@ -561,6 +577,8 @@ object TreeChecker {
       checkOwner(impl)
       checkOwner(impl.constr)
 
+      checkParents(cdef)
+
       def isNonMagicalMember(x: Symbol) =
         !x.isValueClassConvertMethod &&
         !x.name.is(DocArtifactName) &&
@@ -812,7 +830,7 @@ object TreeChecker {
             else err.getStackTrace.nn.mkString("  ", "  \n", "")
 
           report.error(
-            s"""Malformed tree was found while expanding macro with -Xcheck-macros.
+            em"""Malformed tree was found while expanding macro with -Xcheck-macros.
                |The tree does not conform to the compiler's tree invariants.
                |
                |Macro was:

diff --git a/tests/neg-macros/i18677-a.check b/tests/neg-macros/i18677-a.check
@@ -0,0 +1,22 @@
+
+-- Error: tests/neg-macros/i18677-a/Test_2.scala:4:6 -------------------------------------------------------------------
+3 |@extendFoo
+4 |class AFoo // error
+  |^
+  |Malformed tree was found while expanding macro with -Xcheck-macros.
+  |The tree does not conform to the compiler's tree invariants.
+  |
+  |Macro was:
+  |@scala.annotation.internal.SourceFile("tests/neg-macros/i18677-a/Test_2.scala") @extendFoo class AFoo()
+  |
+  |The macro returned:
+  |@scala.annotation.internal.SourceFile("tests/neg-macros/i18677-a/Test_2.scala") @extendFoo class AFoo() extends Foo
+  |
+  |Error:
+  |assertion failed: Parents of class symbol differs from the parents in the tree for class AFoo
+  |
+  |Parents in symbol: [class Object]
+  |Parents in tree: [trait Foo]
+  |
+  |
+  |stacktrace available when compiling with `-Ydebug`
diff --git a/tests/neg-macros/i18677-a/Macro_1.scala b/tests/neg-macros/i18677-a/Macro_1.scala
@@ -0,0 +1,18 @@
+//> using -expermiental
+
+import annotation.MacroAnnotation
+import quoted.*
+
+trait Foo
+
+class extendFoo extends MacroAnnotation :
+    override def transform(using Quotes)(tree: quotes.reflect.Definition): List[quotes.reflect.Definition] =
+        import quotes.reflect.*
+        tree match
+            case ClassDef(name, ctr, p, self, body) =>
+                val parents = List(TypeTree.of[Foo])
+                val newTree = ClassDef.copy(tree)(name, ctr,  parents, self, body)
+                newTree :: Nil
+            case _ =>
+                report.error("@extendFoo can only annotate class definitions")
+                tree :: Nil
diff --git a/tests/neg-macros/i18677-a/Test_2.scala b/tests/neg-macros/i18677-a/Test_2.scala
@@ -0,0 +1,4 @@
+//> using -expermiental
+
+@extendFoo
+class AFoo // error
diff --git a/tests/neg-macros/i18677-b.check b/tests/neg-macros/i18677-b.check
@@ -0,0 +1,22 @@
+
+-- Error: tests/neg-macros/i18677-b/Test_2.scala:4:6 -------------------------------------------------------------------
+3 |@extendFoo
+4 |class AFoo // error
+  |^
+  |Malformed tree was found while expanding macro with -Xcheck-macros.
+  |The tree does not conform to the compiler's tree invariants.
+  |
+  |Macro was:
+  |@scala.annotation.internal.SourceFile("tests/neg-macros/i18677-b/Test_2.scala") @extendFoo class AFoo()
+  |
+  |The macro returned:
+  |@scala.annotation.internal.SourceFile("tests/neg-macros/i18677-b/Test_2.scala") @extendFoo class AFoo() extends Foo
+  |
+  |Error:
+  |assertion failed: Parents of class symbol differs from the parents in the tree for class AFoo
+  |
+  |Parents in symbol: [class Object]
+  |Parents in tree: [class Foo]
+  |
+  |
+  |stacktrace available when compiling with `-Ydebug`
diff --git a/tests/neg-macros/i18677-b/Macro_1.scala b/tests/neg-macros/i18677-b/Macro_1.scala
@@ -0,0 +1,18 @@
+//> using -expermiental
+
+import annotation.MacroAnnotation
+import quoted.*
+
+class Foo
+
+class extendFoo extends MacroAnnotation :
+    override def transform(using Quotes)(tree: quotes.reflect.Definition): List[quotes.reflect.Definition] =
+        import quotes.reflect.*
+        tree match
+            case ClassDef(name, ctr, p, self, body) =>
+                val parents = List(TypeTree.of[Foo])
+                val newTree = ClassDef.copy(tree)(name, ctr,  parents, self, body)
+                newTree :: Nil
+            case _ =>
+                report.error("@extendFoo can only annotate class definitions")
+                tree :: Nil
diff --git a/tests/neg-macros/i18677-b/Test_2.scala b/tests/neg-macros/i18677-b/Test_2.scala
@@ -0,0 +1,4 @@
+//> using -expermiental
+
+@extendFoo
+class AFoo // error
diff --git a/tests/neg-macros/wrong-owner.check b/tests/neg-macros/wrong-owner.check
@@ -5,19 +5,18 @@
 5 |class Foo // error
   |^
   |Malformed tree was found while expanding macro with -Xcheck-macros.
-  |               |The tree does not conform to the compiler's tree invariants.
-  |               |
-  |               |Macro was:
-  |               |@scala.annotation.internal.SourceFile("tests/neg-macros/wrong-owner/Test_2.scala") @wrongOwner @scala.annotation.experimental class Foo()
-  |               |
-  |               |The macro returned:
-  |               |@scala.annotation.internal.SourceFile("tests/neg-macros/wrong-owner/Test_2.scala") @wrongOwner @scala.annotation.experimental class Foo() {
+  |The tree does not conform to the compiler's tree invariants.
+  |
+  |Macro was:
+  |@scala.annotation.internal.SourceFile("tests/neg-macros/wrong-owner/Test_2.scala") @wrongOwner @scala.annotation.experimental class Foo()
+  |
+  |The macro returned:
+  |@scala.annotation.internal.SourceFile("tests/neg-macros/wrong-owner/Test_2.scala") @wrongOwner @scala.annotation.experimental class Foo() {
   |  override def toString(): java.lang.String = "Hello from macro"
   |}
-  |               |
-  |               |Error:
-  |               |assertion failed: bad owner; method toString has owner class String, expected was class Foo
+  |
+  |Error:
+  |assertion failed: bad owner; method toString has owner class String, expected was class Foo
   |owner chain = method toString, class String, package java.lang, package java, package <root>, ctxOwners = class Foo, class Foo, package <empty>, package <empty>, package <empty>, package <root>, package <root>, package <root>, package <root>, package <root>, package <root>, <none>, <none>, <none>, <none>, <none>
-  |               |
+  |
   |stacktrace available when compiling with `-Ydebug`
-  |               |