Implement declarative (macro_rules!) derive macros (RFC 3698)
#145208
+966
−240
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-author
joshtriplett
removed
T-rustdoc
|
(Removing T-clippy and T-rustdoc, because the two commits specific to this PR don't touch them.) |
This comment has been minimized.
This comment has been minimized.
joshtriplett
added
the
S-waiting-on-review
|
(Marking as "waiting on review" because I'm seeking help with the test failure here.) |
|
☔ The latest upstream changes (presumably #135846) made this pull request unmergeable. Please resolve the merge conflicts. |
rustbot
added
S-blocked
joshtriplett
force-pushed
the
mbe-derive
branch
from
52710b2 to
17f6d0a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
joshtriplett
force-pushed
the
mbe-derive
branch
from
17f6d0a to
75f119a
Compare
This comment has been minimized.
This comment has been minimized.
…e kind Review everything that uses `MacroKind`, and switch anything that could refer to more than one kind to use `MacroKinds`. Add a new `SyntaxExtensionKind::MacroRules` for `macro_rules!` macros, using the concrete `MacroRulesMacroExpander` type, and have it track which kinds it can handle. Eliminate the separate optional `attr_ext`, now that a `SyntaxExtension` can handle multiple macro kinds. This also avoids the need to downcast when calling methods on `MacroRulesMacroExpander`, such as `get_unused_rule`. Integrate macro kind checking into name resolution's `sub_namespace_match`, so that we only find a macro if it's the right type, and eliminate the special-case hack for attributes.
I discovered this via research through the git log, and I want to leave additional guidance for future macro spelunkers.
This eliminates the case in `failed_to_match_macro` to check for a function-like invocation of a macro with no function-like rules. Instead, macro kind mismatches now result in an unresolved macro, and we detect this case in `unresolved_macro_suggestions`, which now carefully distinguishes between a kind mismatch and other errors. This also handles cases of forward-referenced attributes and cyclic attributes. Expand test coverage to include all of these cases.
The use of `Not` to describe the `!` in `macro_rules!` reads confusingly, and also results in search collisions with the diagnostic structure `MacroRulesNot` elsewhere in the compiler. Rename it to use the more conventional `Bang` for `!`.
This updates two clippy lints which had exceptions for `MacroKind::Bang` macros to extend those exceptions to any macro, now that a macro_rules macro can be any kind of macro.
This makes the minimal fixes necessary for rustdoc to compile and pass existing tests with the switch to `MacroKinds`. It only works for macros that don't actually have multiple kinds, and will panic (with a `todo!`) if it encounters a macro with multiple kinds. rustdoc needs further fixes to handle macros with multiple kinds, and to handle attributes and derive macros that aren't proc macros.
joshtriplett
added
S-waiting-on-review
|
(Marking for review now that 145153 is queued.) |
|
(I'll wait until the rebase anyway.) |
This handles various kinds of errors, but does not allow applying the derive yet. This adds the feature gate `macro_derive`.
Add infrastructure to apply a derive macro to arguments, consuming and returning a `TokenTree` only. Handle `SyntaxExtensionKind::MacroRules` when expanding a derive, if the macro's kinds support derive. Add tests covering various cases of `macro_rules` derives.
joshtriplett
force-pushed
the
mbe-derive
branch
from
75f119a to
9e6649e
Compare
|
@petrochenkov Rebased atop the latest version of 145153. (All but the last two commits will go away once 145153 is merged.) |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
I meant after #145153 is merged, the github interface makes reviewing stacked PRs quite inconvenient, so I basically never look at them. |
petrochenkov
added
S-blocked
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a draft for review, and should not be merged yet.
This is layered atop #145153 , and has
only two additional commits atop that. The first handles parsing and provides a
test for various parse errors. The second implements expansion and handles
application.
This implements RFC 3698, "Declarative (
macro_rules!) derive macros".Tracking issue: #143549
This has one remaining issue, which I could use some help debugging: in
tests/ui/macros/macro-rules-derive-error.rs, the diagnostics forderive(fn_only)(for afn_onlywith noderiverules) andderive(ForwardReferencedDerive)both get emitted twice, as a duplicatediagnostic.
From what I can tell via adding some debugging code,
unresolved_macro_suggestionsis getting called twice fromfinalize_macro_resolutionsfor each of them, becauseself.single_segment_macro_resolutionshas two entries for the macro, with twodifferent
parent_scopevalues. I'm not clear on why that happened; it doesn'thappen with the equivalent code using attrs.
I'd welcome any suggestions for fixing this.