From 426331b9e44a955b0e43da9b1542b7cf15fac31e Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Fri, 23 Jun 2017 18:29:30 -0700 Subject: [PATCH 1/2] remove unused parameters from LintStore.find_lint Long ago, in the before-time, the find_lint method was created with the unused_variables ("unused_variable" in the singular, as it was called at the time) attribute in anticipation of using the session and span in the handling of renamed lints (31b7d64fd), and indeed, the session and span came to be used in this method, while the unused_variables attribute remained (1ad1e2e29). In modern times, the session and span are again no longer used (ca81d3dd); it seems we can safely prune them from the method signature, for justice, and mercy. --- src/librustc/lint/context.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index a9e0ef511024f..a550db34d6f23 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -291,16 +291,13 @@ impl LintStore { self.by_name.insert(name.into(), Removed(reason.into())); } - #[allow(unused_variables)] - fn find_lint(&self, lint_name: &str, sess: &Session, span: Option) - -> Result - { + fn find_lint(&self, lint_name: &str) -> Result { match self.by_name.get(lint_name) { Some(&Id(lint_id)) => Ok(lint_id), Some(&Renamed(_, lint_id)) => { Ok(lint_id) }, - Some(&Removed(ref reason)) => { + Some(&Removed(_)) => { Err(FindLintError::Removed) }, None => Err(FindLintError::NotFound) @@ -313,7 +310,7 @@ impl LintStore { &lint_name[..], level); let lint_flag_val = Symbol::intern(&lint_name); - match self.find_lint(&lint_name[..], sess, None) { + match self.find_lint(&lint_name[..]) { Ok(lint_id) => self.levels.set(lint_id, (level, CommandLine(lint_flag_val))), Err(FindLintError::Removed) => { } Err(_) => { @@ -731,7 +728,7 @@ pub trait LintContext<'tcx>: Sized { continue; } Ok((lint_name, level, span)) => { - match self.lints().find_lint(&lint_name.as_str(), &self.sess(), Some(span)) { + match self.lints().find_lint(&lint_name.as_str()) { Ok(lint_id) => vec![(lint_id, level, span)], Err(FindLintError::NotFound) => { match self.lints().lint_groups.get(&*lint_name.as_str()) { @@ -1420,7 +1417,7 @@ impl Decodable for LintId { fn decode(d: &mut D) -> Result { let s = d.read_str()?; ty::tls::with(|tcx| { - match tcx.sess.lint_store.borrow().find_lint(&s, tcx.sess, None) { + match tcx.sess.lint_store.borrow().find_lint(&s) { Ok(id) => Ok(id), Err(_) => panic!("invalid lint-id `{}`", s), } From 890a76f479e32d7509b85aadd48301634508de39 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Fri, 23 Jun 2017 19:22:06 -0700 Subject: [PATCH 2/2] only set "overruled by outer forbid" once for lint groups, by group name Previously, conflicting forbid/allow attributes for a lint group would result in a separate "allow(L) overruled by outer forbid(L)" error for every lint L in the group. This was needlessly and annoyingly verbose; we prefer to just have one error pointing out the conflicting attributes. Resolves #42873. --- src/librustc/lint/context.rs | 26 ++++++++++++++++--------- src/test/ui/lint/outer-forbid.rs | 22 +++++++++++++++++++++ src/test/ui/lint/outer-forbid.stderr | 29 ++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/lint/outer-forbid.rs create mode 100644 src/test/ui/lint/outer-forbid.stderr diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index a550db34d6f23..466d163854f1d 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -721,7 +721,7 @@ pub trait LintContext<'tcx>: Sized { let mut pushed = 0; for result in gather_attrs(attrs) { - let v = match result { + let (is_group, lint_level_spans) = match result { Err(span) => { span_err!(self.sess(), span, E0452, "malformed lint attribute"); @@ -729,13 +729,14 @@ pub trait LintContext<'tcx>: Sized { } Ok((lint_name, level, span)) => { match self.lints().find_lint(&lint_name.as_str()) { - Ok(lint_id) => vec![(lint_id, level, span)], + Ok(lint_id) => (false, vec![(lint_id, level, span)]), Err(FindLintError::NotFound) => { match self.lints().lint_groups.get(&*lint_name.as_str()) { - Some(&(ref v, _)) => v.iter() + Some(&(ref v, _)) => (true, + v.iter() .map(|lint_id: &LintId| (*lint_id, level, span)) - .collect(), + .collect()), None => { // The lint or lint group doesn't exist. // This is an error, but it was handled @@ -751,14 +752,18 @@ pub trait LintContext<'tcx>: Sized { let lint_attr_name = result.expect("lint attribute should be well-formed").0; - for (lint_id, level, span) in v { + for (lint_id, level, span) in lint_level_spans { let (now, now_source) = self.lint_sess().get_source(lint_id); if now == Forbid && level != Forbid { - let lint_name = lint_id.to_string(); + let forbidden_lint_name = match now_source { + LintSource::Default => lint_id.to_string(), + LintSource::Node(name, _) => name.to_string(), + LintSource::CommandLine(name) => name.to_string(), + }; let mut diag_builder = struct_span_err!(self.sess(), span, E0453, "{}({}) overruled by outer forbid({})", - level.as_str(), lint_name, - lint_name); + level.as_str(), lint_attr_name, + forbidden_lint_name); diag_builder.span_label(span, "overruled by previous forbid"); match now_source { LintSource::Default => &mut diag_builder, @@ -769,7 +774,10 @@ pub trait LintContext<'tcx>: Sized { LintSource::CommandLine(_) => { diag_builder.note("`forbid` lint level was set on command line") } - }.emit() + }.emit(); + if is_group { // don't set a separate error for every lint in the group + break; + } } else if now != level { let cx = self.lint_sess_mut(); cx.stack.push((lint_id, (now, now_source))); diff --git a/src/test/ui/lint/outer-forbid.rs b/src/test/ui/lint/outer-forbid.rs new file mode 100644 index 0000000000000..d71da58829a72 --- /dev/null +++ b/src/test/ui/lint/outer-forbid.rs @@ -0,0 +1,22 @@ +// Copyright 2017 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Forbidding a group (here, `unused`) overrules subsequent allowance of both +// the group, and an individual lint in the group (here, `unused_variables`); +// and, forbidding an individual lint (here, `non_snake_case`) overrules +// subsequent allowance of a lint group containing it (here, `bad_style`). See +// Issue #42873. + +#![forbid(unused, non_snake_case)] + +#[allow(unused, unused_variables, bad_style)] +fn main() { + println!("hello forbidden world") +} diff --git a/src/test/ui/lint/outer-forbid.stderr b/src/test/ui/lint/outer-forbid.stderr new file mode 100644 index 0000000000000..831b3f65634b2 --- /dev/null +++ b/src/test/ui/lint/outer-forbid.stderr @@ -0,0 +1,29 @@ +error[E0453]: allow(unused) overruled by outer forbid(unused) + --> $DIR/outer-forbid.rs:19:9 + | +17 | #![forbid(unused, non_snake_case)] + | ------ `forbid` level set here +18 | +19 | #[allow(unused, unused_variables, bad_style)] + | ^^^^^^ overruled by previous forbid + +error[E0453]: allow(unused_variables) overruled by outer forbid(unused) + --> $DIR/outer-forbid.rs:19:17 + | +17 | #![forbid(unused, non_snake_case)] + | ------ `forbid` level set here +18 | +19 | #[allow(unused, unused_variables, bad_style)] + | ^^^^^^^^^^^^^^^^ overruled by previous forbid + +error[E0453]: allow(bad_style) overruled by outer forbid(non_snake_case) + --> $DIR/outer-forbid.rs:19:35 + | +17 | #![forbid(unused, non_snake_case)] + | -------------- `forbid` level set here +18 | +19 | #[allow(unused, unused_variables, bad_style)] + | ^^^^^^^^^ overruled by previous forbid + +error: aborting due to previous error(s) +