From a41a13f775dfbc73b7c99e92619fd301ffcc4a53 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 6 Sep 2021 16:59:24 -0500 Subject: [PATCH] Add `ConstraintCategory::Usage` for handling aggregate construction In some cases, we emit borrowcheck diagnostics pointing at a particular field expression in a struct expression (e.g. `MyStruct { field: my_expr }`). However, this behavior currently relies on us choosing the `ConstraintCategory::Boring` with the 'correct' span. When adding additional variants to `ConstraintCategory`, (or changing existing usages away from `ConstraintCategory::Boring`), the current behavior can easily get broken, since a non-boring constraint will get chosen over a boring one. To make the diagnostic output less fragile, this commit adds a `ConstraintCategory::Usage` variant. We use this variant for the temporary assignments created for each field of an aggregate we are constructing. Using this new variant, we can emit a message mentioning "this usage", emphasizing the fact that the error message is related to the specific use site (in the struct expression). This is preparation for additional work on improving NLL error messages (see #57374) --- .../src/diagnostics/region_errors.rs | 1 + compiler/rustc_borrowck/src/type_check/mod.rs | 13 ++++++++++ compiler/rustc_middle/src/mir/mod.rs | 3 +++ compiler/rustc_middle/src/mir/query.rs | 8 +++---- .../src/build/expr/as_operand.rs | 17 +++++++++---- .../src/build/expr/as_rvalue.rs | 24 +++++++++++-------- .../rustc_mir_build/src/build/expr/into.rs | 11 +++++++-- .../ui/async-await/issue-76547.nll.stderr | 22 +++++++++-------- src/test/ui/issues/issue-61882-2.stderr | 2 +- src/test/ui/nll/issue-46036.stderr | 2 +- .../user-annotations/adt-brace-enums.stderr | 6 ++--- .../user-annotations/adt-brace-structs.stderr | 6 ++--- .../user-annotations/adt-tuple-enums.stderr | 6 ++--- .../user-annotations/adt-tuple-struct.stderr | 6 ++--- .../ui/nll/where_clauses_in_structs.stderr | 2 +- 15 files changed, 83 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs index 57d2a3c5ce91b..0761d63c66540 100644 --- a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs @@ -40,6 +40,7 @@ impl ConstraintDescription for ConstraintCategory { ConstraintCategory::CopyBound => "copying this value ", ConstraintCategory::OpaqueType => "opaque type ", ConstraintCategory::ClosureUpvar(_) => "closure capture ", + ConstraintCategory::Usage => "this usage ", ConstraintCategory::Boring | ConstraintCategory::BoringNoLocation | ConstraintCategory::Internal => "", diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index 3e757827a5e6c..98d2e91502847 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -1388,11 +1388,24 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { ConstraintCategory::Return(ReturnConstraint::Normal) } } + Some(l) + if matches!( + body.local_decls[l].local_info, + Some(box LocalInfo::AggregateTemp) + ) => + { + ConstraintCategory::Usage + } Some(l) if !body.local_decls[l].is_user_variable() => { ConstraintCategory::Boring } _ => ConstraintCategory::Assignment, }; + debug!( + "assignment category: {:?} {:?}", + category, + place.as_local().map(|l| &body.local_decls[l]) + ); let place_ty = place.ty(body, tcx).ty; let place_ty = self.normalize(place_ty, location); diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index e8d30034dc4e6..3e9c02ee268ca 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -992,6 +992,9 @@ pub enum LocalInfo<'tcx> { StaticRef { def_id: DefId, is_thread_local: bool }, /// A temporary created that references the const with the given `DefId` ConstRef { def_id: DefId }, + /// A temporary created during the creation of an aggregate + /// (e.g. a temporary for `foo` in `MyStruct { my_field: foo }`) + AggregateTemp, } impl<'tcx> LocalDecl<'tcx> { diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs index 567f65e83d98c..b003a504691bb 100644 --- a/compiler/rustc_middle/src/mir/query.rs +++ b/compiler/rustc_middle/src/mir/query.rs @@ -332,17 +332,15 @@ pub enum ConstraintCategory { CopyBound, SizedBound, Assignment, + /// A constraint that came from a usage of a variable (e.g. in an ADT expression + /// like `Foo { field: my_val }`) + Usage, OpaqueType, ClosureUpvar(hir::HirId), /// A "boring" constraint (caused by the given location) is one that /// the user probably doesn't want to see described in diagnostics, /// because it is kind of an artifact of the type system setup. - /// Example: `x = Foo { field: y }` technically creates - /// intermediate regions representing the "type of `Foo { field: y - /// }`", and data flows from `y` into those variables, but they - /// are not very interesting. The assignment into `x` on the other - /// hand might be. Boring, // Boring and applicable everywhere. BoringNoLocation, diff --git a/compiler/rustc_mir_build/src/build/expr/as_operand.rs b/compiler/rustc_mir_build/src/build/expr/as_operand.rs index bbb2f89fda939..b627b0763a286 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_operand.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_operand.rs @@ -20,7 +20,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { expr: &Expr<'tcx>, ) -> BlockAnd> { let local_scope = self.local_scope(); - self.as_operand(block, Some(local_scope), expr) + self.as_operand(block, Some(local_scope), expr, None) } /// Returns an operand suitable for use until the end of the current scope expression and @@ -85,6 +85,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// temporary `tmp = x`, so that we capture the value of `x` at /// this time. /// + /// If we end up needing to create a temporary, then we will use + /// `local_info` as its `LocalInfo`, unless `as_temporary` + /// has already assigned it a non-`None` `LocalInfo`. + /// Normally, you should use `None` for `local_info` + /// /// The operand is known to be live until the end of `scope`. /// /// Like `as_local_call_operand`, except that the argument will @@ -94,15 +99,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { mut block: BasicBlock, scope: Option, expr: &Expr<'tcx>, + local_info: Option>>, ) -> BlockAnd> { - debug!("as_operand(block={:?}, expr={:?})", block, expr); + debug!("as_operand(block={:?}, expr={:?} local_info={:?})", block, expr, local_info); let this = self; if let ExprKind::Scope { region_scope, lint_level, value } = expr.kind { let source_info = this.source_info(expr.span); let region_scope = (region_scope, source_info); return this.in_scope(region_scope, lint_level, |this| { - this.as_operand(block, scope, &this.thir[value]) + this.as_operand(block, scope, &this.thir[value], local_info) }); } @@ -115,6 +121,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } Category::Place | Category::Rvalue(..) => { let operand = unpack!(block = this.as_temp(block, scope, expr, Mutability::Mut)); + if this.local_decls[operand].local_info.is_none() { + this.local_decls[operand].local_info = local_info; + } block.and(Operand::Move(Place::from(operand))) } } @@ -167,6 +176,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } - this.as_operand(block, scope, expr) + this.as_operand(block, scope, expr, None) } } diff --git a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs index be0d5d2f1b2af..4b40faaf1956d 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -52,16 +52,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } ExprKind::Repeat { value, count } => { let value_operand = - unpack!(block = this.as_operand(block, scope, &this.thir[value])); + unpack!(block = this.as_operand(block, scope, &this.thir[value], None)); block.and(Rvalue::Repeat(value_operand, count)) } ExprKind::Binary { op, lhs, rhs } => { - let lhs = unpack!(block = this.as_operand(block, scope, &this.thir[lhs])); - let rhs = unpack!(block = this.as_operand(block, scope, &this.thir[rhs])); + let lhs = unpack!(block = this.as_operand(block, scope, &this.thir[lhs], None)); + let rhs = unpack!(block = this.as_operand(block, scope, &this.thir[rhs], None)); this.build_binary_op(block, op, expr_span, expr.ty, lhs, rhs) } ExprKind::Unary { op, arg } => { - let arg = unpack!(block = this.as_operand(block, scope, &this.thir[arg])); + let arg = unpack!(block = this.as_operand(block, scope, &this.thir[arg], None)); // Check for -MIN on signed integers if this.check_overflow && op == UnOp::Neg && expr.ty.is_signed() { let bool_ty = this.tcx.types.bool; @@ -116,11 +116,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block.and(Rvalue::Use(Operand::Move(Place::from(result)))) } ExprKind::Cast { source } => { - let source = unpack!(block = this.as_operand(block, scope, &this.thir[source])); + let source = + unpack!(block = this.as_operand(block, scope, &this.thir[source], None)); block.and(Rvalue::Cast(CastKind::Misc, source, expr.ty)) } ExprKind::Pointer { cast, source } => { - let source = unpack!(block = this.as_operand(block, scope, &this.thir[source])); + let source = + unpack!(block = this.as_operand(block, scope, &this.thir[source], None)); block.and(Rvalue::Cast(CastKind::Pointer(cast), source, expr.ty)) } ExprKind::Array { ref fields } => { @@ -155,7 +157,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let fields: Vec<_> = fields .into_iter() .copied() - .map(|f| unpack!(block = this.as_operand(block, scope, &this.thir[f]))) + .map(|f| unpack!(block = this.as_operand(block, scope, &this.thir[f], None))) .collect(); block.and(Rvalue::Aggregate(Box::new(AggregateKind::Array(el_ty)), fields)) @@ -166,7 +168,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let fields: Vec<_> = fields .into_iter() .copied() - .map(|f| unpack!(block = this.as_operand(block, scope, &this.thir[f]))) + .map(|f| unpack!(block = this.as_operand(block, scope, &this.thir[f], None))) .collect(); block.and(Rvalue::Aggregate(Box::new(AggregateKind::Tuple), fields)) @@ -242,7 +244,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { &this.thir[arg], ) ), - _ => unpack!(block = this.as_operand(block, scope, upvar)), + _ => { + unpack!(block = this.as_operand(block, scope, upvar, None)) + } } } } @@ -304,7 +308,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { Category::of(&expr.kind), Some(Category::Rvalue(RvalueFunc::AsRvalue)) )); - let operand = unpack!(block = this.as_operand(block, scope, expr)); + let operand = unpack!(block = this.as_operand(block, scope, expr, None)); block.and(Rvalue::Use(operand)) } } diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index e30e758e63779..1803a18441ce2 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -326,10 +326,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let fields_map: FxHashMap<_, _> = fields .into_iter() .map(|f| { + let local_info = Box::new(LocalInfo::AggregateTemp); ( f.name, unpack!( - block = this.as_operand(block, Some(scope), &this.thir[f.expr]) + block = this.as_operand( + block, + Some(scope), + &this.thir[f.expr], + Some(local_info) + ) ), ) }) @@ -508,7 +514,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ExprKind::Yield { value } => { let scope = this.local_scope(); - let value = unpack!(block = this.as_operand(block, Some(scope), &this.thir[value])); + let value = + unpack!(block = this.as_operand(block, Some(scope), &this.thir[value], None)); let resume = this.cfg.start_new_block(); this.cfg.terminate( block, diff --git a/src/test/ui/async-await/issue-76547.nll.stderr b/src/test/ui/async-await/issue-76547.nll.stderr index 2456d6a147488..0ac2a39666966 100644 --- a/src/test/ui/async-await/issue-76547.nll.stderr +++ b/src/test/ui/async-await/issue-76547.nll.stderr @@ -1,20 +1,22 @@ error: lifetime may not live long enough - --> $DIR/issue-76547.rs:19:14 + --> $DIR/issue-76547.rs:20:13 | LL | async fn fut(bufs: &mut [&mut [u8]]) { - | ^^^^ - - let's call the lifetime of this reference `'2` - | | | - | | let's call the lifetime of this reference `'1` - | assignment requires that `'1` must outlive `'2` + | - - let's call the lifetime of this reference `'2` + | | + | let's call the lifetime of this reference `'1` +LL | ListFut(bufs).await + | ^^^^ this usage requires that `'1` must outlive `'2` error: lifetime may not live long enough - --> $DIR/issue-76547.rs:33:15 + --> $DIR/issue-76547.rs:34:14 | LL | async fn fut2(bufs: &mut [&mut [u8]]) -> i32 { - | ^^^^ - - let's call the lifetime of this reference `'2` - | | | - | | let's call the lifetime of this reference `'1` - | assignment requires that `'1` must outlive `'2` + | - - let's call the lifetime of this reference `'2` + | | + | let's call the lifetime of this reference `'1` +LL | ListFut2(bufs).await + | ^^^^ this usage requires that `'1` must outlive `'2` error: aborting due to 2 previous errors diff --git a/src/test/ui/issues/issue-61882-2.stderr b/src/test/ui/issues/issue-61882-2.stderr index 03a65540ced04..0b8e134c9662e 100644 --- a/src/test/ui/issues/issue-61882-2.stderr +++ b/src/test/ui/issues/issue-61882-2.stderr @@ -5,7 +5,7 @@ LL | Self(&x); | ^^ | | | borrowed value does not live long enough - | requires that `x` is borrowed for `'static` + | this usage requires that `x` is borrowed for `'static` LL | LL | } | - `x` dropped here while still borrowed diff --git a/src/test/ui/nll/issue-46036.stderr b/src/test/ui/nll/issue-46036.stderr index 49dd0e267b8ea..e6e95ee613647 100644 --- a/src/test/ui/nll/issue-46036.stderr +++ b/src/test/ui/nll/issue-46036.stderr @@ -5,7 +5,7 @@ LL | let foo = Foo { x: &a }; | ^^ | | | borrowed value does not live long enough - | requires that `a` is borrowed for `'static` + | this usage requires that `a` is borrowed for `'static` LL | loop { } LL | } | - `a` dropped here while still borrowed diff --git a/src/test/ui/nll/user-annotations/adt-brace-enums.stderr b/src/test/ui/nll/user-annotations/adt-brace-enums.stderr index e38b77fdcea01..253e382511045 100644 --- a/src/test/ui/nll/user-annotations/adt-brace-enums.stderr +++ b/src/test/ui/nll/user-annotations/adt-brace-enums.stderr @@ -5,7 +5,7 @@ LL | SomeEnum::SomeVariant::<&'static u32> { t: &c }; | ^^ | | | borrowed value does not live long enough - | requires that `c` is borrowed for `'static` + | this usage requires that `c` is borrowed for `'static` LL | } | - `c` dropped here while still borrowed @@ -19,7 +19,7 @@ LL | SomeEnum::SomeVariant::<&'a u32> { t: &c }; | ^^ | | | borrowed value does not live long enough - | requires that `c` is borrowed for `'a` + | this usage requires that `c` is borrowed for `'a` LL | } | - `c` dropped here while still borrowed @@ -33,7 +33,7 @@ LL | SomeEnum::SomeVariant::<&'a u32> { t: &c }; | ^^ | | | borrowed value does not live long enough - | requires that `c` is borrowed for `'a` + | this usage requires that `c` is borrowed for `'a` LL | }; | - `c` dropped here while still borrowed diff --git a/src/test/ui/nll/user-annotations/adt-brace-structs.stderr b/src/test/ui/nll/user-annotations/adt-brace-structs.stderr index 3c3003477c2d6..8b9d1705df6ad 100644 --- a/src/test/ui/nll/user-annotations/adt-brace-structs.stderr +++ b/src/test/ui/nll/user-annotations/adt-brace-structs.stderr @@ -5,7 +5,7 @@ LL | SomeStruct::<&'static u32> { t: &c }; | ^^ | | | borrowed value does not live long enough - | requires that `c` is borrowed for `'static` + | this usage requires that `c` is borrowed for `'static` LL | } | - `c` dropped here while still borrowed @@ -19,7 +19,7 @@ LL | SomeStruct::<&'a u32> { t: &c }; | ^^ | | | borrowed value does not live long enough - | requires that `c` is borrowed for `'a` + | this usage requires that `c` is borrowed for `'a` LL | } | - `c` dropped here while still borrowed @@ -33,7 +33,7 @@ LL | SomeStruct::<&'a u32> { t: &c }; | ^^ | | | borrowed value does not live long enough - | requires that `c` is borrowed for `'a` + | this usage requires that `c` is borrowed for `'a` LL | }; | - `c` dropped here while still borrowed diff --git a/src/test/ui/nll/user-annotations/adt-tuple-enums.stderr b/src/test/ui/nll/user-annotations/adt-tuple-enums.stderr index 810912bf88618..2fa7042631d21 100644 --- a/src/test/ui/nll/user-annotations/adt-tuple-enums.stderr +++ b/src/test/ui/nll/user-annotations/adt-tuple-enums.stderr @@ -5,7 +5,7 @@ LL | SomeEnum::SomeVariant::<&'static u32>(&c); | ^^ | | | borrowed value does not live long enough - | requires that `c` is borrowed for `'static` + | this usage requires that `c` is borrowed for `'static` LL | } | - `c` dropped here while still borrowed @@ -19,7 +19,7 @@ LL | SomeEnum::SomeVariant::<&'a u32>(&c); | ^^ | | | borrowed value does not live long enough - | requires that `c` is borrowed for `'a` + | this usage requires that `c` is borrowed for `'a` LL | } | - `c` dropped here while still borrowed @@ -33,7 +33,7 @@ LL | SomeEnum::SomeVariant::<&'a u32>(&c); | ^^ | | | borrowed value does not live long enough - | requires that `c` is borrowed for `'a` + | this usage requires that `c` is borrowed for `'a` LL | }; | - `c` dropped here while still borrowed diff --git a/src/test/ui/nll/user-annotations/adt-tuple-struct.stderr b/src/test/ui/nll/user-annotations/adt-tuple-struct.stderr index 4d2140eca1b02..76b5252258c7b 100644 --- a/src/test/ui/nll/user-annotations/adt-tuple-struct.stderr +++ b/src/test/ui/nll/user-annotations/adt-tuple-struct.stderr @@ -5,7 +5,7 @@ LL | SomeStruct::<&'static u32>(&c); | ^^ | | | borrowed value does not live long enough - | requires that `c` is borrowed for `'static` + | this usage requires that `c` is borrowed for `'static` LL | } | - `c` dropped here while still borrowed @@ -19,7 +19,7 @@ LL | SomeStruct::<&'a u32>(&c); | ^^ | | | borrowed value does not live long enough - | requires that `c` is borrowed for `'a` + | this usage requires that `c` is borrowed for `'a` LL | } | - `c` dropped here while still borrowed @@ -33,7 +33,7 @@ LL | SomeStruct::<&'a u32>(&c); | ^^ | | | borrowed value does not live long enough - | requires that `c` is borrowed for `'a` + | this usage requires that `c` is borrowed for `'a` LL | }; | - `c` dropped here while still borrowed diff --git a/src/test/ui/nll/where_clauses_in_structs.stderr b/src/test/ui/nll/where_clauses_in_structs.stderr index 2e990131e61de..8499b00f6f556 100644 --- a/src/test/ui/nll/where_clauses_in_structs.stderr +++ b/src/test/ui/nll/where_clauses_in_structs.stderr @@ -6,7 +6,7 @@ LL | fn bar<'a, 'b>(x: Cell<&'a u32>, y: Cell<&'b u32>) { | | | lifetime `'a` defined here LL | Foo { x, y }; - | ^ requires that `'a` must outlive `'b` + | ^ this usage requires that `'a` must outlive `'b` | = help: consider adding the following bound: `'a: 'b`