Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enter missing symbols generated by the MacroAnnotation expansion #18826

Merged
merged 1 commit into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/Inlining.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.*

Expand Down Expand Up @@ -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
Expand Down
32 changes: 26 additions & 6 deletions compiler/src/dotty/tools/dotc/transform/MacroAnnotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.*
Expand All @@ -23,7 +23,8 @@ import scala.util.control.NonFatal

import java.lang.reflect.InvocationTargetException

class MacroAnnotations:
class MacroAnnotations(phase: IdentityDenotTransformer):

import tpd.*
import MacroAnnotations.*

Expand Down Expand Up @@ -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)
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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 {
hamzaremmal marked this conversation as resolved.
Show resolved Hide resolved
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` */
Expand Down
3 changes: 3 additions & 0 deletions tests/neg-macros/i18825.check
Original file line number Diff line number Diff line change
@@ -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
19 changes: 19 additions & 0 deletions tests/neg-macros/i18825/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions tests/neg-macros/i18825/Test_2.scala
Original file line number Diff line number Diff line change
@@ -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)
23 changes: 23 additions & 0 deletions tests/neg-macros/wrong-owner.check
Original file line number Diff line number Diff line change
@@ -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 <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`
| |
19 changes: 19 additions & 0 deletions tests/neg-macros/wrong-owner/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions tests/neg-macros/wrong-owner/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import scala.annotation.experimental

@experimental
@wrongOwner
class Foo // error
1 change: 1 addition & 0 deletions tests/run-macros/i18806.check
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
hi
24 changes: 24 additions & 0 deletions tests/run-macros/i18806/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -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)
14 changes: 14 additions & 0 deletions tests/run-macros/i18806/Test_2.scala
Original file line number Diff line number Diff line change
@@ -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())
2 changes: 1 addition & 1 deletion tests/run/quotes-add-erased/Macro_1.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading