-
Notifications
You must be signed in to change notification settings - Fork 13k
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
macros 1.1: future proofing and cleanup #37198
Conversation
cc @nrc |
f870188
to
aac6dca
Compare
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.
Looks good to me but I'm not 100% familiar with all the changes, so
r? @nrc
|
||
#![feature(proc_macro)] | ||
|
||
#[macro_use] | ||
extern crate derive_a; | ||
#[macro_use] | ||
extern crate derive_a_2; //~ ERROR: cannot shadow existing derive mode `A` | ||
extern crate derive_a; //~ ERROR `derive_a` has already been defined |
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.
Hm isn't this testing something else than before? Before it was preventing two #[proc_macro_derive(A)]
was an error, but here it's erroring out on the extern crate
annotations?
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.
Yeah, it used to test that a #[macro_use]
-imported custom derive cannot shadow an existing #[macro_use]
-imported custom derive.
This PR changes #[macro_use]
-imported custom derives to have the same scope and shadowing restrictions as other #[macro_use]
-imported macros, so it is no longer an error for an unexpanded #[macro_use]
to import a shadowing custom derive (just as it is not an error today for ordinary unexpanded #[macro_use]
s to shadow).
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.
(c.f. the second bullet point in the initial comment)
@bors: r+ |
📌 Commit aac6dca has been approved by |
…=nrc macros 1.1: future proofing and cleanup This PR - uses the macro namespace for custom derives (instead of a dedicated custom derive namespace), - relaxes the shadowing rules for `#[macro_use]`-imported custom derives to match the shadowing rules for ordinary `#[macro_use]`-imported macros, and - treats custom derive `extern crate`s like empty modules so that we can eventually allow, for example, `extern crate serde_derive; use serde_derive::Serialize;` backwards compatibly. r? @alexcrichton
…=nrc macros 1.1: future proofing and cleanup This PR - uses the macro namespace for custom derives (instead of a dedicated custom derive namespace), - relaxes the shadowing rules for `#[macro_use]`-imported custom derives to match the shadowing rules for ordinary `#[macro_use]`-imported macros, and - treats custom derive `extern crate`s like empty modules so that we can eventually allow, for example, `extern crate serde_derive; use serde_derive::Serialize;` backwards compatibly. r? @alexcrichton
This PR
#[macro_use]
-imported custom derives to match the shadowing rules for ordinary#[macro_use]
-imported macros, andextern crate
s like empty modules so that we can eventually allow, for example,extern crate serde_derive; use serde_derive::Serialize;
backwards compatibly.r? @alexcrichton