From 038b51e99d2068f4ef17864148619dcf047244b5 Mon Sep 17 00:00:00 2001 From: Hamza Remmal <56235032+hamzaremmal@users.noreply.github.com> Date: Thu, 2 Nov 2023 13:54:57 +0100 Subject: [PATCH] Enter missing symbols in MacroAnnotations --- .../dotty/tools/dotc/transform/Inlining.scala | 5 +-- .../dotc/transform/MacroAnnotations.scala | 32 +++++++++++++++---- tests/neg-macros/i18825.check | 3 ++ tests/neg-macros/i18825/Macro_1.scala | 19 +++++++++++ tests/neg-macros/i18825/Test_2.scala | 15 +++++++++ tests/neg-macros/wrong-owner.check | 23 +++++++++++++ tests/neg-macros/wrong-owner/Macro_1.scala | 19 +++++++++++ tests/neg-macros/wrong-owner/Test_2.scala | 5 +++ tests/run-macros/i18806.check | 1 + tests/run-macros/i18806/Macro_1.scala | 24 ++++++++++++++ tests/run-macros/i18806/Test_2.scala | 14 ++++++++ tests/run/quotes-add-erased/Macro_1.scala | 2 +- 12 files changed, 153 insertions(+), 9 deletions(-) create mode 100644 tests/neg-macros/i18825.check create mode 100644 tests/neg-macros/i18825/Macro_1.scala create mode 100644 tests/neg-macros/i18825/Test_2.scala create mode 100644 tests/neg-macros/wrong-owner.check create mode 100644 tests/neg-macros/wrong-owner/Macro_1.scala create mode 100644 tests/neg-macros/wrong-owner/Test_2.scala create mode 100644 tests/run-macros/i18806.check create mode 100644 tests/run-macros/i18806/Macro_1.scala create mode 100644 tests/run-macros/i18806/Test_2.scala diff --git a/compiler/src/dotty/tools/dotc/transform/Inlining.scala b/compiler/src/dotty/tools/dotc/transform/Inlining.scala index bfc44f868cb6..a51ba93ab9ac 100644 --- a/compiler/src/dotty/tools/dotc/transform/Inlining.scala +++ b/compiler/src/dotty/tools/dotc/transform/Inlining.scala @@ -17,7 +17,8 @@ import dotty.tools.dotc.staging.StagingLevel import scala.collection.mutable.ListBuffer /** Inlines all calls to inline methods that are not in an inline method or a quote */ -class Inlining extends MacroTransform { +class Inlining extends MacroTransform, IdentityDenotTransformer { + self => import tpd.* @@ -75,7 +76,7 @@ class Inlining extends MacroTransform { && StagingLevel.level == 0 && MacroAnnotations.hasMacroAnnotation(tree.symbol) then - val trees = (new MacroAnnotations).expandAnnotations(tree) + val trees = (new MacroAnnotations(self)).expandAnnotations(tree) val trees1 = trees.map(super.transform) // Find classes added to the top level from a package object diff --git a/compiler/src/dotty/tools/dotc/transform/MacroAnnotations.scala b/compiler/src/dotty/tools/dotc/transform/MacroAnnotations.scala index 40c6eee1382c..dbc1639f4b55 100644 --- a/compiler/src/dotty/tools/dotc/transform/MacroAnnotations.scala +++ b/compiler/src/dotty/tools/dotc/transform/MacroAnnotations.scala @@ -9,7 +9,7 @@ import dotty.tools.dotc.config.Printers.{macroAnnot => debug} import dotty.tools.dotc.core.Annotations.* import dotty.tools.dotc.core.Contexts.* import dotty.tools.dotc.core.Decorators.* -import dotty.tools.dotc.core.DenotTransformers.DenotTransformer +import dotty.tools.dotc.core.DenotTransformers.IdentityDenotTransformer import dotty.tools.dotc.core.Flags.* import dotty.tools.dotc.core.MacroClassLoader import dotty.tools.dotc.core.Symbols.* @@ -23,7 +23,8 @@ import scala.util.control.NonFatal import java.lang.reflect.InvocationTargetException -class MacroAnnotations: +class MacroAnnotations(phase: IdentityDenotTransformer): + import tpd.* import MacroAnnotations.* @@ -58,9 +59,11 @@ class MacroAnnotations: case (prefixed, newTree :: suffixed) => allTrees ++= prefixed insertedAfter = suffixed :: insertedAfter - prefixed.foreach(checkMacroDef(_, tree, annot)) - suffixed.foreach(checkMacroDef(_, tree, annot)) - transform.TreeChecker.checkMacroGeneratedTree(tree, newTree) + for prefixedTree <- prefixed do + checkMacroDef(prefixedTree, tree, annot) + for suffixedTree <- suffixed do + checkMacroDef(suffixedTree, tree, annot) + TreeChecker.checkMacroGeneratedTree(tree, newTree) newTree case (Nil, Nil) => report.error(i"Unexpected `Nil` returned by `(${annot.tree}).transform(..)` during macro expansion", annot.tree.srcPos) @@ -76,6 +79,7 @@ class MacroAnnotations: insertedAfter.foreach(allTrees.++=) val result = allTrees.result() + for tree <- result do enterMissingSymbols(tree) debug.println(result.map(_.show).mkString("expanded to:\n", "\n", "")) result @@ -120,7 +124,7 @@ class MacroAnnotations: /** Check that this tree can be added by the macro annotation */ private def checkMacroDef(newTree: DefTree, annotatedTree: Tree, annot: Annotation)(using Context) = - transform.TreeChecker.checkMacroGeneratedTree(annotatedTree, newTree) + TreeChecker.checkMacroGeneratedTree(annotatedTree, newTree) val sym = newTree.symbol val annotated = annotatedTree.symbol if sym.isType && !sym.isClass then @@ -130,6 +134,22 @@ class MacroAnnotations: else if annotated.isClass && annotated.owner.is(Package) /*&& !sym.isClass*/ then report.error(i"macro annotation can not add top-level ${sym.showKind}. $annot tried to add $sym.", annot.tree) + /** + * Enter the symbols generated by MacroAnnotations + */ + private def enterMissingSymbols(tree: DefTree)(using Context) = new TreeTraverser { + def traverse(tree: tpd.Tree)(using Context): Unit = tree match + case tdef @ TypeDef(_, template: Template) => + val isSymbolInDecls = tdef.symbol.asClass.info.decls.toList.toSet + for tree <- template.body do + if tree.symbol.owner != tdef.symbol then + report.error(em"Macro added a definition with the wrong owner - ${tree.symbol.owner} - ${tdef.symbol} in ${tree.source}", tree.srcPos) + else if !isSymbolInDecls(tree.symbol) then + tree.symbol.enteredAfter(phase) + traverseChildren(tree) + case _ => traverseChildren(tree) + }.traverse(tree) + object MacroAnnotations: /** Is this an annotation that implements `scala.annation.MacroAnnotation` */ diff --git a/tests/neg-macros/i18825.check b/tests/neg-macros/i18825.check new file mode 100644 index 000000000000..0269f9880828 --- /dev/null +++ b/tests/neg-macros/i18825.check @@ -0,0 +1,3 @@ + +error overriding method toString in class Foo of type (): String; + method toString of type (): String cannot override final member method toString in class Foo diff --git a/tests/neg-macros/i18825/Macro_1.scala b/tests/neg-macros/i18825/Macro_1.scala new file mode 100644 index 000000000000..c099954f3858 --- /dev/null +++ b/tests/neg-macros/i18825/Macro_1.scala @@ -0,0 +1,19 @@ +import scala.annotation.experimental +import scala.annotation.MacroAnnotation +import scala.quoted.* + +@experimental +class toString extends MacroAnnotation : + def transform(using Quotes)(tree: quotes.reflect.Definition): List[quotes.reflect.Definition] = + import quotes.reflect.* + tree match + case ClassDef(name, ctr, parents, self, body) => + val cls = tree.symbol + val toStringSym = Symbol.requiredMethod("java.lang.Object.toString") + val toStringOverrideSym = Symbol.newMethod(cls, "toString", toStringSym.info, Flags.Override, Symbol.noSymbol) + val toStringDef = DefDef(toStringOverrideSym, _ => Some(Literal(StringConstant("Hello from macro")))) + val newClassDef = ClassDef.copy(tree)(name, ctr, parents, self, toStringDef :: body) + List(newClassDef) + case _ => + report.error("@toString can only be annotated on class definitions") + tree :: Nil diff --git a/tests/neg-macros/i18825/Test_2.scala b/tests/neg-macros/i18825/Test_2.scala new file mode 100644 index 000000000000..83ae9c778704 --- /dev/null +++ b/tests/neg-macros/i18825/Test_2.scala @@ -0,0 +1,15 @@ +// nopos-error + +import annotation.experimental + +class Foo : + final override def toString(): String = "Hello" + +@experimental +@toString +class AFoo extends Foo //: + //override def toString(): String = "Hello from macro" + +@experimental +@main def run = + println(new AFoo().toString) diff --git a/tests/neg-macros/wrong-owner.check b/tests/neg-macros/wrong-owner.check new file mode 100644 index 000000000000..ccaca98e3948 --- /dev/null +++ b/tests/neg-macros/wrong-owner.check @@ -0,0 +1,23 @@ + +-- Error: tests/neg-macros/wrong-owner/Test_2.scala:5:6 ---------------------------------------------------------------- +3 |@experimental +4 |@wrongOwner +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() { + | override def toString(): java.lang.String = "Hello from macro" + |} + | | + | |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 , ctxOwners = class Foo, class Foo, package , package , package , package , package , package , package , package , package , , , , , + | | + |stacktrace available when compiling with `-Ydebug` + | | diff --git a/tests/neg-macros/wrong-owner/Macro_1.scala b/tests/neg-macros/wrong-owner/Macro_1.scala new file mode 100644 index 000000000000..85127b701f81 --- /dev/null +++ b/tests/neg-macros/wrong-owner/Macro_1.scala @@ -0,0 +1,19 @@ +import scala.annotation.experimental +import scala.annotation.MacroAnnotation +import scala.quoted.* + +@experimental +class wrongOwner extends MacroAnnotation : + def transform(using Quotes)(tree: quotes.reflect.Definition): List[quotes.reflect.Definition] = + import quotes.reflect.* + tree match + case ClassDef(name, ctr, parents, self, body) => + val cls = tree.symbol + val toStringSym = Symbol.requiredMethod("java.lang.Object.toString") + val toStringOverrideSym = Symbol.newMethod(Symbol.classSymbol("java.lang.String"), "toString", toStringSym.info, Flags.Override, Symbol.noSymbol) + val toStringDef = DefDef(toStringOverrideSym, _ => Some(Literal(StringConstant("Hello from macro")))) + val newClassDef = ClassDef.copy(tree)(name, ctr, parents, self, toStringDef :: body) + List(newClassDef) + case _ => + report.error("@toString can only be annotated on class definitions") + tree :: Nil diff --git a/tests/neg-macros/wrong-owner/Test_2.scala b/tests/neg-macros/wrong-owner/Test_2.scala new file mode 100644 index 000000000000..ccba69beeb86 --- /dev/null +++ b/tests/neg-macros/wrong-owner/Test_2.scala @@ -0,0 +1,5 @@ +import scala.annotation.experimental + +@experimental +@wrongOwner +class Foo // error diff --git a/tests/run-macros/i18806.check b/tests/run-macros/i18806.check new file mode 100644 index 000000000000..32f95c0d1244 --- /dev/null +++ b/tests/run-macros/i18806.check @@ -0,0 +1 @@ +hi \ No newline at end of file diff --git a/tests/run-macros/i18806/Macro_1.scala b/tests/run-macros/i18806/Macro_1.scala new file mode 100644 index 000000000000..461080b67b95 --- /dev/null +++ b/tests/run-macros/i18806/Macro_1.scala @@ -0,0 +1,24 @@ +import scala.annotation.{experimental, MacroAnnotation} +import scala.quoted._ + +@experimental +class gen1 extends MacroAnnotation: + def transform(using Quotes)(tree: quotes.reflect.Definition): List[quotes.reflect.Definition] = + import quotes.reflect._ + tree match + case ClassDef(name, ctr, parents, self, body) => + val cls = tree.symbol + // val meth = cls.methodMember("foo").head + // val fooTpe = cls.typeRef.memberType(meth) + + val overrideTpe = MethodType(Nil)(_ => Nil, _ => defn.StringClass.typeRef) + + val fooOverrideSym = Symbol.newMethod(cls, "foo", overrideTpe, Flags.Override, Symbol.noSymbol) + + val fooDef = DefDef(fooOverrideSym, _ => Some(Literal(StringConstant("hi")))) + + val newClassDef = ClassDef.copy(tree)(name, ctr, parents, self, fooDef :: body) + List(newClassDef) + case _ => + report.error("Annotation only supports `class`") + List(tree) diff --git a/tests/run-macros/i18806/Test_2.scala b/tests/run-macros/i18806/Test_2.scala new file mode 100644 index 000000000000..3db56c895bdd --- /dev/null +++ b/tests/run-macros/i18806/Test_2.scala @@ -0,0 +1,14 @@ +import scala.annotation.experimental + +class Base: + def foo(): Object = ??? + +@experimental +@gen1 +class Sub extends Base +// > override def foo(): String = "hi" + +@experimental +@main def Test(): Unit = + val sub = new Sub + println(sub.foo()) diff --git a/tests/run/quotes-add-erased/Macro_1.scala b/tests/run/quotes-add-erased/Macro_1.scala index 66f8475da96d..56247d45cd23 100644 --- a/tests/run/quotes-add-erased/Macro_1.scala +++ b/tests/run/quotes-add-erased/Macro_1.scala @@ -15,7 +15,7 @@ class erasedParamsMethod extends MacroAnnotation: assert(methType.hasErasedParams) assert(methType.erasedParams == List(true, false)) - val methSym = Symbol.newMethod(tree.symbol, "takesErased", methType, Flags.EmptyFlags, Symbol.noSymbol) + val methSym = Symbol.newMethod(tree.symbol, "takesErased", methType, Flags.Override, Symbol.noSymbol) val methDef = DefDef(methSym, _ => Some(Literal(IntConstant(1)))) val clsDef = ClassDef.copy(tree)(name, ctr, parents, self, methDef :: body)