Support macro annotation expansions in -Wmacros:MODE #8799
Conversation
| } | ||
| object checkMacroExpandee extends UnusedPrivates { | ||
| override def traverse(t: Tree): Unit = | ||
| super.traverse(if (hasMacroExpansionAttachment(t)) macroExpandee(t) else t) | ||
| if (!(t.hasSymbol && isExpanded(t.symbol))) | ||
| super.traverse(if (hasMacroExpansionAttachment(t)) macroExpandee(t) else t) |
There was a problem hiding this comment.
I think the condition is flipped? This is the "yes, lint the expandee".
Moving the test to neg and including a Test_3 with -Wmacro:after to show the error would verify.
There was a problem hiding this comment.
I only just glanced at the code comment in StdAttachments about "should be a better way".
There was a problem hiding this comment.
I agree having a "pos" test for -Wmacro:before and a "neg" test for -Wmacro:after is a good idea, but I'm a bit confused by your suggestion. Is the semantics of neg tests that all the files except the last one should compile, and the last one should not compile? So we can write both the pos and the neg parts in the same test?
There was a problem hiding this comment.
Yes, it would compile A_1, then B_2, then error on C_3, and each round can use different flags. If you think that is too confusing or subtle, pos and neg tests is OK.
There was a problem hiding this comment.
Thanks for the clarification. I added a separate neg test as I think that's slightly clearer.
There was a problem hiding this comment.
Not obvious from my first comment, but for "expandee" I always conflate "tree to be expanded" and "result of expansion". I would be less confused by "pre" and "post" or "original" and "expanded", etc.
There was a problem hiding this comment.
I agree, the naming in this area is mighty confusing.
There was a problem hiding this comment.
Some followup to my confusion in #10693 (in case I have to research the nomenclature again in future). The other naming that confuses me still is each instance of UnusedPrivates as it relates to -Wmacros settings. Maybe I only have trouble with names I didn't come up with myself. For an improved default behavior for -Wmacros, I could only come up with default.
Specifically, for `-Wmacros:none` or `-Wmacros:before`, we shouldn't warn about unused symbols in code resulting from expansion of a macro annotation.
lrytz
force-pushed
the
macro-annotations-unused-symbols
branch
from
f167ae6
to
9b505e2
Compare
|
Squashed. @som-snytt feel free to merge if LGTY. |
|
@som-snytt not sure at all what you're talking about here, but see the linked PR for an example of typer-based linting interacting with |
From the compiler options documentation:
I would argue that if we don't warn about unused symbols in trees expanded from def macros, we shouldn't warn about unused symbols in trees expanded from macro annotations either.