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.

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

Perform unused assignment and unused variables lints on MIR. #101500

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 1 addition & 3 deletions compiler/rustc_borrowck/src/diagnostics/move_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
// whether or not the right-hand side is a place expression
if let LocalInfo::User(BindingForm::Var(VarBindingForm {
opt_match_place: Some((opt_match_place, match_span)),
binding_mode: _,
opt_ty_info: _,
pat_span: _,
..
})) = *local_decl.local_info()
{
let stmt_source_info = self.body.source_info(location);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
LocalInfo::User(BindingForm::Var(mir::VarBindingForm {
binding_mode: BindingAnnotation(ByRef::No, Mutability::Not),
opt_ty_info: Some(sp),
opt_match_place: _,
pat_span: _,
..
})) => {
if suggest {
err.span_note(sp, "the binding is already a mutable borrow");
Expand Down Expand Up @@ -729,6 +728,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
opt_ty_info: _,
opt_match_place: _,
pat_span,
introductions: _,
})) => pat_span,
_ => local_decl.source_info.span,
};
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,7 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
sess.time("MIR_effect_checking", || {
for def_id in tcx.hir().body_owners() {
tcx.ensure().has_ffi_unwind_calls(def_id);
tcx.ensure().check_liveness(def_id);

// If we need to codegen, ensure that we emit all errors from
// `mir_drops_elaborated_and_const_checked` now, to avoid discovering
Expand Down
16 changes: 7 additions & 9 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,8 @@ pub struct VarBindingForm<'tcx> {
pub opt_match_place: Option<(Option<Place<'tcx>>, Span)>,
/// The span of the pattern in which this variable was bound.
pub pat_span: Span,
/// For each introduction place, record here the span and whether this was a shorthand pattern.
pub introductions: Vec<(Span, /* is_shorthand */ bool)>,
}

#[derive(Clone, Debug, TyEncodable, TyDecodable)]
Expand All @@ -1008,7 +1010,7 @@ pub enum BindingForm<'tcx> {
/// Binding for a `self`/`&self`/`&mut self` binding where the type is implicit.
ImplicitSelf(ImplicitSelfKind),
/// Reference used in a guard expression to ensure immutability.
RefForGuard,
RefForGuard(Local),
}

TrivialTypeTraversalImpls! { BindingForm<'tcx> }
Expand All @@ -1025,7 +1027,7 @@ mod binding_form_impl {
match self {
Var(binding) => binding.hash_stable(hcx, hasher),
ImplicitSelf(kind) => kind.hash_stable(hcx, hasher),
RefForGuard => (),
RefForGuard(local) => local.hash_stable(hcx, hasher),
}
}
}
Expand Down Expand Up @@ -1208,9 +1210,7 @@ impl<'tcx> LocalDecl<'tcx> {
LocalInfo::User(
BindingForm::Var(VarBindingForm {
binding_mode: BindingAnnotation(ByRef::No, _),
opt_ty_info: _,
opt_match_place: _,
pat_span: _,
..
}) | BindingForm::ImplicitSelf(ImplicitSelfKind::Imm),
)
)
Expand All @@ -1225,9 +1225,7 @@ impl<'tcx> LocalDecl<'tcx> {
LocalInfo::User(
BindingForm::Var(VarBindingForm {
binding_mode: BindingAnnotation(ByRef::No, _),
opt_ty_info: _,
opt_match_place: _,
pat_span: _,
..
}) | BindingForm::ImplicitSelf(_),
)
)
Expand All @@ -1244,7 +1242,7 @@ impl<'tcx> LocalDecl<'tcx> {
/// expression that is used to access said variable for the guard of the
/// match arm.
pub fn is_ref_for_guard(&self) -> bool {
matches!(self.local_info(), LocalInfo::User(BindingForm::RefForGuard))
matches!(self.local_info(), LocalInfo::User(BindingForm::RefForGuard(_)))
}

/// Returns `Some` if this is a reference to a static item that is used to
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl<'tcx> StatementKind<'tcx> {
impl<V, T> ProjectionElem<V, T> {
/// Returns `true` if the target of this projection may refer to a different region of memory
/// than the base.
fn is_indirect(&self) -> bool {
pub fn is_indirect(&self) -> bool {
match self {
Self::Deref => true,

Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -943,8 +943,10 @@ rustc_queries! {
desc { |tcx| "checking privacy in {}", describe_as_module(key.to_local_def_id(), tcx) }
}

query check_liveness(key: LocalDefId) {
desc { |tcx| "checking liveness of variables in `{}`", tcx.def_path_str(key) }
query check_liveness(key: LocalDefId) -> &'tcx rustc_index::bit_set::BitSet<abi::FieldIdx> {
arena_cache
desc { |tcx| "checking liveness of variables in `{}`", tcx.def_path_str(key.to_def_id()) }
cache_on_disk_if(tcx) { tcx.is_typeck_child(key.to_def_id()) }
}

/// Return the live symbols in the crate for dead code check.
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/thir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,7 @@ pub enum PatKind<'tcx> {
/// Is this the leftmost occurrence of the binding, i.e., is `var` the
/// `HirId` of this pattern?
is_primary: bool,
is_shorthand: bool,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you document what "shorthand" means?

},

/// `Foo(...)` or `Foo{...}` or `Foo`, where `Foo` is a variant name from an ADT with
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_middle/src/ty/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,8 @@ pub fn place_to_string_for_capture<'tcx>(tcx: TyCtxt<'tcx>, place: &HirPlace<'tc
)
}
},
// Just change the type to the hidden type, so we can actually project.
HirProjectionKind::OpaqueCast => {}
proj => bug!("{:?} unexpected because it isn't captured", proj),
}
}
Expand Down
10 changes: 9 additions & 1 deletion compiler/rustc_mir_build/src/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
block,
node,
span,
false,
OutsideGuard,
true,
);
Expand Down Expand Up @@ -290,7 +291,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
pattern,
UserTypeProjections::none(),
&mut |this, _, _, node, span, _, _| {
this.storage_live_binding(block, node, span, OutsideGuard, true);
this.storage_live_binding(
block,
node,
span,
false,
OutsideGuard,
true,
);
this.schedule_drop_for_binding(node, span, OutsideGuard);
},
)
Expand Down
32 changes: 27 additions & 5 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -627,8 +627,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
subpattern: None,
..
} => {
let place =
self.storage_live_binding(block, var, irrefutable_pat.span, OutsideGuard, true);
let place = self.storage_live_binding(
block,
var,
irrefutable_pat.span,
false,
OutsideGuard,
true,
);
unpack!(block = self.expr_into_dest(place, block, initializer_id));

// Inject a fake read, see comments on `FakeReadCause::ForLet`.
Expand Down Expand Up @@ -661,8 +667,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
},
ascription: thir::Ascription { ref annotation, variance: _ },
} => {
let place =
self.storage_live_binding(block, var, irrefutable_pat.span, OutsideGuard, true);
let place = self.storage_live_binding(
block,
var,
irrefutable_pat.span,
false,
OutsideGuard,
true,
);
unpack!(block = self.expr_into_dest(place, block, initializer_id));

// Inject a fake read, see comments on `FakeReadCause::ForLet`.
Expand Down Expand Up @@ -855,6 +867,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
block: BasicBlock,
var: LocalVarId,
span: Span,
is_shorthand: bool,
for_guard: ForGuard,
schedule_drop: bool,
) -> Place<'tcx> {
Expand All @@ -868,6 +881,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
{
self.schedule_drop(span, region_scope, local_id, DropKind::Storage);
}
let local_info = self.local_decls[local_id].local_info.as_mut().assert_crate_local();
if let LocalInfo::User(BindingForm::Var(var_info)) = &mut **local_info {
var_info.introductions.push((span, is_shorthand));
}
Place::from(local_id)
}

