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

Fix how allow/warn/deny/forbid warnings is handled #85298

Closed
wants to merge 4 commits into from
Closed

Fix how allow/warn/deny/forbid warnings is handled #85298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
440b639
Check whether -F is ever overridden on the command line
FabianWolff May 13, 2021
034f1fb
Fix handling of allow/deny/forbid(warnings)
FabianWolff May 14, 2021
80ae9ef
Fix test case failure in ui-fulldeps suite
FabianWolff May 14, 2021
88ff8f8
Fix compilation errors after rebase
FabianWolff Jun 4, 2021
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
14 changes: 13 additions & 1 deletion compiler/rustc_errors/src/diagnostic_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ struct DiagnosticBuilderInner<'a> {
handler: &'a Handler,
diagnostic: Diagnostic,
allow_suggestions: bool,
is_lint: bool,
}

/// In general, the `DiagnosticBuilder` uses deref to allow access to
Expand Down Expand Up @@ -100,7 +101,7 @@ impl<'a> DerefMut for DiagnosticBuilder<'a> {
impl<'a> DiagnosticBuilder<'a> {
/// Emit the diagnostic.
pub fn emit(&mut self) {
self.0.handler.emit_diagnostic(&self);
self.0.handler.emit_diagnostic_with_ignore(&self, self.0.is_lint);
self.cancel();
}

Expand Down Expand Up @@ -373,6 +374,16 @@ impl<'a> DiagnosticBuilder<'a> {
self
}

/// Set by LintDiagnosticBuilder to distinguish lint warnings from other
/// warnings. Necessary because `-A warnings` on the command line turns
/// off all regular warnings, but lint warnings can be turned on again
/// with #[warn(...)].
pub fn is_lint(&mut self, value: bool) -> &mut Self {
debug!("is_lint({:?})", value);
self.0.is_lint = value;
self
}

/// Convenience function for internal use, clients should use one of the
/// `struct_*` methods on [`Handler`].
crate fn new(handler: &'a Handler, level: Level, message: &str) -> DiagnosticBuilder<'a> {
Expand All @@ -399,6 +410,7 @@ impl<'a> DiagnosticBuilder<'a> {
handler,
diagnostic,
allow_suggestions: true,
is_lint: false,
}))
}
}
Expand Down
25 changes: 19 additions & 6 deletions compiler/rustc_errors/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,11 +553,7 @@ impl Handler {

/// Construct a builder at the `Warning` level with the `msg`.
pub fn struct_warn(&self, msg: &str) -> DiagnosticBuilder<'_> {
let mut result = DiagnosticBuilder::new(self, Level::Warning, msg);
if !self.flags.can_emit_warnings {
result.cancel();
}
result
DiagnosticBuilder::new(self, Level::Warning, msg)
}

/// Construct a builder at the `Allow` level with the `msg`.
Expand Down Expand Up @@ -752,6 +748,14 @@ impl Handler {
self.inner.borrow_mut().force_print_diagnostic(db)
}

pub fn emit_diagnostic_with_ignore(
&self,
diagnostic: &Diagnostic,
ignore_can_emit_warnings: bool,
) {
self.inner.borrow_mut().emit_diagnostic_with_ignore(diagnostic, ignore_can_emit_warnings)
}

pub fn emit_diagnostic(&self, diagnostic: &Diagnostic) {
self.inner.borrow_mut().emit_diagnostic(diagnostic)
}
Expand Down Expand Up @@ -794,6 +798,14 @@ impl HandlerInner {
}

fn emit_diagnostic(&mut self, diagnostic: &Diagnostic) {
self.emit_diagnostic_with_ignore(diagnostic, false);
}

