Skip to content

Commit

Permalink
Suppress unused_mut if unused_variables is reported
Browse files Browse the repository at this point in the history
  • Loading branch information
sanxiyn committed Sep 18, 2022
1 parent bc7b17c commit 50f8558
Show file tree
Hide file tree
Showing 12 changed files with 176 additions and 133 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/consumers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ pub fn get_body_with_borrowck_facts<'tcx>(
tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bind(def.did)).enter(|infcx| {
let input_body: &Body<'_> = &input_body.borrow();
let promoted: &IndexVec<_, _> = &promoted.borrow();
*super::do_mir_borrowck(&infcx, input_body, promoted, true).1.unwrap()
*super::do_mir_borrowck(&infcx, input_body, promoted, &None, true).1.unwrap()
})
}
21 changes: 18 additions & 3 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ extern crate rustc_middle;
#[macro_use]
extern crate tracing;

use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
use rustc_data_structures::graph::dominators::Dominators;
use rustc_errors::{Diagnostic, DiagnosticBuilder, ErrorGuaranteed};
use rustc_hir as hir;
Expand Down Expand Up @@ -129,16 +129,22 @@ fn mir_borrowck<'tcx>(
def: ty::WithOptConstParam<LocalDefId>,
) -> &'tcx BorrowCheckResult<'tcx> {
let (input_body, promoted) = tcx.mir_promoted(def);
debug!("run query mir_borrowck: {}", tcx.def_path_str(def.did.to_def_id()));

let def_id = def.did.to_def_id();
debug!("run query mir_borrowck: {}", tcx.def_path_str(def_id));

let hir_owner = tcx.hir().local_def_id_to_hir_id(def.did).owner;

let is_fn = matches!(tcx.hir().body_owner_kind(def.did), hir::BodyOwnerKind::Fn);
let unused_variables_spans = if is_fn { tcx.check_liveness(def_id) } else { &None };

let opt_closure_req = tcx
.infer_ctxt()
.with_opaque_type_inference(DefiningAnchor::Bind(hir_owner))
.enter(|infcx| {
let input_body: &Body<'_> = &input_body.borrow();
let promoted: &IndexVec<_, _> = &promoted.borrow();
do_mir_borrowck(&infcx, input_body, promoted, false).0
do_mir_borrowck(&infcx, input_body, promoted, unused_variables_spans, false).0
});
debug!("mir_borrowck done");

Expand All @@ -155,6 +161,7 @@ fn do_mir_borrowck<'a, 'tcx>(
infcx: &InferCtxt<'a, 'tcx>,
input_body: &Body<'tcx>,
input_promoted: &IndexVec<Promoted, Body<'tcx>>,
unused_variables_spans: &Option<FxIndexSet<Span>>,
return_body_with_facts: bool,
) -> (BorrowCheckResult<'tcx>, Option<Box<BodyWithBorrowckFacts<'tcx>>>) {
let def = input_body.source.with_opt_param().as_local().unwrap();
Expand Down Expand Up @@ -428,6 +435,14 @@ fn do_mir_borrowck<'a, 'tcx>(
}

let mut_span = tcx.sess.source_map().span_until_non_whitespace(span);
let ident_span = span.with_lo(mut_span.hi());

// Suppress lints if we already reported unused variables
if let Some(unused_variables_spans) = unused_variables_spans {
if unused_variables_spans.contains(&ident_span) {
continue;
}
}

tcx.emit_spanned_lint(UNUSED_MUT, lint_root, span, VarNeedNotMut { span: mut_span })
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,8 @@ rustc_queries! {
desc { |tcx| "checking privacy in {}", describe_as_module(key, tcx) }
}

query check_liveness(key: DefId) {
query check_liveness(key: DefId) -> Option<FxIndexSet<Span>> {
arena_cache
desc { |tcx| "checking liveness of variables in {}", tcx.def_path_str(key) }
}

Expand Down
52 changes: 34 additions & 18 deletions compiler/rustc_passes/src/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ use self::LiveNodeKind::*;
use self::VarKind::*;

use rustc_ast::InlineAsmOptions;
use rustc_data_structures::fx::FxIndexMap;
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::def::*;
Expand All @@ -99,6 +99,7 @@ use rustc_session::lint;
use rustc_span::symbol::{kw, sym, Symbol};
use rustc_span::{BytePos, Span};

use std::cell::RefCell;
use std::collections::VecDeque;
use std::io;
use std::io::prelude::*;
Expand Down Expand Up @@ -138,9 +139,9 @@ fn live_node_kind_to_string(lnk: LiveNodeKind, tcx: TyCtxt<'_>) -> String {
}
}

fn check_liveness(tcx: TyCtxt<'_>, def_id: DefId) {
fn check_liveness(tcx: TyCtxt<'_>, def_id: DefId) -> Option<FxIndexSet<Span>> {
let local_def_id = match def_id.as_local() {
None => return,
None => return None,
Some(def_id) => def_id,
};

Expand All @@ -149,12 +150,12 @@ fn check_liveness(tcx: TyCtxt<'_>, def_id: DefId) {
if let DefKind::Impl = tcx.def_kind(parent)
&& tcx.has_attr(parent.to_def_id(), sym::automatically_derived)
{
return;
return None;
}

// Don't run unused pass for #[naked]
if tcx.has_attr(def_id, sym::naked) {
return;
return None;
}

let mut maps = IrMaps::new(tcx);
Expand Down Expand Up @@ -182,6 +183,8 @@ fn check_liveness(tcx: TyCtxt<'_>, def_id: DefId) {
lsets.visit_body(body);
lsets.warn_about_unused_upvars(entry_ln);
lsets.warn_about_unused_args(body, entry_ln);

Some(lsets.unused_variables_spans.into_inner())
}

pub fn provide(providers: &mut Providers) {
Expand Down Expand Up @@ -517,6 +520,8 @@ struct Liveness<'a, 'tcx> {
// it probably doesn't now)
break_ln: HirIdMap<LiveNode>,
cont_ln: HirIdMap<LiveNode>,

unused_variables_spans: RefCell<FxIndexSet<Span>>,
}

impl<'a, 'tcx> Liveness<'a, 'tcx> {
Expand All @@ -541,6 +546,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
exit_ln,
break_ln: Default::default(),
cont_ln: Default::default(),
unused_variables_spans: Default::default(),
}
}

Expand Down Expand Up @@ -1504,6 +1510,7 @@ impl<'tcx> Liveness<'_, 'tcx> {
}
} else {
if let Some(name) = self.should_warn(var) {
self.unused_variables_spans.borrow_mut().insert(span);
self.ir.tcx.struct_span_lint_hir(
lint::builtin::UNUSED_VARIABLES,
var_hir_id,
Expand Down Expand Up @@ -1594,24 +1601,29 @@ impl<'tcx> Liveness<'_, 'tcx> {
if ln == self.exit_ln { false } else { self.assigned_on_exit(ln, var) };

if is_assigned {
let spans = hir_ids_and_spans
.into_iter()
.map(|(_, _, ident_span)| ident_span)
.collect::<Vec<_>>();
self.unused_variables_spans.borrow_mut().extend(&spans);
self.ir.tcx.struct_span_lint_hir(
lint::builtin::UNUSED_VARIABLES,
first_hir_id,
hir_ids_and_spans
.into_iter()
.map(|(_, _, ident_span)| ident_span)
.collect::<Vec<_>>(),
spans,
|lint| {
lint.build(&format!("variable `{}` is assigned to, but never used", name))
.note(&format!("consider using `_{}` instead", name))
.emit();
},
)
} else if can_remove {
let spans =
hir_ids_and_spans.iter().map(|(_, pat_span, _)| *pat_span).collect::<Vec<_>>();
self.unused_variables_spans.borrow_mut().extend(&spans);
self.ir.tcx.struct_span_lint_hir(
lint::builtin::UNUSED_VARIABLES,
first_hir_id,
hir_ids_and_spans.iter().map(|(_, pat_span, _)| *pat_span).collect::<Vec<_>>(),
spans,
|lint| {
let mut err = lint.build(&format!("unused variable: `{}`", name));
err.multipart_suggestion(
Expand Down Expand Up @@ -1654,13 +1666,15 @@ impl<'tcx> Liveness<'_, 'tcx> {
)
.collect::<Vec<_>>();

let spans = hir_ids_and_spans
.iter()
.map(|(_, pat_span, _)| *pat_span)
.collect::<Vec<_>>();
self.unused_variables_spans.borrow_mut().extend(&spans);
self.ir.tcx.struct_span_lint_hir(
lint::builtin::UNUSED_VARIABLES,
first_hir_id,
hir_ids_and_spans
.iter()
.map(|(_, pat_span, _)| *pat_span)
.collect::<Vec<_>>(),
spans,
|lint| {
let mut err = lint.build(&format!("unused variable: `{}`", name));
err.multipart_suggestion(
Expand All @@ -1677,13 +1691,15 @@ impl<'tcx> Liveness<'_, 'tcx> {
.map(|(_, _, ident_span)| (ident_span, format!("_{}", name)))
.collect::<Vec<_>>();

let spans = hir_ids_and_spans
.iter()
.map(|(_, _, ident_span)| *ident_span)
.collect::<Vec<_>>();
self.unused_variables_spans.borrow_mut().extend(&spans);
self.ir.tcx.struct_span_lint_hir(
lint::builtin::UNUSED_VARIABLES,
first_hir_id,
hir_ids_and_spans
.iter()
.map(|(_, _, ident_span)| *ident_span)
.collect::<Vec<_>>(),
spans,
|lint| {
let mut err = lint.build(&format!("unused variable: `{}`", name));
if self.has_added_lit_match_name_span(&name, opt_body, &mut err) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,22 @@ mod foo {
}

#[expect(
unused_mut,
unused_variables,
//~^ WARNING this lint expectation is unfulfilled [unfulfilled_lint_expectations]
//~| NOTE this `expect` is overridden by a `warn` attribute before the `unused_mut` lint is triggered
reason = "this `expect` is overridden by a `warn` attribute before the `unused_mut` lint is triggered"
//~| NOTE this `expect` is overridden by a `warn` attribute before the `unused_variables` lint is triggered
reason = "this `expect` is overridden by a `warn` attribute before the `unused_variables` lint is triggered"
)]
mod oof {
#[warn(
unused_mut,
unused_variables,
//~^ NOTE the lint level is defined here
reason = "this overrides the previous `expect` lint level and warns about the `unused_mut` lint here"
reason = "this overrides the previous `expect` lint level and warns about the `unused_variables` lint here"
)]
fn bar() {
let mut v = 0;
//~^ WARNING variable does not need to be mutable [unused_mut]
//~| NOTE this overrides the previous `expect` lint level and warns about the `unused_mut` lint here
//~| HELP remove this `mut`
//~^ WARNING unused variable: `v` [unused_variables]
//~| NOTE this overrides the previous `expect` lint level and warns about the `unused_variables` lint here
//~| HELP if this is intentional, prefix it with an underscore
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
warning: unused variable: `v`
--> $DIR/expect_nested_lint_levels.rs:36:17
|
LL | let mut v = 0;
| ^ help: if this is intentional, prefix it with an underscore: `_v`
|
= note: this overrides the previous `expect` lint level and warns about the `unused_variables` lint here
note: the lint level is defined here
--> $DIR/expect_nested_lint_levels.rs:31:9
|
LL | unused_variables,
| ^^^^^^^^^^^^^^^^

error: unused variable: `this_is_my_function`
--> $DIR/expect_nested_lint_levels.rs:48:9
|
Expand All @@ -10,21 +23,6 @@ note: the lint level is defined here
LL | #[forbid(unused_variables)]
| ^^^^^^^^^^^^^^^^

warning: variable does not need to be mutable
--> $DIR/expect_nested_lint_levels.rs:36:13
|
LL | let mut v = 0;
| ----^
| |
| help: remove this `mut`
|
= note: this overrides the previous `expect` lint level and warns about the `unused_mut` lint here
note: the lint level is defined here
--> $DIR/expect_nested_lint_levels.rs:31:9
|
LL | unused_mut,
| ^^^^^^^^^^

warning: this lint expectation is unfulfilled
--> $DIR/expect_nested_lint_levels.rs:7:5
|
Expand All @@ -37,10 +35,10 @@ LL | unused_mut,
warning: this lint expectation is unfulfilled
--> $DIR/expect_nested_lint_levels.rs:24:5
|
LL | unused_mut,
| ^^^^^^^^^^
LL | unused_variables,
| ^^^^^^^^^^^^^^^^
|
= note: this `expect` is overridden by a `warn` attribute before the `unused_mut` lint is triggered
= note: this `expect` is overridden by a `warn` attribute before the `unused_variables` lint is triggered

warning: this lint expectation is unfulfilled
--> $DIR/expect_nested_lint_levels.rs:43:10
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,10 @@ fn main() {

let mut mut_unused_var = 1;
//~^ WARNING unused variable: `mut_unused_var`
//~| WARNING variable does not need to be mutable

let (mut var, unused_var) = (1, 2);
//~^ WARNING unused variable: `var`
//~| WARNING unused variable: `unused_var`
//~| WARNING variable does not need to be mutable
// NOTE: `var` comes after `unused_var` lexicographically yet the warning
// for `var` will be emitted before the one for `unused_var`. We use an
// `IndexMap` to ensure this is the case instead of a `BTreeMap`.
Expand Down
Loading

0 comments on commit 50f8558

Please sign in to comment.