Expand Down Expand Up @@ -1149,6 +1166,7 @@ struct Binding<'tcx> {
source: Place<'tcx>,
var_id: LocalVarId,
binding_mode: BindingAnnotation,
is_shorthand: bool,
}

/// Indicates that the type of `source` must be a subtype of the
Expand Down Expand Up @@ -2332,6 +2350,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
block,
binding.var_id,
binding.span,
binding.is_shorthand,
RefWithinGuard,
schedule_drops,
);
Expand All @@ -2345,6 +2364,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
block,
binding.var_id,
binding.span,
binding.is_shorthand,
OutsideGuard,
schedule_drops,
);
Expand Down Expand Up @@ -2384,6 +2404,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
block,
binding.var_id,
binding.span,
binding.is_shorthand,
OutsideGuard,
schedule_drops,
)
Expand Down Expand Up @@ -2437,6 +2458,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
opt_ty_info: None,
opt_match_place,
pat_span,
introductions: Vec::new(),
},
)))),
};
Expand All @@ -2457,7 +2479,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
user_ty: None,
source_info,
local_info: ClearCrossCrate::Set(Box::new(LocalInfo::User(
BindingForm::RefForGuard,
BindingForm::RefForGuard(for_arm_body),
))),
});
self.var_debug_info.push(VarDebugInfo {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_mir_build/src/build/matches/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,13 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
TestCase::Irrefutable { ascription, binding: None }
}

PatKind::Binding { mode, var, ref subpattern, .. } => {
PatKind::Binding { mode, var, ref subpattern, is_shorthand, .. } => {
let binding = place.map(|source| super::Binding {
span: pattern.span,
source,
var_id: var,
binding_mode: mode,
is_shorthand,
});

if let Some(subpattern) = subpattern.as_ref() {
Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_mir_build/src/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,6 @@ pub(crate) fn mir_build<'tcx>(tcx: TyCtxtAt<'tcx>, def: LocalDefId) -> Body<'tcx
thir::BodyTy::Const(ty) => construct_const(tcx, def, thir, expr, ty),
};

// this must run before MIR dump, because
// "not all control paths return a value" is reported here.
//
// maybe move the check to a MIR pass?
tcx.ensure().check_liveness(def);

// Don't steal here, instead steal in unsafeck. This is so that
// pattern inline constants can be evaluated as part of building the
// THIR of the parent function without a cycle.
Expand Down Expand Up @@ -947,6 +941,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
opt_ty_info: param.ty_span,
opt_match_place: Some((None, span)),
pat_span: span,
introductions: vec![(span, false)],
}))
};
self.var_indices.insert(var, LocalsForNode::One(local));
Expand Down
11 changes: 8 additions & 3 deletions compiler/rustc_mir_build/src/thir/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
ty: var_ty,
subpattern: self.lower_opt_pattern(sub),
is_primary: id == pat.hir_id,
is_shorthand: false,
}
}

