Add #[allow_internal_unstable] to track stability for macros better. #22899
Conversation
This file contains 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
|
r? @kmcallister (rust_highfive has picked a reviewer for you, use r? to override) |
|
r? @alexcrichton, @aturon @alexcrichton this is the fix for the macro issues we were discussing on IRC a few days ago. |
|
🏆 |
|
Nice! This looks pretty great to me. Can this de-stabilize all of the internal fields of the thread local structs as well? (now that they can explicitly be left as unstable) |
|
Updated, destabilising essentially everything in |
|
@bors: r+ 5ea24dd |
|
This is awesome :D |
|
⌛ Testing commit 5ea24dd with merge 2196aa3... |
|
💔 Test failed - auto-mac-32-opt |
|
|
|
@bors r=alexcrichton 49ef |
|
⌛ Testing commit 49ef199 with merge 29c07e2... |
|
💔 Test failed - auto-mac-32-opt |
|
|
☔ The latest upstream changes (presumably #22995) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Updated; I've modified the approach and several extra commits (the I've had to add another feature gating pass, because the post-expansion feature-gating pass ran before the second configuration pass, and so would miss custom attributes inserted by |
|
This looks good to me, r=me. Could you also knock out some |
|
What about |
|
|
Unstable items used in a macro expansion will now always trigger
stability warnings, *unless* the unstable items are directly inside a
macro marked with `#[allow_internal_unstable]`. IOW, the compiler warns
unless the span of the unstable item is a subspan of the definition of a
macro marked with that attribute.
E.g.
#[allow_internal_unstable]
macro_rules! foo {
($e: expr) => {{
$e;
unstable(); // no warning
only_called_by_foo!();
}}
}
macro_rules! only_called_by_foo {
() => { unstable() } // warning
}
foo!(unstable()) // warning
The unstable inside `foo` is fine, due to the attribute. But the
`unstable` inside `only_called_by_foo` is not, since that macro doesn't
have the attribute, and the `unstable` passed into `foo` is also not
fine since it isn't contained in the macro itself (that is, even though
it is only used directly in the macro).
In the process this makes the stability tracking much more precise,
e.g. previously `println!("{}", unstable())` got no warning, but now it
does. As such, this is a bug fix that may cause [breaking-change]s.
The attribute is definitely feature gated, since it explicitly allows
side-stepping the feature gating system.
This destabilises all the implementation details of `thread_local!`, since they do not *need* to be stable with the new attribute.
This ensures we catch everything; previously, an unknown attribute inserted by #[cfg_attr(...)] in a macro expansion would not be detected.
|
@bors r=alexcrichton b5c6 |
|
⌛ Testing commit b5c6ab2 with merge ffb90ea... |
|
💔 Test failed - auto-linux-64-x-android-t |
|
@bors: retry |
1 similar comment
|
@bors: retry |
Manishearth
added a commit
to Manishearth/rust
that referenced
this pull request
Mar 6, 2015
Unstable items used in a macro expansion will now always trigger
stability warnings, *unless* the unstable items are directly inside a
macro marked with `#[allow_internal_unstable]`. IOW, the compiler warns
unless the span of the unstable item is a subspan of the definition of a
macro marked with that attribute.
E.g.
#[allow_internal_unstable]
macro_rules! foo {
($e: expr) => {{
$e;
unstable(); // no warning
only_called_by_foo!();
}}
}
macro_rules! only_called_by_foo {
() => { unstable() } // warning
}
foo!(unstable()) // warning
The unstable inside `foo` is fine, due to the attribute. But the
`unstable` inside `only_called_by_foo` is not, since that macro doesn't
have the attribute, and the `unstable` passed into `foo` is also not
fine since it isn't contained in the macro itself (that is, even though
it is only used directly in the macro).
In the process this makes the stability tracking much more precise,
e.g. previously `println!(\"{}\", unstable())` got no warning, but now it
does. As such, this is a bug fix that may cause [breaking-change]s.
The attribute is definitely feature gated, since it explicitly allows
side-stepping the feature gating system.
---
This updates `thread_local!` macro to use the attribute, since it uses
unstable features internally (initialising a struct with unstable
fields).
bors
added a commit
that referenced
this pull request
Mar 6, 2015
Unstable items used in a macro expansion will now always trigger
stability warnings, *unless* the unstable items are directly inside a
macro marked with `#[allow_internal_unstable]`. IOW, the compiler warns
unless the span of the unstable item is a subspan of the definition of a
macro marked with that attribute.
E.g.
#[allow_internal_unstable]
macro_rules! foo {
($e: expr) => {{
$e;
unstable(); // no warning
only_called_by_foo!();
}}
}
macro_rules! only_called_by_foo {
() => { unstable() } // warning
}
foo!(unstable()) // warning
The unstable inside `foo` is fine, due to the attribute. But the
`unstable` inside `only_called_by_foo` is not, since that macro doesn't
have the attribute, and the `unstable` passed into `foo` is also not
fine since it isn't contained in the macro itself (that is, even though
it is only used directly in the macro).
In the process this makes the stability tracking much more precise,
e.g. previously `println!("{}", unstable())` got no warning, but now it
does. As such, this is a bug fix that may cause [breaking-change]s.
The attribute is definitely feature gated, since it explicitly allows
side-stepping the feature gating system.
---
This updates `thread_local!` macro to use the attribute, since it uses
unstable features internally (initialising a struct with unstable
fields).
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.
Unstable items used in a macro expansion will now always trigger
stability warnings, unless the unstable items are directly inside a
macro marked with
#[allow_internal_unstable]. IOW, the compiler warnsunless the span of the unstable item is a subspan of the definition of a
macro marked with that attribute.
E.g.
The unstable inside
foois fine, due to the attribute. But theunstableinsideonly_called_by_foois not, since that macro doesn'thave the attribute, and the
unstablepassed intofoois also notfine since it isn't contained in the macro itself (that is, even though
it is only used directly in the macro).
In the process this makes the stability tracking much more precise,
e.g. previously
println!("{}", unstable())got no warning, but now itdoes. As such, this is a bug fix that may cause [breaking-change]s.
The attribute is definitely feature gated, since it explicitly allows
side-stepping the feature gating system.
This updates
thread_local!macro to use the attribute, since it usesunstable features internally (initialising a struct with unstable
fields).