From f90de1110d93ea389342220caad19e05c4e6ad10 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Fri, 12 Oct 2018 00:21:21 -0700 Subject: [PATCH] in which lint reasons are restricted to come last in the attribute Vadim Petrochenkov suggested this in review ("an error? just to be conservative"), and it turns out to be convenient from the implementer's perspective: in the initial proposed implementation (or `HEAD~2`, as some might prefer to call it), we were doing an entire whole iteration over the meta items just to find the reason (before iterating over them to set the actual lint levels). This way, we can just peek at the end rather than adding that extra loop (or restructuring the existing code). The RFC doesn't seem to take a position on this, and there's some precedent for restricting things to be at the end of a sequence (we only allow `..` at the end of a struct pattern, even if it would be possible to let it appear anywhere in the sequence). --- src/librustc/lint/levels.rs | 73 +++++++++++++---------- src/test/ui/lint/reasons-erroneous.rs | 8 ++- src/test/ui/lint/reasons-erroneous.stderr | 24 ++++++-- 3 files changed, 69 insertions(+), 36 deletions(-) diff --git a/src/librustc/lint/levels.rs b/src/librustc/lint/levels.rs index ee878da00660e..d44facedc8b7c 100644 --- a/src/librustc/lint/levels.rs +++ b/src/librustc/lint/levels.rs @@ -21,6 +21,7 @@ use rustc_data_structures::stable_hasher::{HashStable, ToStableHashKey, use session::Session; use syntax::ast; use syntax::attr; +use syntax::feature_gate; use syntax::source_map::MultiSpan; use syntax::symbol::Symbol; use util::nodemap::FxHashMap; @@ -210,7 +211,7 @@ impl<'a> LintLevelsBuilder<'a> { let meta = unwrap_or!(attr.meta(), continue); attr::mark_used(attr); - let metas = if let Some(metas) = meta.meta_item_list() { + let mut metas = if let Some(metas) = meta.meta_item_list() { metas } else { let mut err = bad_attr(meta.span); @@ -218,44 +219,43 @@ impl<'a> LintLevelsBuilder<'a> { continue }; - // Before processing the lint names, look for a reason (RFC 2383). + // Before processing the lint names, look for a reason (RFC 2383) + // at the end. let mut reason = None; - for li in metas { - if let Some(item) = li.meta_item() { - match item.node { - ast::MetaItemKind::Word => {} // actual lint names handled later - ast::MetaItemKind::NameValue(ref name_value) => { - let gate_reasons = !self.sess.features_untracked().lint_reasons; - let name_ident = item.ident.segments[0].ident; - let name = name_ident.name.as_str(); - - if name == "reason" { - if let ast::LitKind::Str(rationale, _) = name_value.node { - if gate_reasons { - feature_gate::emit_feature_err( - &self.sess.parse_sess, - "lint_reasons", - item.span, - feature_gate::GateIssue::Language, - "lint reasons are experimental" - ); - } else { - reason = Some(rationale); - } + let tail_li = &metas[metas.len()-1]; + if let Some(item) = tail_li.meta_item() { + match item.node { + ast::MetaItemKind::Word => {} // actual lint names handled later + ast::MetaItemKind::NameValue(ref name_value) => { + let gate_reasons = !self.sess.features_untracked().lint_reasons; + if item.ident == "reason" { + // found reason, reslice meta list to exclude it + metas = &metas[0..metas.len()-1]; + if let ast::LitKind::Str(rationale, _) = name_value.node { + if gate_reasons { + feature_gate::emit_feature_err( + &self.sess.parse_sess, + "lint_reasons", + item.span, + feature_gate::GateIssue::Language, + "lint reasons are experimental" + ); } else { - let mut err = bad_attr(name_value.span); - err.help("reason must be a string literal"); - err.emit(); + reason = Some(rationale); } } else { - let mut err = bad_attr(item.span); + let mut err = bad_attr(name_value.span); + err.help("reason must be a string literal"); err.emit(); } - }, - ast::MetaItemKind::List(_) => { + } else { let mut err = bad_attr(item.span); err.emit(); } + }, + ast::MetaItemKind::List(_) => { + let mut err = bad_attr(item.span); + err.emit(); } } } @@ -263,7 +263,18 @@ impl<'a> LintLevelsBuilder<'a> { for li in metas { let word = match li.word() { Some(word) => word, - None => { continue; } + None => { + 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" { + err.help("reason in lint attribute must come last"); + } + } + } + err.emit(); + continue; + } }; let tool_name = if let Some(lint_tool) = word.is_scoped() { if !attr::is_known_lint_tool(lint_tool) { diff --git a/src/test/ui/lint/reasons-erroneous.rs b/src/test/ui/lint/reasons-erroneous.rs index 90111a9c4e1e9..e42b329338b5a 100644 --- a/src/test/ui/lint/reasons-erroneous.rs +++ b/src/test/ui/lint/reasons-erroneous.rs @@ -12,7 +12,13 @@ //~^ ERROR malformed lint attribute #![warn(elided_lifetimes_in_paths, reason("disrespectful to ancestors", "irresponsible to heirs"))] //~^ ERROR malformed lint attribute -#![warn(ellipsis_inclusive_range_patterns, reason)] +#![warn(ellipsis_inclusive_range_patterns, reason = "born barren", reason = "a freak growth")] +//~^ ERROR malformed lint attribute +//~| HELP reason in lint attribute must come last +#![warn(keyword_idents, reason = "root in rubble", macro_use_extern_crate)] +//~^ ERROR malformed lint attribute +//~| HELP reason in lint attribute must come last +#![warn(missing_copy_implementations, reason)] //~^ WARN unknown lint fn main() {} diff --git a/src/test/ui/lint/reasons-erroneous.stderr b/src/test/ui/lint/reasons-erroneous.stderr index f66999e927203..6842686ecbab5 100644 --- a/src/test/ui/lint/reasons-erroneous.stderr +++ b/src/test/ui/lint/reasons-erroneous.stderr @@ -32,14 +32,30 @@ error[E0452]: malformed lint attribute LL | #![warn(elided_lifetimes_in_paths, reason("disrespectful to ancestors", "irresponsible to heirs"))] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -warning: unknown lint: `reason` +error[E0452]: malformed lint attribute --> $DIR/reasons-erroneous.rs:15:44 | -LL | #![warn(ellipsis_inclusive_range_patterns, reason)] - | ^^^^^^ +LL | #![warn(ellipsis_inclusive_range_patterns, reason = "born barren", reason = "a freak growth")] + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = help: reason in lint attribute must come last + +error[E0452]: malformed lint attribute + --> $DIR/reasons-erroneous.rs:18:25 + | +LL | #![warn(keyword_idents, reason = "root in rubble", macro_use_extern_crate)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: reason in lint attribute must come last + +warning: unknown lint: `reason` + --> $DIR/reasons-erroneous.rs:21:39 + | +LL | #![warn(missing_copy_implementations, reason)] + | ^^^^^^ | = note: #[warn(unknown_lints)] on by default -error: aborting due to 5 previous errors +error: aborting due to 7 previous errors For more information about this error, try `rustc --explain E0452`.