Skip to content

Implement RFC 2539: cfg_attr with multiple attributes #54862

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

Merged
merged 4 commits into from
Oct 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/doc/unstable-book/src/language-features/cfg-attr-multi.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# `cfg_attr_multi`

The tracking issue for this feature is: [#54881]
The RFC for this feature is: [#2539]

[#54881]: https://github.com/rust-lang/rust/issues/54881
[#2539]: https://github.com/rust-lang/rfcs/pull/2539

------------------------

This feature flag lets you put multiple attributes into a `cfg_attr` attribute.

Example:

```rust,ignore
#[cfg_attr(all(), must_use, optimize)]
```

Because `cfg_attr` resolves before procedural macros, this does not affect
macro resolution at all.
95 changes: 78 additions & 17 deletions src/libsyntax/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,14 @@
// except according to those terms.

use attr::HasAttrs;
use feature_gate::{feature_err, EXPLAIN_STMT_ATTR_SYNTAX, Features, get_features, GateIssue};
use feature_gate::{
feature_err,
EXPLAIN_STMT_ATTR_SYNTAX,
Features,
get_features,
GateIssue,
emit_feature_err,
};
use {fold, attr};
use ast;
use source_map::Spanned;
Expand Down Expand Up @@ -73,49 +80,103 @@ impl<'a> StripUnconfigured<'a> {
if self.in_cfg(node.attrs()) { Some(node) } else { None }
}

/// Parse and expand all `cfg_attr` attributes into a list of attributes
/// that are within each `cfg_attr` that has a true configuration predicate.
///
/// Gives compiler warnigns if any `cfg_attr` does not contain any
/// attributes and is in the original source code. Gives compiler errors if
/// the syntax of any `cfg_attr` is incorrect.
pub fn process_cfg_attrs<T: HasAttrs>(&mut self, node: T) -> T {
node.map_attrs(|attrs| {
attrs.into_iter().filter_map(|attr| self.process_cfg_attr(attr)).collect()
attrs.into_iter().flat_map(|attr| self.process_cfg_attr(attr)).collect()
})
}

fn process_cfg_attr(&mut self, attr: ast::Attribute) -> Option<ast::Attribute> {
/// Parse and expand a single `cfg_attr` attribute into a list of attributes
/// when the configuration predicate is true, or otherwise expand into an
/// empty list of attributes.
///
/// Gives a compiler warning when the `cfg_attr` contains no attribtes and
/// is in the original source file. Gives a compiler error if the syntax of
/// the attribute is incorrect
fn process_cfg_attr(&mut self, attr: ast::Attribute) -> Vec<ast::Attribute> {
if !attr.check_name("cfg_attr") {
return Some(attr);
return vec![attr];
}

let (cfg, path, tokens, span) = match attr.parse(self.sess, |parser| {
let gate_cfg_attr_multi = if let Some(ref features) = self.features {
!features.cfg_attr_multi
} else {
false
};
let cfg_attr_span = attr.span;

let (cfg_predicate, expanded_attrs) = match attr.parse(self.sess, |parser| {
parser.expect(&token::OpenDelim(token::Paren))?;
let cfg = parser.parse_meta_item()?;

let cfg_predicate = parser.parse_meta_item()?;
parser.expect(&token::Comma)?;
let lo = parser.span.lo();
let (path, tokens) = parser.parse_meta_item_unrestricted()?;
parser.eat(&token::Comma); // Optional trailing comma

// Presumably, the majority of the time there will only be one attr.
let mut expanded_attrs = Vec::with_capacity(1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe premature, but have you considered using SmallVec to save an allocation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how to measure whether or not that would be a win. And yes, it's a bit premature, since I want to get the code right first.


while !parser.check(&token::CloseDelim(token::Paren)) {
let lo = parser.span.lo();
let (path, tokens) = parser.parse_meta_item_unrestricted()?;
expanded_attrs.push((path, tokens, parser.prev_span.with_lo(lo)));
parser.expect_one_of(&[token::Comma], &[token::CloseDelim(token::Paren)])?;
}

parser.expect(&token::CloseDelim(token::Paren))?;
Ok((cfg, path, tokens, parser.prev_span.with_lo(lo)))
Ok((cfg_predicate, expanded_attrs))
}) {
Ok(result) => result,
Err(mut e) => {
e.emit();
return None;
return Vec::new();
}
};

if attr::cfg_matches(&cfg, self.sess, self.features) {
self.process_cfg_attr(ast::Attribute {
// Check feature gate and lint on zero attributes in source. Even if the feature is gated,
// we still compute as if it wasn't, since the emitted error will stop compilation futher
// along the compilation.
match (expanded_attrs.len(), gate_cfg_attr_multi) {
(0, false) => {
// FIXME: Emit unused attribute lint here.
},
(1, _) => {},
(_, true) => {
emit_feature_err(
self.sess,
"cfg_attr_multi",
cfg_attr_span,
GateIssue::Language,
"cfg_attr with zero or more than one attributes is experimental",
);
},
(_, false) => {}
}

if attr::cfg_matches(&cfg_predicate, self.sess, self.features) {
// We call `process_cfg_attr` recursively in case there's a
// `cfg_attr` inside of another `cfg_attr`. E.g.
// `#[cfg_attr(false, cfg_attr(true, some_attr))]`.
expanded_attrs.into_iter()
.flat_map(|(path, tokens, span)| self.process_cfg_attr(ast::Attribute {
id: attr::mk_attr_id(),
style: attr.style,
path,
tokens,
is_sugared_doc: false,
span,
})
}))
.collect()
} else {
None
Vec::new()
}
}

// Determine if a node with the given attributes should be included in this configuration.
/// Determine if a node with the given attributes should be included in this configuration.
pub fn in_cfg(&mut self, attrs: &[ast::Attribute]) -> bool {
attrs.iter().all(|attr| {
if !is_cfg(attr) {
Expand Down Expand Up @@ -165,7 +226,7 @@ impl<'a> StripUnconfigured<'a> {
})
}

// Visit attributes on expression and statements (but not attributes on items in blocks).
/// Visit attributes on expression and statements (but not attributes on items in blocks).
fn visit_expr_attrs(&mut self, attrs: &[ast::Attribute]) {
// flag the offending attributes
for attr in attrs.iter() {
Expand Down
3 changes: 3 additions & 0 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,9 @@ declare_features! (

// Allows `impl Trait` in bindings (`let`, `const`, `static`)
(active, impl_trait_in_bindings, "1.30.0", Some(34511), None),

// #[cfg_attr(predicate, multiple, attributes, here)]
(active, cfg_attr_multi, "1.31.0", Some(54881), None),
);

declare_features! (
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ impl<'a> Parser<'a> {
/// Expect next token to be edible or inedible token. If edible,
/// then consume it; if inedible, then return without consuming
/// anything. Signal a fatal error if next token is unexpected.
fn expect_one_of(&mut self,
pub fn expect_one_of(&mut self,
edible: &[token::Token],
inedible: &[token::Token]) -> PResult<'a, ()>{
fn tokens_to_string(tokens: &[TokenType]) -> String {
Expand Down
13 changes: 0 additions & 13 deletions src/test/ui/cfg-attr-trailing-comma.rs

This file was deleted.

14 changes: 0 additions & 14 deletions src/test/ui/cfg-attr-trailing-comma.stderr

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0658]: no_core is experimental (see issue #29639)
--> $DIR/cfg-attr-crate-2.rs:15:21
|
LL | #![cfg_attr(broken, no_core)] //~ ERROR no_core is experimental
| ^^^^^^^^
| ^^^^^^^
|
= help: add #![feature(no_core)] to the crate attributes to enable

Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/conditional-compilation/cfg-attr-multi-false.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Test that cfg_attr doesn't emit any attributes when the
// configuation variable is false. This mirrors `cfg-attr-multi-true.rs`

// compile-pass

#![warn(unused_must_use)]
#![feature(cfg_attr_multi)]

#[cfg_attr(any(), deprecated, must_use)]
struct Struct {}

impl Struct {
fn new() -> Struct {
Struct {}
}
}

fn main() {
Struct::new();
}
16 changes: 16 additions & 0 deletions src/test/ui/conditional-compilation/cfg-attr-multi-invalid-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
//
// compile-flags: --cfg broken

#![feature(cfg_attr_multi)]
#![cfg_attr(broken, no_core, no_std)] //~ ERROR no_core is experimental

fn main() { }
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0658]: no_core is experimental (see issue #29639)
--> $DIR/cfg-attr-multi-invalid-1.rs:14:21
|
LL | #![cfg_attr(broken, no_core, no_std)] //~ ERROR no_core is experimental
| ^^^^^^^
|
= help: add #![feature(no_core)] to the crate attributes to enable

error: aborting due to previous error

For more information about this error, try `rustc --explain E0658`.
16 changes: 16 additions & 0 deletions src/test/ui/conditional-compilation/cfg-attr-multi-invalid-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
//
// compile-flags: --cfg broken

#![feature(cfg_attr_multi)]
#![cfg_attr(broken, no_std, no_core)] //~ ERROR no_core is experimental

fn main() { }
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0658]: no_core is experimental (see issue #29639)
--> $DIR/cfg-attr-multi-invalid-2.rs:14:29
|
LL | #![cfg_attr(broken, no_std, no_core)] //~ ERROR no_core is experimental
| ^^^^^^^
|
= help: add #![feature(no_core)] to the crate attributes to enable

error: aborting due to previous error

For more information about this error, try `rustc --explain E0658`.
22 changes: 22 additions & 0 deletions src/test/ui/conditional-compilation/cfg-attr-multi-true.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Test that cfg_attr with multiple attributes actually emits both attributes.
// This is done by emitting two attributes that cause new warnings, and then
// triggering those warnings.

// compile-pass

#![warn(unused_must_use)]
#![feature(cfg_attr_multi)]

#[cfg_attr(all(), deprecated, must_use)]
struct MustUseDeprecated {}

impl MustUseDeprecated { //~ warning: use of deprecated item
fn new() -> MustUseDeprecated { //~ warning: use of deprecated item
MustUseDeprecated {} //~ warning: use of deprecated item
}
}

fn main() {
MustUseDeprecated::new(); //~ warning: use of deprecated item
//| warning: unused `MustUseDeprecated` which must be used
}
38 changes: 38 additions & 0 deletions src/test/ui/conditional-compilation/cfg-attr-multi-true.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
warning: use of deprecated item 'MustUseDeprecated'
--> $DIR/cfg-attr-multi-true.rs:13:6
|
LL | impl MustUseDeprecated { //~ warning: use of deprecated item
| ^^^^^^^^^^^^^^^^^
|
= note: #[warn(deprecated)] on by default

warning: use of deprecated item 'MustUseDeprecated'
--> $DIR/cfg-attr-multi-true.rs:20:5
|
LL | MustUseDeprecated::new(); //~ warning: use of deprecated item
| ^^^^^^^^^^^^^^^^^^^^^^

warning: use of deprecated item 'MustUseDeprecated'
--> $DIR/cfg-attr-multi-true.rs:14:17
|
LL | fn new() -> MustUseDeprecated { //~ warning: use of deprecated item
| ^^^^^^^^^^^^^^^^^

warning: use of deprecated item 'MustUseDeprecated'
--> $DIR/cfg-attr-multi-true.rs:15:9
|
LL | MustUseDeprecated {} //~ warning: use of deprecated item
| ^^^^^^^^^^^^^^^^^

warning: unused `MustUseDeprecated` which must be used
--> $DIR/cfg-attr-multi-true.rs:20:5
|
LL | MustUseDeprecated::new(); //~ warning: use of deprecated item
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/cfg-attr-multi-true.rs:7:9
|
LL | #![warn(unused_must_use)]
| ^^^^^^^^^^^^^^^

Loading