From 7d840de70e5e762dc7a9a2d3fb022b7cc7973e43 Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Wed, 29 Apr 2020 18:19:34 +0200 Subject: [PATCH 1/2] Emit warning when assigning to a temporary variable --- src/librustc_typeck/check/expr.rs | 51 +++++++++++++++++++++++++++++++ src/librustc_typeck/check/op.rs | 1 + 2 files changed, 52 insertions(+) diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index d287589789e2d..51176f71e4ff5 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -35,6 +35,7 @@ use rustc_middle::ty::adjustment::{ use rustc_middle::ty::Ty; use rustc_middle::ty::TypeFoldable; use rustc_middle::ty::{AdtKind, Visibility}; +use rustc_session::lint::builtin::UNUSED_ASSIGNMENTS; use rustc_span::hygiene::DesugaringKind; use rustc_span::source_map::Span; use rustc_span::symbol::{kw, sym, Symbol}; @@ -800,6 +801,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.require_type_is_sized(lhs_ty, lhs.span, traits::AssignmentLhsSized); + self.check_unused_assign_to_field(lhs, span); + if lhs_ty.references_error() || rhs_ty.references_error() { self.tcx.types.err } else { @@ -807,6 +810,54 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + /// Check the LHS for unused assignments to fields, when they aren't bound to a local variable + pub(crate) fn check_unused_assign_to_field( + &self, + lhs: &'tcx hir::Expr<'tcx>, + expr_span: &Span, + ) { + debug!("check_unused_assign(lhs = {:?})", &lhs); + match lhs.kind { + ExprKind::Struct(..) | ExprKind::Tup(..) => { + self.tcx.struct_span_lint_hir(UNUSED_ASSIGNMENTS, lhs.hir_id, *expr_span, |lint| { + lint.build("unused assignement to temporary") + .span_label(lhs.span, "this is not bound to any variable") + .emit(); + }) + } + // Assigning to a field of a const is useless, since the constant will + // get evaluated and injected into the expression + ExprKind::Path(QPath::Resolved( + _, + hir::Path { res: Res::Def(DefKind::Const, _), ref segments, .. }, + )) => { + let const_name = segments.last().unwrap().ident.as_str(); + let mut path_str = String::new(); + for (i, seg) in segments.iter().enumerate() { + if i > 0 { + path_str.push_str("::"); + } + if seg.ident.name != kw::PathRoot { + path_str.push_str(&seg.ident.as_str()); + } + } + + self.tcx.struct_span_lint_hir(UNUSED_ASSIGNMENTS, lhs.hir_id, *expr_span, |lint| { + lint.build("unused assignement to temporary") + .span_label(lhs.span, "this is not bound to any variable") + .note("in Rust, constants are not associated with a specific memory location, and are inlined wherever they are used") + .span_suggestion( + lhs.span.shrink_to_lo(), + "consider introducing a local variable", + format!("let mut {} = {};", (&*const_name).to_lowercase(), path_str), Applicability::MaybeIncorrect) + .emit(); + }) + } + ExprKind::Field(ref base, _) => self.check_unused_assign_to_field(base, expr_span), + _ => {} + } + } + fn check_expr_loop( &self, body: &'tcx hir::Block<'tcx>, diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index cac9113fd5d30..38e7a08afdd05 100644 --- a/src/librustc_typeck/check/op.rs +++ b/src/librustc_typeck/check/op.rs @@ -35,6 +35,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }; self.check_lhs_assignable(lhs, "E0067", &op.span); + self.check_unused_assign_to_field(lhs, &op.span); ty } From 0729e9fc87975aca367c9513a2af368ac4295e87 Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Wed, 29 Apr 2020 18:19:55 +0200 Subject: [PATCH 2/2] Add UI tests --- src/test/ui/unused/unused-field-assign.rs | 34 ++++++++ src/test/ui/unused/unused-field-assign.stderr | 84 +++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 src/test/ui/unused/unused-field-assign.rs create mode 100644 src/test/ui/unused/unused-field-assign.stderr diff --git a/src/test/ui/unused/unused-field-assign.rs b/src/test/ui/unused/unused-field-assign.rs new file mode 100644 index 0000000000000..f9b1cb4d7ab26 --- /dev/null +++ b/src/test/ui/unused/unused-field-assign.rs @@ -0,0 +1,34 @@ +#![deny(unused_assignments)] + +struct Foo { + x: i16 +} + +const FOO: Foo = Foo { x: 1 }; +const BAR: (i16, bool) = (2, false); + +fn main() { + Foo { x: 2 }.x = 3; + //~^ ERROR unused assignement to temporary + Foo { x: 2 }.x += 1; + //~^ ERROR unused assignement to temporary + + (2, 12).0 += 10; + //~^ ERROR unused assignement to temporary + (10, false).1 = true; + //~^ ERROR unused assignement to temporary + + FOO.x = 2; + //~^ ERROR unused assignement to temporary + //~| HELP consider introducing a local variable + FOO.x -= 6; + //~^ ERROR unused assignement to temporary + //~| HELP consider introducing a local variable + + BAR.1 = true; + //~^ ERROR unused assignement to temporary + //~| HELP consider introducing a local variable + BAR.0 *= 2; + //~^ ERROR unused assignement to temporary + //~| HELP consider introducing a local variable +} diff --git a/src/test/ui/unused/unused-field-assign.stderr b/src/test/ui/unused/unused-field-assign.stderr new file mode 100644 index 0000000000000..56803ac794972 --- /dev/null +++ b/src/test/ui/unused/unused-field-assign.stderr @@ -0,0 +1,84 @@ +error: unused assignement to temporary + --> $DIR/unused-field-assign.rs:11:20 + | +LL | Foo { x: 2 }.x = 3; + | ------------ ^ + | | + | this is not bound to any variable + | +note: the lint level is defined here + --> $DIR/unused-field-assign.rs:1:9 + | +LL | #![deny(unused_assignments)] + | ^^^^^^^^^^^^^^^^^^ + +error: unused assignement to temporary + --> $DIR/unused-field-assign.rs:13:20 + | +LL | Foo { x: 2 }.x += 1; + | ------------ ^^ + | | + | this is not bound to any variable + +error: unused assignement to temporary + --> $DIR/unused-field-assign.rs:16:15 + | +LL | (2, 12).0 += 10; + | ------- ^^ + | | + | this is not bound to any variable + +error: unused assignement to temporary + --> $DIR/unused-field-assign.rs:18:19 + | +LL | (10, false).1 = true; + | ----------- ^ + | | + | this is not bound to any variable + +error: unused assignement to temporary + --> $DIR/unused-field-assign.rs:21:11 + | +LL | FOO.x = 2; + | --- ^ + | | + | this is not bound to any variable + | help: consider introducing a local variable: `let mut foo = FOO;` + | + = note: in Rust, constants are not associated with a specific memory location, and are inlined wherever they are used + +error: unused assignement to temporary + --> $DIR/unused-field-assign.rs:24:11 + | +LL | FOO.x -= 6; + | --- ^^ + | | + | this is not bound to any variable + | help: consider introducing a local variable: `let mut foo = FOO;` + | + = note: in Rust, constants are not associated with a specific memory location, and are inlined wherever they are used + +error: unused assignement to temporary + --> $DIR/unused-field-assign.rs:28:11 + | +LL | BAR.1 = true; + | --- ^ + | | + | this is not bound to any variable + | help: consider introducing a local variable: `let mut bar = BAR;` + | + = note: in Rust, constants are not associated with a specific memory location, and are inlined wherever they are used + +error: unused assignement to temporary + --> $DIR/unused-field-assign.rs:31:11 + | +LL | BAR.0 *= 2; + | --- ^^ + | | + | this is not bound to any variable + | help: consider introducing a local variable: `let mut bar = BAR;` + | + = note: in Rust, constants are not associated with a specific memory location, and are inlined wherever they are used + +error: aborting due to 8 previous errors +