Skip to content

Implement RFC#1559: allow all literals in attributes #35850

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 1 commit into from
Aug 30, 2016
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
19 changes: 12 additions & 7 deletions src/librustc/hir/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
use session::Session;

use syntax::ast;
use syntax::attr::AttrMetaMethods;
use syntax::attr::{AttrNestedMetaItemMethods, AttrMetaMethods};
use syntax::visit;
use syntax::visit::Visitor;

Expand Down Expand Up @@ -52,18 +52,22 @@ impl<'a> CheckAttrVisitor<'a> {
return;
}
};

for word in words {
let word: &str = &word.name();
let message = match word {
let name = match word.name() {
Some(word) => word,
None => continue,
};

let message = match &*name {
"C" => {
if target != Target::Struct && target != Target::Enum {
"attribute should be applied to struct or enum"
"attribute should be applied to struct or enum"
} else {
continue
}
}
"packed" |
"simd" => {
"packed" | "simd" => {
if target != Target::Struct {
"attribute should be applied to struct"
} else {
Expand All @@ -74,13 +78,14 @@ impl<'a> CheckAttrVisitor<'a> {
"i32" | "u32" | "i64" | "u64" |
"isize" | "usize" => {
if target != Target::Enum {
"attribute should be applied to enum"
"attribute should be applied to enum"
} else {
continue
}
}
_ => continue,
};

span_err!(self.sess, attr.span, E0517, "{}", message);
}
}
Expand Down
23 changes: 20 additions & 3 deletions src/librustc/hir/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
//! and returns a piece of the same type.

use hir::*;
use syntax::ast::{Name, NodeId, DUMMY_NODE_ID, Attribute, Attribute_, MetaItem};
use syntax::ast::MetaItemKind;
use syntax::ast::{Name, NodeId, DUMMY_NODE_ID, Attribute, Attribute_};
use syntax::ast::{NestedMetaItem, NestedMetaItemKind, MetaItem, MetaItemKind};
use hir;
use syntax_pos::Span;
use syntax::codemap::{respan, Spanned};
Expand All @@ -38,6 +38,10 @@ pub trait Folder : Sized {
noop_fold_meta_items(meta_items, self)
}

fn fold_meta_list_item(&mut self, list_item: NestedMetaItem) -> NestedMetaItem {
noop_fold_meta_list_item(list_item, self)
}

fn fold_meta_item(&mut self, meta_item: P<MetaItem>) -> P<MetaItem> {
noop_fold_meta_item(meta_item, self)
}
Expand Down Expand Up @@ -486,13 +490,26 @@ pub fn noop_fold_attribute<T: Folder>(at: Attribute, fld: &mut T) -> Option<Attr
})
}

pub fn noop_fold_meta_list_item<T: Folder>(li: NestedMetaItem, fld: &mut T)
-> NestedMetaItem {
Spanned {
node: match li.node {
NestedMetaItemKind::MetaItem(mi) => {
NestedMetaItemKind::MetaItem(fld.fold_meta_item(mi))
},
NestedMetaItemKind::Literal(lit) => NestedMetaItemKind::Literal(lit)
},
span: fld.new_span(li.span)
}
}

pub fn noop_fold_meta_item<T: Folder>(mi: P<MetaItem>, fld: &mut T) -> P<MetaItem> {
mi.map(|Spanned { node, span }| {
Spanned {
node: match node {
MetaItemKind::Word(id) => MetaItemKind::Word(id),
MetaItemKind::List(id, mis) => {
MetaItemKind::List(id, mis.move_map(|e| fld.fold_meta_item(e)))
MetaItemKind::List(id, mis.move_map(|e| fld.fold_meta_list_item(e)))
}
MetaItemKind::NameValue(id, s) => MetaItemKind::NameValue(id, s),
},
Expand Down
12 changes: 5 additions & 7 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use util::nodemap::FnvHashMap;
use std::cmp;
use std::default::Default as StdDefault;
use std::mem;
use syntax::attr::{self, AttrMetaMethods};
use syntax::attr::{self, AttrMetaMethods, AttrNestedMetaItemMethods};
use syntax::parse::token::InternedString;
use syntax::ast;
use syntax_pos::Span;
Expand Down Expand Up @@ -372,12 +372,10 @@ pub fn gather_attr(attr: &ast::Attribute)
return out;
};

for meta in metas {
out.push(if meta.is_word() {
Ok((meta.name().clone(), level, meta.span))
} else {
Err(meta.span)
});
for li in metas {
out.push(li.word().map_or(Err(li.span), |word| {
Ok((word.name().clone(), level, word.span))
}));
}

out
Expand Down
7 changes: 4 additions & 3 deletions src/librustc_borrowck/borrowck/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
use borrowck::BorrowckCtxt;

use syntax::ast::{self, MetaItem};
use syntax::attr::AttrMetaMethods;
use syntax::attr::{AttrMetaMethods, AttrNestedMetaItemMethods};
use syntax::ptr::P;
use syntax_pos::{Span, DUMMY_SP};

Expand Down Expand Up @@ -43,8 +43,9 @@ fn has_rustc_mir_with(attrs: &[ast::Attribute], name: &str) -> Option<P<MetaItem
if attr.check_name("rustc_mir") {
let items = attr.meta_item_list();
for item in items.iter().flat_map(|l| l.iter()) {
if item.check_name(name) {
return Some(item.clone())
match item.meta_item() {
Some(mi) if mi.check_name(name) => return Some(mi.clone()),
_ => continue
}
}
}
Expand Down
12 changes: 7 additions & 5 deletions src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,17 +655,19 @@ impl RustcDefaultCalls {
if !allow_unstable_cfg && GatedCfg::gate(&*cfg).is_some() {
continue;
}

if cfg.is_word() {
println!("{}", cfg.name());
} else if cfg.is_value_str() {
if let Some(s) = cfg.value_str() {
println!("{}=\"{}\"", cfg.name(), s);
}
} else if let Some(s) = cfg.value_str() {
println!("{}=\"{}\"", cfg.name(), s);
} else if cfg.is_meta_item_list() {
// Right now there are not and should not be any
// MetaItemKind::List items in the configuration returned by
// `build_configuration`.
panic!("MetaItemKind::List encountered in default cfg")
panic!("Found an unexpected list in cfg attribute '{}'!", cfg.name())
} else {
// There also shouldn't be literals.
panic!("Found an unexpected literal in cfg attribute '{}'!", cfg.name())
Copy link
Member

Choose a reason for hiding this comment

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

Given that this is not meant to be a user-facing error message, I don't think we need two different messages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The two different error messages should help out if the conditions are ever hit. I propose we keep them: doing so doesn't hurt and can only help.

}
}
}
Expand Down
39 changes: 24 additions & 15 deletions src/librustc_incremental/assert_dep_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ use std::env;
use std::fs::File;
use std::io::Write;
use syntax::ast;
use syntax::attr::AttrMetaMethods;
use syntax::attr::{AttrNestedMetaItemMethods, AttrMetaMethods};
use syntax::parse::token::InternedString;
use syntax_pos::Span;

Expand Down Expand Up @@ -116,31 +116,40 @@ impl<'a, 'tcx> IfThisChanged<'a, 'tcx> {
for attr in self.tcx.get_attrs(def_id).iter() {
if attr.check_name(IF_THIS_CHANGED) {
let mut id = None;
for meta_item in attr.meta_item_list().unwrap_or_default() {
if meta_item.is_word() && id.is_none() {
id = Some(meta_item.name().clone());
} else {
// FIXME better-encapsulate meta_item (don't directly access `node`)
span_bug!(meta_item.span(), "unexpected meta-item {:?}", meta_item.node)
for list_item in attr.meta_item_list().unwrap_or_default() {
match list_item.word() {
Some(word) if id.is_none() => {
id = Some(word.name().clone())
},
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum Aug 20, 2016

Choose a reason for hiding this comment

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

node is still directly accessed in the span_bug! so perhaps the FIXME shouldn't be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This happens in a lot of places without a similar comment. Is it more important to place such a comment here than in other places?

Copy link
Contributor

Choose a reason for hiding this comment

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

I added those fixmes; ultimately, there should be some encapsulation to make meta-items more opaque (so that their internal representation is less obvious and more resilient to changes). If it's done other places, this fixme should be added there, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll re-add them.

_ => {
// FIXME better-encapsulate meta_item (don't directly access `node`)
span_bug!(list_item.span(), "unexpected list-item {:?}", list_item.node)
}
}
}

let id = id.unwrap_or(InternedString::new(ID));
self.if_this_changed.entry(id)
.or_insert(FnvHashSet())
.insert((attr.span, def_id, DepNode::Hir(def_id)));
} else if attr.check_name(THEN_THIS_WOULD_NEED) {
let mut dep_node_interned = None;
let mut id = None;
for meta_item in attr.meta_item_list().unwrap_or_default() {
if meta_item.is_word() && dep_node_interned.is_none() {
dep_node_interned = Some(meta_item.name().clone());
} else if meta_item.is_word() && id.is_none() {
id = Some(meta_item.name().clone());
} else {
// FIXME better-encapsulate meta_item (don't directly access `node`)
span_bug!(meta_item.span(), "unexpected meta-item {:?}", meta_item.node)
for list_item in attr.meta_item_list().unwrap_or_default() {
match list_item.word() {
Some(word) if dep_node_interned.is_none() => {
Copy link
Member

Choose a reason for hiding this comment

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

Same as above; FIXME might still need to be kept.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll re-add them.

dep_node_interned = Some(word.name().clone());
},
Some(word) if id.is_none() => {
id = Some(word.name().clone())
},
_ => {
// FIXME better-encapsulate meta_item (don't directly access `node`)
span_bug!(list_item.span(), "unexpected meta-item {:?}", list_item.node)
}
}
}

let dep_node = match dep_node_interned {
Some(ref n) => {
match DepNode::from_label_string(&n[..], def_id) {
Expand Down
16 changes: 10 additions & 6 deletions src/librustc_incremental/persist/dirty_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ use rustc::hir;
use rustc::hir::def_id::DefId;
use rustc::hir::intravisit::Visitor;
use rustc_data_structures::fnv::FnvHashSet;
use syntax::ast::{self, Attribute, MetaItem};
use syntax::attr::AttrMetaMethods;
use syntax::ast::{self, Attribute, NestedMetaItem};
use syntax::attr::{AttrNestedMetaItemMethods, AttrMetaMethods};
use syntax::parse::token::InternedString;
use rustc::ty::TyCtxt;

Expand Down Expand Up @@ -71,13 +71,17 @@ pub struct DirtyCleanVisitor<'a, 'tcx:'a> {
}

impl<'a, 'tcx> DirtyCleanVisitor<'a, 'tcx> {
fn expect_associated_value(&self, item: &MetaItem) -> InternedString {
fn expect_associated_value(&self, item: &NestedMetaItem) -> InternedString {
if let Some(value) = item.value_str() {
value
} else {
self.tcx.sess.span_fatal(
item.span,
&format!("associated value expected for `{}`", item.name()));
let msg = if let Some(name) = item.name() {
format!("associated value expected for `{}`", name)
} else {
"expected an associated value".to_string()
};

self.tcx.sess.span_fatal(item.span, &msg);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ use lint::{LintPass, LateLintPass};
use std::collections::HashSet;

use syntax::{ast};
use syntax::attr::{self, AttrMetaMethods, AttributeMethods};
use syntax_pos::Span;
use syntax::attr::{self, AttrMetaMethods, AttrNestedMetaItemMethods, AttributeMethods};
use syntax_pos::{Span};

use rustc::hir::{self, PatKind};
use rustc::hir::intravisit::FnKind;
Expand Down Expand Up @@ -317,7 +317,7 @@ impl LateLintPass for MissingDoc {
let doc_hidden = self.doc_hidden() || attrs.iter().any(|attr| {
attr.check_name("doc") && match attr.meta_item_list() {
None => false,
Some(l) => attr::contains_name(&l[..], "hidden"),
Some(l) => attr::list_contains_name(&l[..], "hidden"),
}
});
self.doc_hidden_stack.push(doc_hidden);
Expand Down
7 changes: 7 additions & 0 deletions src/librustc_lint/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,10 +234,13 @@ impl LintPass for UnusedAttributes {

impl LateLintPass for UnusedAttributes {
fn check_attribute(&mut self, cx: &LateContext, attr: &ast::Attribute) {
debug!("checking attribute: {:?}", attr);

// Note that check_name() marks the attribute as used if it matches.
for &(ref name, ty, _) in KNOWN_ATTRIBUTES {
match ty {
AttributeType::Whitelisted if attr.check_name(name) => {
debug!("{:?} is Whitelisted", name);
break;
},
_ => ()
Expand All @@ -247,11 +250,13 @@ impl LateLintPass for UnusedAttributes {
let plugin_attributes = cx.sess().plugin_attributes.borrow_mut();
for &(ref name, ty) in plugin_attributes.iter() {
if ty == AttributeType::Whitelisted && attr.check_name(&name) {
debug!("{:?} (plugin attr) is whitelisted with ty {:?}", name, ty);
break;
}
}

if !attr::is_used(attr) {
debug!("Emitting warning for: {:?}", attr);
cx.span_lint(UNUSED_ATTRIBUTES, attr.span, "unused attribute");
// Is it a builtin attribute that must be used at the crate level?
let known_crate = KNOWN_ATTRIBUTES.iter().find(|&&(name, ty, _)| {
Expand All @@ -275,6 +280,8 @@ impl LateLintPass for UnusedAttributes {
};
cx.span_lint(UNUSED_ATTRIBUTES, attr.span, msg);
}
} else {
debug!("Attr was used: {:?}", attr);
}
}
}
Expand Down
15 changes: 1 addition & 14 deletions src/librustc_metadata/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,26 +45,13 @@ pub const tag_items_closure_kind: usize = 0x2a;
pub const tag_items_closure_ty: usize = 0x2b;
pub const tag_def_key: usize = 0x2c;

// GAP 0x2d 0x2e
// GAP 0x2d 0x34

pub const tag_index: usize = 0x110; // top-level only
pub const tag_xref_index: usize = 0x111; // top-level only
pub const tag_xref_data: usize = 0x112; // top-level only

pub const tag_meta_item_name_value: usize = 0x2f;

pub const tag_meta_item_name: usize = 0x30;

pub const tag_meta_item_value: usize = 0x31;

pub const tag_attributes: usize = 0x101; // top-level only

pub const tag_attribute: usize = 0x32;

pub const tag_meta_item_word: usize = 0x33;

pub const tag_meta_item_list: usize = 0x34;

// The list of crates that this crate depends on
pub const tag_crate_deps: usize = 0x102; // top-level only

Expand Down
3 changes: 1 addition & 2 deletions src/librustc_metadata/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ use syntax::ast;
use syntax::abi::Abi;
use syntax::codemap;
use syntax::parse;
use syntax::attr;
use syntax::attr::AttrMetaMethods;
use syntax::attr::{self, AttrMetaMethods, AttrNestedMetaItemMethods};
use syntax::parse::token::InternedString;
use syntax::visit;
use syntax_pos::{self, Span, mk_sp, Pos};
Expand Down
Loading