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

Better attribute and metaitem encapsulation throughout the compiler #34842

Merged
merged 2 commits into from
Jul 30, 2016
Merged

Better attribute and metaitem encapsulation throughout the compiler #34842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a5e5ea1
General MetaItem encapsulation rewrites.
cgswords Jul 15, 2016
5553901
Adressed PR comments.
cgswords Jul 18, 2016
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
18 changes: 9 additions & 9 deletions src/librustc/lint/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,18 +366,18 @@ pub fn gather_attr(attr: &ast::Attribute)
attr::mark_used(attr);

let meta = &attr.node.value;
let metas = match meta.node {
ast::MetaItemKind::List(_, ref metas) => metas,
_ => {
out.push(Err(meta.span));
return out;
}
let metas = if let Some(metas) = meta.meta_item_list() {
metas
} else {
out.push(Err(meta.span));
return out;
};

for meta in metas {
out.push(match meta.node {
ast::MetaItemKind::Word(ref lint_name) => Ok((lint_name.clone(), level, meta.span)),
_ => Err(meta.span),
out.push(if meta.is_word() {
Ok((meta.name().clone(), level, meta.span))
} else {
Err(meta.span)
});
}

Expand Down
33 changes: 14 additions & 19 deletions src/librustc_driver/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ use std::thread;
use rustc::session::early_error;

use syntax::{ast, json};
use syntax::attr::AttrMetaMethods;
use syntax::codemap::{CodeMap, FileLoader, RealFileLoader};
use syntax::feature_gate::{GatedCfg, UnstableFeatures};
use syntax::parse::{self, PResult};
Expand Down Expand Up @@ -392,15 +393,12 @@ fn check_cfg(sopts: &config::Options,

let mut saw_invalid_predicate = false;
for item in sopts.cfg.iter() {
match item.node {
ast::MetaItemKind::List(ref pred, _) => {
saw_invalid_predicate = true;
handler.emit(&MultiSpan::new(),
&format!("invalid predicate in --cfg command line argument: `{}`",
pred),
errors::Level::Fatal);
}
_ => {},
if item.is_meta_item_list() {
saw_invalid_predicate = true;
handler.emit(&MultiSpan::new(),
&format!("invalid predicate in --cfg command line argument: `{}`",
item.name()),
errors::Level::Fatal);
}
}

Expand Down Expand Up @@ -649,20 +647,17 @@ impl RustcDefaultCalls {
if !allow_unstable_cfg && GatedCfg::gate(&*cfg).is_some() {
continue;
}
match cfg.node {
ast::MetaItemKind::Word(ref word) => println!("{}", word),
ast::MetaItemKind::NameValue(ref name, ref value) => {
println!("{}=\"{}\"", name, match value.node {
ast::LitKind::Str(ref s, _) => s,
_ => 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 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`.
ast::MetaItemKind::List(..) => {
panic!("MetaItemKind::List encountered in default cfg")
}
panic!("MetaItemKind::List encountered in default cfg")
}
}
}
Expand Down
29 changes: 12 additions & 17 deletions src/librustc_incremental/assert_dep_graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,11 @@ impl<'a, 'tcx> IfThisChanged<'a, 'tcx> {
if attr.check_name(IF_THIS_CHANGED) {
let mut id = None;
for meta_item in attr.meta_item_list().unwrap_or_default() {
match meta_item.node {
ast::MetaItemKind::Word(ref s) if id.is_none() => id = Some(s.clone()),
_ => {
self.tcx.sess.span_err(
meta_item.span,
&format!("unexpected meta-item {:?}", meta_item.node));
}
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)
}
}
let id = id.unwrap_or(InternedString::new(ID));
Expand All @@ -127,16 +125,13 @@ impl<'a, 'tcx> IfThisChanged<'a, 'tcx> {
let mut dep_node_interned = None;
let mut id = None;
for meta_item in attr.meta_item_list().unwrap_or_default() {
match meta_item.node {
ast::MetaItemKind::Word(ref s) if dep_node_interned.is_none() =>
dep_node_interned = Some(s.clone()),
ast::MetaItemKind::Word(ref s) if id.is_none() =>
id = Some(s.clone()),
_ => {
self.tcx.sess.span_err(
meta_item.span,
&format!("unexpected meta-item {:?}", meta_item.node));
}
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)
}
}
let dep_node = match dep_node_interned {
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_incremental/calculate_svh.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
//! Calculation of a Strict Version Hash for crates. For a length
//! comment explaining the general idea, see `librustc/middle/svh.rs`.

use syntax::attr::AttributeMethods;
use std::hash::{Hash, SipHasher, Hasher};
use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId};
use rustc::hir::svh::Svh;
Expand Down Expand Up @@ -69,7 +70,7 @@ impl<'a, 'tcx> SvhCalculate for TyCtxt<'a, 'tcx, 'tcx> {
// to avoid hashing the AttrId
for attr in &krate.attrs {
debug!("krate attr {:?}", attr);
attr.node.value.hash(&mut state);
attr.meta().hash(&mut state);
Copy link
Member

Choose a reason for hiding this comment

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

I find the name meta to be not useful, maybe you should rename it? (Could be a follow-up PR).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have name suggestions? value()? meta_item() (which is dangerously close to meta_item_list()?

Copy link
Member

Choose a reason for hiding this comment

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

inner() or meta_item() seem good

}

Svh::new(state.finish())
Expand Down
15 changes: 5 additions & 10 deletions src/librustc_lint/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ use lint::{LintPass, LateLintPass};
use std::collections::HashSet;

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

use rustc::hir::{self, PatKind};
Expand Down Expand Up @@ -298,12 +298,7 @@ impl MissingDoc {
}
}

let has_doc = attrs.iter().any(|a| {
match a.node.value.node {
ast::MetaItemKind::NameValue(ref name, _) if *name == "doc" => true,
_ => false
}
});
let has_doc = attrs.iter().any(|a| a.is_value_str() && a.name() == "doc");
if !has_doc {
cx.span_lint(MISSING_DOCS, sp,
&format!("missing documentation for {}", desc));
Expand Down Expand Up @@ -1094,10 +1089,10 @@ impl LintPass for UnstableFeatures {

impl LateLintPass for UnstableFeatures {
fn check_attribute(&mut self, ctx: &LateContext, attr: &ast::Attribute) {
if attr::contains_name(&[attr.node.value.clone()], "feature") {
if let Some(items) = attr.node.value.meta_item_list() {
if attr::contains_name(&[attr.meta().clone()], "feature") {
if let Some(items) = attr.meta().meta_item_list() {
for item in items {
ctx.span_lint(UNSTABLE_FEATURES, item.span, "unstable feature");
ctx.span_lint(UNSTABLE_FEATURES, item.span(), "unstable feature");
}
}
}
Expand Down
10 changes: 1 addition & 9 deletions src/librustc_metadata/decoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1160,15 +1160,7 @@ fn get_attributes(md: rbml::Doc) -> Vec<ast::Attribute> {
// an attribute
assert_eq!(meta_items.len(), 1);
let meta_item = meta_items.into_iter().nth(0).unwrap();
codemap::Spanned {
node: ast::Attribute_ {
id: attr::mk_attr_id(),
style: ast::AttrStyle::Outer,
value: meta_item,
is_sugared_doc: is_sugared_doc,
},
span: syntax_pos::DUMMY_SP
}
attr::mk_doc_attr_outer(attr::mk_attr_id(), meta_item, is_sugared_doc)
}).collect()
},
None => vec![],
Expand Down
37 changes: 17 additions & 20 deletions src/librustc_metadata/encoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ use std::rc::Rc;
use std::u32;
use syntax::abi::Abi;
use syntax::ast::{self, NodeId, Name, CRATE_NODE_ID, CrateNum};
use syntax::attr;
use syntax::attr::{self,AttrMetaMethods,AttributeMethods};
use errors::Handler;
use syntax;
use syntax_pos::BytePos;
Expand Down Expand Up @@ -1431,31 +1431,28 @@ fn encode_item_index(rbml_w: &mut Encoder, index: IndexData) {
}

fn encode_meta_item(rbml_w: &mut Encoder, mi: &ast::MetaItem) {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder why this doesn't use auto-derived serialization.

match mi.node {
ast::MetaItemKind::Word(ref name) => {
if mi.is_word() {
let name = mi.name();
rbml_w.start_tag(tag_meta_item_word);
rbml_w.wr_tagged_str(tag_meta_item_name, name);
rbml_w.wr_tagged_str(tag_meta_item_name, &name);
rbml_w.end_tag();
}
ast::MetaItemKind::NameValue(ref name, ref value) => {
match value.node {
ast::LitKind::Str(ref value, _) => {
rbml_w.start_tag(tag_meta_item_name_value);
rbml_w.wr_tagged_str(tag_meta_item_name, name);
rbml_w.wr_tagged_str(tag_meta_item_value, value);
rbml_w.end_tag();
}
_ => {/* FIXME (#623): encode other variants */ }
}
}
ast::MetaItemKind::List(ref name, ref items) => {
} else if mi.is_value_str() {
let name = mi.name();
/* FIXME (#623): support other literal kinds */
let value = mi.value_str().unwrap();
rbml_w.start_tag(tag_meta_item_name_value);
rbml_w.wr_tagged_str(tag_meta_item_name, &name);
rbml_w.wr_tagged_str(tag_meta_item_value, &value);
rbml_w.end_tag();
} else { // it must be a list
let name = mi.name();
let items = mi.meta_item_list().unwrap();
rbml_w.start_tag(tag_meta_item_list);
rbml_w.wr_tagged_str(tag_meta_item_name, name);
rbml_w.wr_tagged_str(tag_meta_item_name, &name);
for inner_item in items {
encode_meta_item(rbml_w, &inner_item);
}
rbml_w.end_tag();
}
}
}

Expand All @@ -1464,7 +1461,7 @@ fn encode_attributes(rbml_w: &mut Encoder, attrs: &[ast::Attribute]) {
for attr in attrs {
rbml_w.start_tag(tag_attribute);
rbml_w.wr_tagged_u8(tag_attribute_is_sugared_doc, attr.node.is_sugared_doc as u8);
encode_meta_item(rbml_w, &attr.node.value);
encode_meta_item(rbml_w, attr.meta());
rbml_w.end_tag();
}
rbml_w.end_tag();
Expand Down
12 changes: 6 additions & 6 deletions src/librustc_metadata/macro_import.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ impl<'a> ext::base::MacroLoader for MacroLoader<'a> {
}
if let (Some(sel), Some(names)) = (import.as_mut(), names) {
for attr in names {
if let ast::MetaItemKind::Word(ref name) = attr.node {
sel.insert(name.clone(), attr.span);
if attr.is_word() {
sel.insert(attr.name().clone(), attr.span());
} else {
span_err!(self.sess, attr.span, E0466, "bad macro import");
span_err!(self.sess, attr.span(), E0466, "bad macro import");
}
}
}
Expand All @@ -78,10 +78,10 @@ impl<'a> ext::base::MacroLoader for MacroLoader<'a> {
};

for attr in names {
if let ast::MetaItemKind::Word(ref name) = attr.node {
reexport.insert(name.clone(), attr.span);
if attr.is_word() {
reexport.insert(attr.name().clone(), attr.span());
} else {
call_bad_macro_reexport(self.sess, attr.span);
call_bad_macro_reexport(self.sess, attr.span());
}
}
}
Expand Down
61 changes: 31 additions & 30 deletions src/librustdoc/clean/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,21 +498,20 @@ pub enum Attribute {

impl Clean<Attribute> for ast::MetaItem {
fn clean(&self, cx: &DocContext) -> Attribute {
match self.node {
ast::MetaItemKind::Word(ref s) => Word(s.to_string()),
ast::MetaItemKind::List(ref s, ref l) => {
List(s.to_string(), l.clean(cx))
}
ast::MetaItemKind::NameValue(ref s, ref v) => {
NameValue(s.to_string(), lit_to_string(v))
}
}
if self.is_word() {
Word(self.name().to_string())
} else if let Some(v) = self.value_str() {
NameValue(self.name().to_string(), v.to_string())
} else { // must be a list
let l = self.meta_item_list().unwrap();
List(self.name().to_string(), l.clean(cx))
}
}
}

impl Clean<Attribute> for ast::Attribute {
fn clean(&self, cx: &DocContext) -> Attribute {
self.with_desugared_doc(|a| a.node.value.clean(cx))
self.with_desugared_doc(|a| a.meta().clean(cx))
}
}

Expand All @@ -535,6 +534,28 @@ impl attr::AttrMetaMethods for Attribute {
}
}
fn meta_item_list<'a>(&'a self) -> Option<&'a [P<ast::MetaItem>]> { None }

fn is_word(&self) -> bool {
match *self {
Word(_) => true,
_ => false,
}
}

fn is_value_str(&self) -> bool {
match *self {
NameValue(..) => true,
_ => false,
}
}

fn is_meta_item_list(&self) -> bool {
match *self {
List(..) => true,
_ => false,
}
}

fn span(&self) -> syntax_pos::Span { unimplemented!() }
}

Expand Down Expand Up @@ -2568,26 +2589,6 @@ impl ToSource for syntax_pos::Span {
}
}

fn lit_to_string(lit: &ast::Lit) -> String {
match lit.node {
ast::LitKind::Str(ref st, _) => st.to_string(),
ast::LitKind::ByteStr(ref data) => format!("{:?}", data),
ast::LitKind::Byte(b) => {
let mut res = String::from("b'");
for c in (b as char).escape_default() {
res.push(c);
}
res.push('\'');
res
},
ast::LitKind::Char(c) => format!("'{}'", c),
ast::LitKind::Int(i, _t) => i.to_string(),
ast::LitKind::Float(ref f, _t) => f.to_string(),
ast::LitKind::FloatUnsuffixed(ref f) => f.to_string(),
ast::LitKind::Bool(b) => b.to_string(),
}
}

fn name_from_pat(p: &hir::Pat) -> String {
use rustc::hir::*;
debug!("Trying to get a name from pattern: {:?}", p);
Expand Down
Loading