fn emit_diagnostic_with_ignore(
&mut self,
diagnostic: &Diagnostic,
ignore_can_emit_warnings: bool,
) {
if diagnostic.cancelled() {
return;
}
Expand All @@ -802,7 +814,8 @@ impl HandlerInner {
self.future_breakage_diagnostics.push(diagnostic.clone());
}

if diagnostic.level == Warning && !self.flags.can_emit_warnings {
if diagnostic.level == Warning && !self.flags.can_emit_warnings && !ignore_can_emit_warnings
{
if diagnostic.has_future_breakage() {
(*TRACK_DIAGNOSTICS)(diagnostic);
}
Expand Down
91 changes: 70 additions & 21 deletions compiler/rustc_lint/src/levels.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ impl<'s> LintLevelsBuilder<'s> {
}

fn process_command_line(&mut self, sess: &Session, store: &LintStore) {
let mut specs = FxHashMap::default();
self.sets.list.push(LintSet::CommandLine { specs: FxHashMap::default() });
self.sets.lint_cap = sess.opts.lint_cap.unwrap_or(Level::Forbid);

for &(ref lint_name, level) in &sess.opts.lint_opts {
for (position, &(ref lint_name, level)) in (0u32..).zip(sess.opts.lint_opts.iter()) {
store.check_lint_name_cmdline(sess, &lint_name, Some(level));
let orig_level = level;

Expand All @@ -97,15 +97,26 @@ impl<'s> LintLevelsBuilder<'s> {
let level = cmp::min(level, self.sets.lint_cap);

let lint_flag_val = Symbol::intern(lint_name);
let src = LintLevelSource::CommandLine(lint_flag_val, orig_level, position);

let ids = match store.find_lints(&lint_name) {
Ok(ids) => ids,
Err(_) => continue, // errors handled in check_lint_name_cmdline above
};

for id in ids {
self.check_gated_lint(id, DUMMY_SP);
let src = LintLevelSource::CommandLine(lint_flag_val, orig_level);
specs.insert(id, (level, src));
let specs = match self.sets.list.last().unwrap() {
LintSet::CommandLine { ref specs } => specs,
_ => unreachable!(),
};
if self.check_before_insert_spec(specs, id, (level, src)) {
let specs = match self.sets.list.last_mut().unwrap() {
LintSet::CommandLine { ref mut specs } => specs,
_ => unreachable!(),
};
specs.insert(id, (level, src));
}
}
}

Expand All @@ -118,19 +129,30 @@ impl<'s> LintLevelsBuilder<'s> {
self.sets.force_warns.extend(&lints);
}
}
}

self.sets.list.push(LintSet::CommandLine { specs });
fn get_current_depth(&self) -> u32 {
let mut idx = self.cur;
let mut depth = 0;
loop {
match &self.sets.list[idx as usize] {
LintSet::CommandLine { .. } => {
return depth;
}
LintSet::Node { parent, .. } => {
depth += 1;
idx = *parent;
}
}
}
}

/// Attempts to insert the `id` to `level_src` map entry. If unsuccessful
/// (e.g. if a forbid was already inserted on the same scope), then emits a
/// diagnostic with no change to `specs`.
fn insert_spec(
&mut self,
specs: &mut FxHashMap<LintId, LevelAndSource>,
fn check_before_insert_spec(
&self,
specs: &FxHashMap<LintId, LevelAndSource>,
id: LintId,
(level, src): LevelAndSource,
) {
) -> bool {
// 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`.
Expand All @@ -150,8 +172,8 @@ impl<'s> LintLevelsBuilder<'s> {
let id_name = id.lint.name_lower();
let fcw_warning = match old_src {
LintLevelSource::Default => false,
LintLevelSource::Node(symbol, _, _) => self.store.is_lint_group(symbol),
LintLevelSource::CommandLine(symbol, _) => self.store.is_lint_group(symbol),
LintLevelSource::Node(symbol, ..) => self.store.is_lint_group(symbol),
LintLevelSource::CommandLine(symbol, ..) => self.store.is_lint_group(symbol),
LintLevelSource::ForceWarn(_symbol) => {
bug!("forced warn lint returned a forbid lint level")
}
Expand All @@ -170,13 +192,13 @@ impl<'s> LintLevelsBuilder<'s> {
id.to_string()
));
}
LintLevelSource::Node(_, forbid_source_span, reason) => {
LintLevelSource::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());
}
}
LintLevelSource::CommandLine(_, _) => {
LintLevelSource::CommandLine(..) => {
diag_builder.note("`forbid` lint level was set on command line");
}
_ => bug!("forced warn lint returned a forbid lint level"),
Expand Down Expand Up @@ -212,11 +234,25 @@ impl<'s> LintLevelsBuilder<'s> {
// issuing a FCW. In the FCW case, we want to
// respect the new setting.
if !fcw_warning {
return;
return false;
}
}
}
specs.insert(id, (level, src));
true
}

/// Attempts to insert the `id` to `level_src` map entry. If unsuccessful
/// (e.g. if a forbid was already inserted on the same scope), then emits a
/// diagnostic with no change to `specs`.
fn insert_spec(
&mut self,
specs: &mut FxHashMap<LintId, LevelAndSource>,
id: LintId,
(level, src): LevelAndSource,
) {
if self.check_before_insert_spec(specs, id, (level, src)) {
specs.insert(id, (level, src));
}
}

/// Pushes a list of AST lint attributes onto this context.
Expand All @@ -242,7 +278,7 @@ impl<'s> LintLevelsBuilder<'s> {
let mut specs = FxHashMap::default();
let sess = self.sess;
let bad_attr = |span| struct_span_err!(sess, span, E0452, "malformed lint attribute input");
for attr in attrs {
for (attr_pos, attr) in (0u32..).zip(attrs.iter()) {
let level = match Level::from_symbol(attr.name_or_empty()) {
None => continue,
Some(lvl) => lvl,
Expand Down Expand Up @@ -353,7 +389,10 @@ impl<'s> LintLevelsBuilder<'s> {
meta_item.path.segments.last().expect("empty lint name").ident.name,
sp,
reason,
self.get_current_depth(),
attr_pos,
);

for &id in *ids {
self.check_gated_lint(id, attr.span);
self.insert_spec(&mut specs, id, (level, src));
Expand All @@ -368,6 +407,8 @@ impl<'s> LintLevelsBuilder<'s> {
Symbol::intern(complete_name),
sp,
reason,
self.get_current_depth(),
attr_pos,
);
for id in ids {
self.insert_spec(&mut specs, *id, (level, src));
Expand Down Expand Up @@ -404,6 +445,8 @@ impl<'s> LintLevelsBuilder<'s> {
Symbol::intern(&new_lint_name),
sp,
reason,
self.get_current_depth(),
attr_pos,
);
for id in ids {
self.insert_spec(&mut specs, *id, (level, src));
Expand Down Expand Up @@ -474,7 +517,13 @@ impl<'s> LintLevelsBuilder<'s> {
// Ignore any errors or warnings that happen because the new name is inaccurate
// NOTE: `new_name` already includes the tool name, so we don't have to add it again.
if let CheckLintNameResult::Ok(ids) = store.check_lint_name(&new_name, None) {
let src = LintLevelSource::Node(Symbol::intern(&new_name), sp, reason);
let src = LintLevelSource::Node(
Symbol::intern(&new_name),
sp,
reason,
self.get_current_depth(),
attr_pos,
);
for &id in ids {
self.check_gated_lint(id, attr.span);
self.insert_spec(&mut specs, id, (level, src));
Expand All @@ -493,7 +542,7 @@ impl<'s> LintLevelsBuilder<'s> {
}

let (lint_attr_name, lint_attr_span) = match *src {
LintLevelSource::Node(name, span, _) => (name, span),
LintLevelSource::Node(name, span, _, _, _) => (name, span),
_ => continue,
};

Expand Down
Loading