Expand All @@ -328,9 +329,13 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
let res = self.typeck_results.qpath_res(qpath, pat.hir_id);
let subpatterns = fields
.iter()
.map(|field| FieldPat {
field: self.typeck_results.field_index(field.hir_id),
pattern: self.lower_pattern(field.pat),
.map(|field| {
let mut pattern = self.lower_pattern(field.pat);
if let PatKind::Binding { ref mut is_shorthand, .. } = pattern.kind {
*is_shorthand = field.is_shorthand;
}
let field = self.typeck_results.field_index(field.hir_id);
FieldPat { field, pattern }
})
.collect();

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_mir_build/src/thir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,13 +635,14 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> {
self.print_pat(subpattern, depth_lvl + 3);
print_indented!(self, "}", depth_lvl + 1);
}
PatKind::Binding { name, mode, var, ty, subpattern, is_primary } => {
PatKind::Binding { name, mode, var, ty, subpattern, is_primary, is_shorthand } => {
print_indented!(self, "Binding {", depth_lvl + 1);
print_indented!(self, format!("name: {:?}", name), depth_lvl + 2);
print_indented!(self, format!("mode: {:?}", mode), depth_lvl + 2);
print_indented!(self, format!("var: {:?}", var), depth_lvl + 2);
print_indented!(self, format!("ty: {:?}", ty), depth_lvl + 2);
print_indented!(self, format!("is_primary: {:?}", is_primary), depth_lvl + 2);
print_indented!(self, format!("is_shorthand: {:?}", is_shorthand), depth_lvl + 2);

if let Some(subpattern) = subpattern {
print_indented!(self, "subpattern: Some( ", depth_lvl + 2);
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_mir_dataflow/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ pub use self::drop_flag_effects::{
drop_flag_effects_for_function_entry, drop_flag_effects_for_location,
move_path_children_matching, on_all_children_bits, on_lookup_result_bits,
};
use self::framework::SwitchIntEdgeEffects;
pub use self::framework::{
fmt, graphviz, lattice, visit_results, Analysis, AnalysisDomain, Backward, Direction, Engine,
Forward, GenKill, GenKillAnalysis, JoinSemiLattice, MaybeReachable, Results, ResultsCursor,
ResultsVisitable, ResultsVisitor, SwitchIntEdgeEffects,
ResultsVisitable, ResultsVisitor,
};
use self::move_paths::MoveData;

Expand Down
24 changes: 24 additions & 0 deletions compiler/rustc_mir_transform/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,37 @@ mir_transform_ffi_unwind_call = call to {$foreign ->
mir_transform_fn_item_ref = taking a reference to a function item does not give a function pointer
.suggestion = cast `{$ident}` to obtain a function pointer

mir_transform_maybe_string_interpolation = you might have meant to use string interpolation in this string literal

mir_transform_must_not_suspend = {$pre}`{$def_path}`{$post} held across a suspend point, but should not be
.label = the value is held across this suspend point
.note = {$reason}
.help = consider using a block (`{"{ ... }"}`) to shrink the value's scope, ending before the suspend point
mir_transform_operation_will_panic = this operation will panic at runtime

mir_transform_string_interpolation_only_works = string interpolation only works in `format!` invocations

mir_transform_unaligned_packed_ref = reference to packed field is unaligned
.note = packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
.note_ub = creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
.help = copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

mir_transform_unused_assign = value assigned to `{$name}` is never read
.help = maybe it is overwritten before being read?

mir_transform_unused_assign_passed = value passed to `{$name}` is never read
.help = maybe it is overwritten before being read?

mir_transform_unused_capture_maybe_capture_ref = value captured by `{$name}` is never read
.help = did you mean to capture by reference instead?

mir_transform_unused_var_assigned_only = variable `{$name}` is assigned to, but never used
.note = consider using `_{$name}` instead

mir_transform_unused_var_underscore = if this is intentional, prefix it with an underscore

mir_transform_unused_variable = unused variable: `{$name}`

mir_transform_unused_variable_args_in_macro = `{$name}` is captured in macro and introduced a unused variable

mir_transform_unused_variable_try_ignore = try ignoring the field
Loading
Loading