Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Attributes on macro-generated macro invocations are not feature-gate-checked #32782

Closed
LeoTestard opened this issue Apr 6, 2016 · 0 comments · Fixed by #32791
Closed

Attributes on macro-generated macro invocations are not feature-gate-checked #32782

LeoTestard opened this issue Apr 6, 2016 · 0 comments · Fixed by #32791

Comments

@LeoTestard
Copy link
Contributor

There are currently two phases of feature-gate checking: one before and one after macro expansion. The first one can check attributes on macro invocations that are removed by expansion, and the second one can check attributes on items generated by macro expansion. The thing is that items generated by macro expansion which are themselves macro invocations are not checked:

macro_rules! bar (
    () => ()
);

macro_rules! foo (
    () => (
        #[allow_internal_unstable]
        bar!();
    );
);

foo!();

This compiles on nightly. I've got an incoming PR that fixes this, along with #32648.
@LeoTestard LeoTestard mentioned this issue Apr 7, 2016
Merged
LeoTestard added a commit to LeoTestard/rust that referenced this issue Apr 21, 2016
@LeoTestard
Remove the MacroVisitor pass.
d41f7e8 
This pass was supposed to check use of gated features before
`#[cfg]`-stripping but this was not the case since it in fact happens
after. Checks that are actually important and must be done before macro
expansion are now made where the features are actually used. Close rust-lang#32648.
Also ensure that attributes on macro-generated macro invocations are
checked as well. Close rust-lang#32782 and rust-lang#32655.
LeoTestard added a commit to LeoTestard/rust that referenced this issue Apr 21, 2016
@LeoTestard
Remove the MacroVisitor pass.
8b40531 
This pass was supposed to check use of gated features before
`#[cfg]`-stripping but this was not the case since it in fact happens
after. Checks that are actually important and must be done before macro
expansion are now made where the features are actually used. Close rust-lang#32648.
Also ensure that attributes on macro-generated macro invocations are
checked as well. Close rust-lang#32782 and rust-lang#32655.
LeoTestard added a commit to LeoTestard/rust that referenced this issue Apr 21, 2016
@LeoTestard
Remove the MacroVisitor pass.
03ab057 
This pass was supposed to check use of gated features before
`#[cfg]`-stripping but this was not the case since it in fact happens
after. Checks that are actually important and must be done before macro
expansion are now made where the features are actually used. Close rust-lang#32648.
Also ensure that attributes on macro-generated macro invocations are
checked as well. Close rust-lang#32782 and rust-lang#32655.
bors added a commit that referenced this issue Apr 28, 2016
@bors
Auto merge of #32791 - LeoTestard:feature-gate-clean, r=nikomatsakis
435095f 
Feature gate clean

This PR does a bit of cleaning in the feature-gate-handling code of libsyntax. It also fixes two bugs (#32782 and #32648). Changes include:

* Change the way the existing features are declared in `feature_gate.rs`. The array of features and the `Features` struct are now defined together by a single macro. `featureck.py` has been updated accordingly. Note: there are now three different arrays for active, removed and accepted features instead of a single one with a `Status` item to tell wether a feature is active, removed, or accepted. This is mainly due to the way I implemented my macro in the first time and I can switch back to a single array if needed. But an advantage of the way it is now is that when an active feature is used, the parser only searches through the list of active features. It goes through the other arrays only if the feature is not found. I like to think that error checking (in this case, checking that an used feature is active) does not slow down compilation of valid code. :) But this is not very important...
* Feature-gate checking pass now use the `Features` structure instead of looking through a string vector. This should speed them up a bit. The construction of the `Features` struct should be faster too since it is build directly when parsing features instead of calling `has_feature` dozens of times.
* The MacroVisitor pass has been removed, it was mostly useless since the `#[cfg]-stripping` phase happens before (fixes #32648). The features that must actually be checked before expansion are now checked at the time they are used. This also allows us to check attributes that are generated by macro expansion and not visible to MacroVisitor, but are also removed by macro expansion and thus not visible to PostExpansionVisitor either. This fixes #32782. Note that in order for `#[derive_*]` to be feature-gated but still accepted when generated by `#[derive(Trait)]`, I had to do a little bit of trickery with spans that I'm not totally confident into. Please review that part carefully. (It's in `libsyntax_ext/deriving/mod.rs`.)::

Note: this is a [breaking change], since programs with feature-gated attributes on macro-generated macro invocations were not rejected before. For example:

```rust
macro_rules! bar (
    () => ()
);

macro_rules! foo (
    () => (
        #[allow_internal_unstable] //~ ERROR allow_internal_unstable side-steps
        bar!();
    );
);
```
foo!();
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Feature gate clean LeoTestard/rust
1 participant
@LeoTestard