Enforce the shadowing restrictions from RFC 1560 for today's macros #36767
Conversation
jseyfried
force-pushed
the
enforce_rfc_1560_shadowing
branch
from
cc0b377
to
987a536
Compare
jseyfried
force-pushed
the
enforce_rfc_1560_shadowing
branch
from
987a536
to
7f49798
Compare
|
@brson, @nikomatsakis, @eddyb could one of you schedule a crater run please? |
|
@nrc Starting now. |
|
Crater run shows two regressions ( |
|
Code looks good, but I'm not sure about the Crater regressions. They certainly seem real and could indicate more uses in the wild. It is probably prudent to have a warning cycle for this if it is possible. |
jseyfried
force-pushed
the
enforce_rfc_1560_shadowing
branch
from
7f49798
to
3dd5981
Compare
Can it shadow a macro which was previously defined by a macro? I use the above trick in static-cond. If it's not permitted, you can only invoke |
|
@durka Yeah, looks like this PR and unhygienic macro names are a bad combination. I wonder if it would be backwards compatible in practice to make macro names hygienic (EDIT: it isn't). That would avoid breaking If we can't make macro names hygienic, we could instead only disallow macro-expanded shadowing if the shadowing macro is used outside of the expansion. This would also avoid breaking |
jseyfried
force-pushed
the
enforce_rfc_1560_shadowing
branch
from
3dd5981
to
057302b
Compare
|
I weakened the shadowing restrictions to avoid breakage in practice (see the edited initial comment). |
|
Code looks good, I'd like to get another Crater run before we land though. @eddyb, @brson, @nikomatsakis could someone oblige please? |
|
On it! |
|
Crater report looks good (the unrelated breakage is rust-num/num#231, I reused the "before" results). |
|
📌 Commit 057302b has been approved by |
Enforce the shadowing restrictions from RFC 1560 for today's macros
This PR enforces a weakened version of the shadowing restrictions from RFC 1560. More specifically,
- If a macro expansion contains a `macro_rules!` macro definition that is used outside of the expansion, the defined macro may not shadow an existing macro.
- If a macro expansion contains a `#[macro_use] extern crate` macro import that is used outside of the expansion, the imported macro may not shadow an existing macro.
This is a [breaking-change]. For example,
```rust
macro_rules! m { () => {} }
macro_rules! n { () => {
macro_rules! m { () => {} } //< This shadows an existing macro.
m!(); //< This is inside the expansion that generated `m`'s definition, so it is OK.
} }
n!();
m!(); //< This use of `m` is outside the expansion, so it causes the shadowing to be an error.
```
r? @nrc
…e_scopes, r=nrc
macros: clean up scopes of expanded `#[macro_use]` imports
This PR changes the scope of macro-expanded `#[macro_use]` imports to match that of unexpanded `#[macro_use]` imports. For example, this would be allowed:
```rust
example!();
macro_rules! m { () => { #[macro_use(example)] extern crate example_crate; } }
m!();
```
This PR also enforces the full shadowing restrictions from RFC 1560 on `#[macro_use]` imports (currently, we only enforce the weakened restrictions from rust-lang#36767).
This is a [breaking-change], but I believe it is highly unlikely to cause breakage in practice.
r? @nrc
This PR enforces a weakened version of the shadowing restrictions from RFC 1560. More specifically,
macro_rules!macro definition that is used outside of the expansion, the defined macro may not shadow an existing macro.#[macro_use] extern cratemacro import that is used outside of the expansion, the imported macro may not shadow an existing macro.This is a [breaking-change]. For example,
r? @nrc