Skip to content

Commit

Permalink
Rollup merge of rust-lang#58899 - petrochenkov:derval2, r=estebank
Browse files Browse the repository at this point in the history
Do not accidentally treat multi-segment meta-items as single-segment

Fixes rust-lang#55168 and many other regressions from rust-lang#50030

Basically, attributes like `#[any::prefix::foo]` were commonly interpreted as `#[foo]` due to `name()` successfully returning the last segment (this applies to nested things as well `#[attr(any::prefix::foo)]`).
  • Loading branch information
Centril authored Mar 13, 2019
2 parents 9758d97 + bfb200d commit 80049d9
Show file tree
Hide file tree
Showing 52 changed files with 563 additions and 509 deletions.
12 changes: 6 additions & 6 deletions src/librustc/hir/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
// ```
let hints: Vec<_> = item.attrs
.iter()
.filter(|attr| attr.name() == "repr")
.filter(|attr| attr.check_name("repr"))
.filter_map(|attr| attr.meta_item_list())
.flatten()
.collect();
Expand All @@ -177,15 +177,15 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
let mut is_transparent = false;

for hint in &hints {
let name = if let Some(name) = hint.name() {
let name = if let Some(name) = hint.ident_str() {
name
} else {
// Invalid repr hint like repr(42). We don't check for unrecognized hints here
// (libsyntax does that), so just ignore it.
continue;
};

let (article, allowed_targets) = match &*name.as_str() {
let (article, allowed_targets) = match name {
"C" | "align" => {
is_c |= name == "C";
if target != Target::Struct &&
Expand Down Expand Up @@ -233,7 +233,7 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
_ => continue,
};
self.emit_repr_error(
hint.span,
hint.span(),
item.span,
&format!("attribute should be applied to {}", allowed_targets),
&format!("not {} {}", article, allowed_targets),
Expand All @@ -242,7 +242,7 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {

// Just point at all repr hints if there are any incompatibilities.
// This is not ideal, but tracking precisely which ones are at fault is a huge hassle.
let hint_spans = hints.iter().map(|hint| hint.span);
let hint_spans = hints.iter().map(|hint| hint.span());

// Error on repr(transparent, <anything else>).
if is_transparent && hints.len() > 1 {
Expand Down Expand Up @@ -313,7 +313,7 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {

fn check_used(&self, item: &hir::Item, target: Target) {
for attr in &item.attrs {
if attr.name() == "used" && target != Target::Static {
if attr.check_name("used") && target != Target::Static {
self.tcx.sess
.span_err(attr.span, "attribute must be applied to a `static` variable");
}
Expand Down
11 changes: 5 additions & 6 deletions src/librustc/ich/impls_syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ impl<'a> HashStable<StableHashingContext<'a>> for [ast::Attribute] {
let filtered: SmallVec<[&ast::Attribute; 8]> = self
.iter()
.filter(|attr| {
!attr.is_sugared_doc && !hcx.is_ignored_attr(attr.name())
!attr.is_sugared_doc &&
!attr.ident().map_or(false, |ident| hcx.is_ignored_attr(ident.name))
})
.collect();

Expand All @@ -224,7 +225,7 @@ impl<'a> HashStable<StableHashingContext<'a>> for ast::Attribute {
hcx: &mut StableHashingContext<'a>,
hasher: &mut StableHasher<W>) {
// Make sure that these have been filtered out.
debug_assert!(!hcx.is_ignored_attr(self.name()));
debug_assert!(!self.ident().map_or(false, |ident| hcx.is_ignored_attr(ident.name)));
debug_assert!(!self.is_sugared_doc);

let ast::Attribute {
Expand Down Expand Up @@ -359,15 +360,13 @@ fn hash_token<'a, 'gcx, W: StableHasherResult>(
}
}

impl_stable_hash_for_spanned!(::syntax::ast::NestedMetaItemKind);

impl_stable_hash_for!(enum ::syntax::ast::NestedMetaItemKind {
impl_stable_hash_for!(enum ::syntax::ast::NestedMetaItem {
MetaItem(meta_item),
Literal(lit)
});

impl_stable_hash_for!(struct ::syntax::ast::MetaItem {
ident,
path,
node,
span
});
Expand Down
45 changes: 23 additions & 22 deletions src/librustc/lint/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ impl<'a> LintLevelsBuilder<'a> {
struct_span_err!(sess, span, E0452, "malformed lint attribute")
};
for attr in attrs {
let level = match Level::from_str(&attr.name().as_str()) {
let level = match attr.ident_str().and_then(|name| Level::from_str(name)) {
None => continue,
Some(lvl) => lvl,
};
Expand All @@ -220,7 +220,7 @@ impl<'a> LintLevelsBuilder<'a> {
match item.node {
ast::MetaItemKind::Word => {} // actual lint names handled later
ast::MetaItemKind::NameValue(ref name_value) => {
if item.ident == "reason" {
if item.path == "reason" {
// found reason, reslice meta list to exclude it
metas = &metas[0..metas.len()-1];
// FIXME (#55112): issue unused-attributes lint if we thereby
Expand Down Expand Up @@ -254,13 +254,13 @@ impl<'a> LintLevelsBuilder<'a> {
}

for li in metas {
let word = match li.word() {
Some(word) => word,
None => {
let mut err = bad_attr(li.span);
let meta_item = match li.meta_item() {
Some(meta_item) if meta_item.is_word() => meta_item,
_ => {
let mut err = bad_attr(li.span());
if let Some(item) = li.meta_item() {
if let ast::MetaItemKind::NameValue(_) = item.node {
if item.ident == "reason" {
if item.path == "reason" {
err.help("reason in lint attribute must come last");
}
}
Expand All @@ -269,26 +269,27 @@ impl<'a> LintLevelsBuilder<'a> {
continue;
}
};
let tool_name = if let Some(lint_tool) = word.is_scoped() {
if !attr::is_known_lint_tool(lint_tool) {
let tool_name = if meta_item.path.segments.len() > 1 {
let tool_ident = meta_item.path.segments[0].ident;
if !attr::is_known_lint_tool(tool_ident) {
span_err!(
sess,
lint_tool.span,
tool_ident.span,
E0710,
"an unknown tool name found in scoped lint: `{}`",
word.ident
meta_item.path
);
continue;
}

Some(lint_tool.as_str())
Some(tool_ident.as_str())
} else {
None
};
let name = word.name();
let name = meta_item.path.segments.last().expect("empty lint name").ident.name;
match store.check_lint_name(&name.as_str(), tool_name) {
CheckLintNameResult::Ok(ids) => {
let src = LintSource::Node(name, li.span, reason);
let src = LintSource::Node(name, li.span(), reason);
for id in ids {
specs.insert(*id, (level, src));
}
Expand All @@ -299,7 +300,7 @@ impl<'a> LintLevelsBuilder<'a> {
Ok(ids) => {
let complete_name = &format!("{}::{}", tool_name.unwrap(), name);
let src = LintSource::Node(
Symbol::intern(complete_name), li.span, reason
Symbol::intern(complete_name), li.span(), reason
);
for id in ids {
specs.insert(*id, (level, src));
Expand All @@ -321,18 +322,18 @@ impl<'a> LintLevelsBuilder<'a> {
lint,
lvl,
src,
Some(li.span.into()),
Some(li.span().into()),
&msg,
);
err.span_suggestion(
li.span,
li.span(),
"change it to",
new_lint_name.to_string(),
Applicability::MachineApplicable,
).emit();

let src = LintSource::Node(
Symbol::intern(&new_lint_name), li.span, reason
Symbol::intern(&new_lint_name), li.span(), reason
);
for id in ids {
specs.insert(*id, (level, src));
Expand All @@ -359,11 +360,11 @@ impl<'a> LintLevelsBuilder<'a> {
lint,
level,
src,
Some(li.span.into()),
Some(li.span().into()),
&msg);
if let Some(new_name) = renamed {
err.span_suggestion(
li.span,
li.span(),
"use the new name",
new_name,
Applicability::MachineApplicable
Expand All @@ -382,12 +383,12 @@ impl<'a> LintLevelsBuilder<'a> {
lint,
level,
src,
Some(li.span.into()),
Some(li.span().into()),
&msg);

if let Some(suggestion) = suggestion {
db.span_suggestion(
li.span,
li.span(),
"did you mean",
suggestion.to_string(),
Applicability::MachineApplicable,
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/middle/lib_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ impl<'a, 'tcx> LibFeatureCollector<'a, 'tcx> {
for meta in metas {
if let Some(mi) = meta.meta_item() {
// Find the `feature = ".."` meta-item.
match (&*mi.name().as_str(), mi.value_str()) {
("feature", val) => feature = val,
("since", val) => since = val,
match (mi.ident_str(), mi.value_str()) {
(Some("feature"), val) => feature = val,
(Some("since"), val) => since = val,
_ => {}
}
}
Expand Down
11 changes: 6 additions & 5 deletions src/librustc/middle/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,12 @@ impl<'a, 'tcx: 'a> Annotator<'a, 'tcx> {
} else {
// Emit errors for non-staged-api crates.
for attr in attrs {
let tag = attr.name();
if tag == "unstable" || tag == "stable" || tag == "rustc_deprecated" {
attr::mark_used(attr);
self.tcx.sess.span_err(attr.span(), "stability attributes may not be used \
outside of the standard library");
if let Some(tag) = attr.ident_str() {
if tag == "unstable" || tag == "stable" || tag == "rustc_deprecated" {
attr::mark_used(attr);
self.tcx.sess.span_err(attr.span, "stability attributes may not be used \
outside of the standard library");
}
}
}

Expand Down
5 changes: 3 additions & 2 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1824,7 +1824,7 @@ pub fn parse_cfgspecs(cfgspecs: Vec<String>) -> FxHashSet<(String, Option<String

match &mut parser.parse_meta_item() {
Ok(meta_item) if parser.token == token::Eof => {
if meta_item.ident.segments.len() != 1 {
if meta_item.path.segments.len() != 1 {
error!("argument key must be an identifier");
}
match &meta_item.node {
Expand All @@ -1835,7 +1835,8 @@ pub fn parse_cfgspecs(cfgspecs: Vec<String>) -> FxHashSet<(String, Option<String
error!("argument value must be a string");
}
MetaItemKind::NameValue(..) | MetaItemKind::Word => {
return (meta_item.name(), meta_item.value_str());
let ident = meta_item.ident().expect("multi-segment cfg key");
return (ident.name, meta_item.value_str());
}
}
}
Expand Down
14 changes: 8 additions & 6 deletions src/librustc/traits/on_unimplemented.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedDirective {
{
if let Some(items) = item.meta_item_list() {
if let Ok(subcommand) =
Self::parse(tcx, trait_def_id, &items, item.span, false)
Self::parse(tcx, trait_def_id, &items, item.span(), false)
{
subcommands.push(subcommand);
} else {
Expand All @@ -118,7 +118,7 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedDirective {
}

// nothing found
parse_error(tcx, item.span,
parse_error(tcx, item.span(),
"this attribute must have a valid value",
"expected value here",
Some(r#"eg `#[rustc_on_unimplemented(message="foo")]`"#));
Expand Down Expand Up @@ -177,10 +177,12 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedDirective {
for command in self.subcommands.iter().chain(Some(self)).rev() {
if let Some(ref condition) = command.condition {
if !attr::eval_condition(condition, &tcx.sess.parse_sess, &mut |c| {
options.contains(&(
c.name().as_str().to_string(),
c.value_str().map(|s| s.as_str().to_string())
))
c.ident_str().map_or(false, |name| {
options.contains(&(
name.to_string(),
c.value_str().map(|s| s.as_str().to_string())
))
})
}) {
debug!("evaluate: skipping {:?} due to condition", command);
continue
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ impl RustcDefaultCalls {

let mut cfgs = sess.parse_sess.config.iter().filter_map(|&(name, ref value)| {
let gated_cfg = GatedCfg::gate(&ast::MetaItem {
ident: ast::Path::from_ident(ast::Ident::with_empty_ctxt(name)),
path: ast::Path::from_ident(ast::Ident::with_empty_ctxt(name)),
node: ast::MetaItemKind::Word,
span: DUMMY_SP,
});
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_incremental/assert_dep_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,12 @@ impl<'a, 'tcx> IfThisChanged<'a, 'tcx> {
fn argument(&self, attr: &ast::Attribute) -> Option<ast::Name> {
let mut value = None;
for list_item in attr.meta_item_list().unwrap_or_default() {
match list_item.word() {
Some(word) if value.is_none() =>
value = Some(word.name()),
match list_item.ident() {
Some(ident) if list_item.is_word() && value.is_none() =>
value = Some(ident.name),
_ =>
// FIXME better-encapsulate meta_item (don't directly access `node`)
span_bug!(list_item.span(), "unexpected meta-item {:?}", list_item.node),
span_bug!(list_item.span(), "unexpected meta-item {:?}", list_item),
}
}
value
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_incremental/assert_module_sources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ impl<'a, 'tcx> AssertModuleSource<'a, 'tcx> {
return value;
} else {
self.tcx.sess.span_fatal(
item.span,
item.span(),
&format!("associated value expected for `{}`", name));
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_incremental/persist/dirty_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,13 +430,13 @@ impl<'a, 'tcx> DirtyCleanVisitor<'a, 'tcx> {
if DepNode::has_label_string(label) {
if out.contains(label) {
self.tcx.sess.span_fatal(
item.span,
item.span(),
&format!("dep-node label `{}` is repeated", label));
}
out.insert(label.to_string());
} else {
self.tcx.sess.span_fatal(
item.span,
item.span(),
&format!("dep-node label `{}` not recognized", label));
}
}
Expand Down Expand Up @@ -576,13 +576,13 @@ fn expect_associated_value(tcx: TyCtxt<'_, '_, '_>, item: &NestedMetaItem) -> as
if let Some(value) = item.value_str() {
value
} else {
let msg = if let Some(name) = item.name() {
let msg = if let Some(name) = item.ident_str() {
format!("associated value expected for `{}`", name)
} else {
"expected an associated value".to_string()
};

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

Expand Down
4 changes: 2 additions & 2 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ impl LintPass for DeprecatedAttr {
impl EarlyLintPass for DeprecatedAttr {
fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &ast::Attribute) {
for &&(n, _, _, ref g) in &self.depr_attrs {
if attr.name() == n {
if attr.ident_str() == Some(n) {
if let &AttributeGate::Gated(Stability::Deprecated(link, suggestion),
ref name,
ref reason,
Expand Down Expand Up @@ -831,7 +831,7 @@ impl UnusedDocComment {

let span = sugared_span.take().unwrap_or_else(|| attr.span);

if attr.name() == "doc" {
if attr.check_name("doc") {
let mut err = cx.struct_span_lint(UNUSED_DOC_COMMENTS, span, "unused doc comment");

err.span_label(
Expand Down
Loading

0 comments on commit 80049d9

Please sign in to comment.