diff options
| author | Mark Rousskov <mark.simulacrum@gmail.com> | 2020-11-07 18:14:38 -0500 |
|---|---|---|
| committer | Mark Rousskov <mark.simulacrum@gmail.com> | 2020-11-14 15:56:07 -0500 |
| commit | 64efcbe0e9e0e2dc02960031e6df4fe7450782ca (patch) | |
| tree | 77db9d6b4940a2c4e2b83f395580a7739e0e9505 /compiler/rustc_lint/src | |
| parent | cf9cf7c923eb01146971429044f216a3ca905e06 (diff) | |
Use true previous lint level when detecting overriden forbids
Previously, cap-lints was ignored when checking the previous forbid level, which meant that it was a hard error to do so. This is different from the normal behavior of lints, which are silenced by cap-lints; if the forbid would not take effect regardless, there is not much point in complaining about the fact that we are reducing its level. It might be considered a bug that even `--cap-lints deny` would suffice to silence the error on overriding forbid, depending on if one cares about failing the build or precisely forbid being set. But setting cap-lints to deny is quite odd and not really done in practice, so we don't try to handle it specially. This also unifies the code paths for nested and same-level scopes. However, the special case for CLI lint flags is left in place (introduced by #70918) to fix the regression noted in #70819. That means that CLI flags do not lint on forbid being overridden by a non-forbid level. It is unclear whether this is a bug or a desirable feature, but it is certainly inconsistent. CLI flags are a sufficiently different "type" of place though that this is deemed out of scope for this commit.
Diffstat (limited to 'compiler/rustc_lint/src')
| -rw-r--r-- | compiler/rustc_lint/src/levels.rs | 70 |
1 files changed, 21 insertions, 49 deletions
diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index aca28988364..d2a5875cb2f 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -106,18 +106,32 @@ impl<'s> LintLevelsBuilder<'s> { id: LintId, (level, src): LevelSource, ) { - if let Some((old_level, old_src)) = specs.get(&id) { - if old_level == &Level::Forbid && level != Level::Forbid { + // Setting to a non-forbid level is an error if the lint previously had + // a forbid level. Note that this is not necessarily true even with a + // `#[forbid(..)]` attribute present, as that is overriden by `--cap-lints`. + // + // This means that this only errors if we're truly lowering the lint + // level from forbid. + if level != Level::Forbid { + if let (Level::Forbid, old_src) = + self.sets.get_lint_level(id.lint, self.cur, Some(&specs), &self.sess) + { let mut diag_builder = struct_span_err!( self.sess, src.span(), E0453, - "{}({}) incompatible with previous forbid in same scope", + "{}({}) incompatible with previous forbid", level.as_str(), src.name(), ); - match *old_src { - LintSource::Default => {} + diag_builder.span_label(src.span(), "overruled by previous forbid"); + match old_src { + LintSource::Default => { + diag_builder.note(&format!( + "`forbid` lint level is the default for {}", + id.to_string() + )); + } LintSource::Node(_, forbid_source_span, reason) => { diag_builder.span_label(forbid_source_span, "`forbid` level set here"); if let Some(rationale) = reason { @@ -129,6 +143,8 @@ impl<'s> LintLevelsBuilder<'s> { } } diag_builder.emit(); + + // Retain the forbid lint level return; } } @@ -412,50 +428,6 @@ impl<'s> LintLevelsBuilder<'s> { } } - for (id, &(level, ref src)) in specs.iter() { - if level == Level::Forbid { - continue; - } - let forbid_src = match self.sets.get_lint_id_level(*id, self.cur, None) { - (Some(Level::Forbid), src) => src, - _ => continue, - }; - let forbidden_lint_name = match forbid_src { - LintSource::Default => id.to_string(), - LintSource::Node(name, _, _) => name.to_string(), - LintSource::CommandLine(name, _) => name.to_string(), - }; - let (lint_attr_name, lint_attr_span) = match *src { - LintSource::Node(name, span, _) => (name, span), - _ => continue, - }; - let mut diag_builder = struct_span_err!( - self.sess, - lint_attr_span, - E0453, - "{}({}) overruled by outer forbid({})", - level.as_str(), - lint_attr_name, - forbidden_lint_name - ); - diag_builder.span_label(lint_attr_span, "overruled by previous forbid"); - match forbid_src { - LintSource::Default => {} - LintSource::Node(_, forbid_source_span, reason) => { - diag_builder.span_label(forbid_source_span, "`forbid` level set here"); - if let Some(rationale) = reason { - diag_builder.note(&rationale.as_str()); - } - } - LintSource::CommandLine(_, _) => { - diag_builder.note("`forbid` lint level was set on command line"); - } - } - diag_builder.emit(); - // don't set a separate error for every lint in the group - break; - } - let prev = self.cur; if !specs.is_empty() { self.cur = self.sets.list.len() as u32; |
