-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Conversation
There is a failure in |
The failure was
I assume that we simply missed the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also squash and rebase the commit on main.
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}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should have a test that produces this error. This test should have a checkfile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this check anymore. When running the wrong-owner
test, I have the following result in testlog
-- Error: tests/neg-macros/wrong-owner/Test_2.scala:7:6 ----------------------------------------------------------------
5 |@experimental
6 |@wrongOwner
7 |class Foo
|^
|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
| |
Macro added a definition with the wrong owner - class String - class Foo in tests/neg-macros/wrong-owner/Test_2.scala
2 errors found
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. The issue was that tests under the /tests/neg-macros
run with the -Xcheck-macros
which itself trigger the second error. We agreed with @nicolasstucki to keep both error messages in the sense that if the flag is enabled, the more developed error message will be displayed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, maybe we should add this information in the new error message. e.g "More information available under with the -Xcheck-macros
"
fda646c
to
27dbeb3
Compare
How I fixed it :
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@hamzaremmal could you squash your commits into one?
@nicolasstucki I've squashed the commits into one with all the necessary changes |
Closes #18825
Closes #18806