Auto merge of #88708 - Aaron1011:aggregate-usage, r=oli-obk

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)
This commit is contained in:
bors 2021-09-20 00:53:13 +00:00
commit 0c0826c3e2
15 changed files with 83 additions and 46 deletions

View file

@ -40,6 +40,7 @@ impl ConstraintDescription for ConstraintCategory {
ConstraintCategory::CopyBound => "copying this value ", ConstraintCategory::CopyBound => "copying this value ",
ConstraintCategory::OpaqueType => "opaque type ", ConstraintCategory::OpaqueType => "opaque type ",
ConstraintCategory::ClosureUpvar(_) => "closure capture ", ConstraintCategory::ClosureUpvar(_) => "closure capture ",
ConstraintCategory::Usage => "this usage ",
ConstraintCategory::Boring ConstraintCategory::Boring
| ConstraintCategory::BoringNoLocation | ConstraintCategory::BoringNoLocation
| ConstraintCategory::Internal => "", | ConstraintCategory::Internal => "",

View file

@ -1388,11 +1388,24 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
ConstraintCategory::Return(ReturnConstraint::Normal) 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() => { Some(l) if !body.local_decls[l].is_user_variable() => {
ConstraintCategory::Boring ConstraintCategory::Boring
} }
_ => ConstraintCategory::Assignment, _ => 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 = place.ty(body, tcx).ty;
let place_ty = self.normalize(place_ty, location); let place_ty = self.normalize(place_ty, location);

View file

@ -992,6 +992,9 @@ pub enum LocalInfo<'tcx> {
StaticRef { def_id: DefId, is_thread_local: bool }, StaticRef { def_id: DefId, is_thread_local: bool },
/// A temporary created that references the const with the given `DefId` /// A temporary created that references the const with the given `DefId`
ConstRef { def_id: 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> { impl<'tcx> LocalDecl<'tcx> {

View file

@ -332,17 +332,15 @@ pub enum ConstraintCategory {
CopyBound, CopyBound,
SizedBound, SizedBound,
Assignment, Assignment,
/// A constraint that came from a usage of a variable (e.g. in an ADT expression
/// like `Foo { field: my_val }`)
Usage,
OpaqueType, OpaqueType,
ClosureUpvar(hir::HirId), ClosureUpvar(hir::HirId),
/// A "boring" constraint (caused by the given location) is one that /// A "boring" constraint (caused by the given location) is one that
/// the user probably doesn't want to see described in diagnostics, /// the user probably doesn't want to see described in diagnostics,
/// because it is kind of an artifact of the type system setup. /// 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,
// Boring and applicable everywhere. // Boring and applicable everywhere.
BoringNoLocation, BoringNoLocation,

View file

@ -20,7 +20,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
expr: &Expr<'tcx>, expr: &Expr<'tcx>,
) -> BlockAnd<Operand<'tcx>> { ) -> BlockAnd<Operand<'tcx>> {
let local_scope = self.local_scope(); 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 /// 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 /// temporary `tmp = x`, so that we capture the value of `x` at
/// this time. /// 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`. /// The operand is known to be live until the end of `scope`.
/// ///
/// Like `as_local_call_operand`, except that the argument will /// Like `as_local_call_operand`, except that the argument will
@ -94,15 +99,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
mut block: BasicBlock, mut block: BasicBlock,
scope: Option<region::Scope>, scope: Option<region::Scope>,
expr: &Expr<'tcx>, expr: &Expr<'tcx>,
local_info: Option<Box<LocalInfo<'tcx>>>,
) -> BlockAnd<Operand<'tcx>> { ) -> BlockAnd<Operand<'tcx>> {
debug!("as_operand(block={:?}, expr={:?})", block, expr); debug!("as_operand(block={:?}, expr={:?} local_info={:?})", block, expr, local_info);
let this = self; let this = self;
if let ExprKind::Scope { region_scope, lint_level, value } = expr.kind { if let ExprKind::Scope { region_scope, lint_level, value } = expr.kind {
let source_info = this.source_info(expr.span); let source_info = this.source_info(expr.span);
let region_scope = (region_scope, source_info); let region_scope = (region_scope, source_info);
return this.in_scope(region_scope, lint_level, |this| { 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(..) => { Category::Place | Category::Rvalue(..) => {
let operand = unpack!(block = this.as_temp(block, scope, expr, Mutability::Mut)); 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))) 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)
} }
} }

View file

@ -52,16 +52,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
} }
ExprKind::Repeat { value, count } => { ExprKind::Repeat { value, count } => {
let value_operand = 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)) block.and(Rvalue::Repeat(value_operand, count))
} }
ExprKind::Binary { op, lhs, rhs } => { ExprKind::Binary { op, lhs, rhs } => {
let lhs = unpack!(block = this.as_operand(block, scope, &this.thir[lhs])); 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])); 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) this.build_binary_op(block, op, expr_span, expr.ty, lhs, rhs)
} }
ExprKind::Unary { op, arg } => { 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 // Check for -MIN on signed integers
if this.check_overflow && op == UnOp::Neg && expr.ty.is_signed() { if this.check_overflow && op == UnOp::Neg && expr.ty.is_signed() {
let bool_ty = this.tcx.types.bool; 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)))) block.and(Rvalue::Use(Operand::Move(Place::from(result))))
} }
ExprKind::Cast { source } => { 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)) block.and(Rvalue::Cast(CastKind::Misc, source, expr.ty))
} }
ExprKind::Pointer { cast, source } => { 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)) block.and(Rvalue::Cast(CastKind::Pointer(cast), source, expr.ty))
} }
ExprKind::Array { ref fields } => { ExprKind::Array { ref fields } => {
@ -155,7 +157,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let fields: Vec<_> = fields let fields: Vec<_> = fields
.into_iter() .into_iter()
.copied() .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(); .collect();
block.and(Rvalue::Aggregate(Box::new(AggregateKind::Array(el_ty)), fields)) 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 let fields: Vec<_> = fields
.into_iter() .into_iter()
.copied() .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(); .collect();
block.and(Rvalue::Aggregate(Box::new(AggregateKind::Tuple), fields)) block.and(Rvalue::Aggregate(Box::new(AggregateKind::Tuple), fields))
@ -242,7 +244,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
&this.thir[arg], &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), Category::of(&expr.kind),
Some(Category::Rvalue(RvalueFunc::AsRvalue)) 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)) block.and(Rvalue::Use(operand))
} }
} }

View file

@ -326,10 +326,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let fields_map: FxHashMap<_, _> = fields let fields_map: FxHashMap<_, _> = fields
.into_iter() .into_iter()
.map(|f| { .map(|f| {
let local_info = Box::new(LocalInfo::AggregateTemp);
( (
f.name, f.name,
unpack!( 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 } => { ExprKind::Yield { value } => {
let scope = this.local_scope(); 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(); let resume = this.cfg.start_new_block();
this.cfg.terminate( this.cfg.terminate(
block, block,

View file

@ -1,20 +1,22 @@
error: lifetime may not live long enough 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]]) { 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 `'2`
| | | | |
| | let's call the lifetime of this reference `'1` | let's call the lifetime of this reference `'1`
| assignment requires that `'1` must outlive `'2` LL | ListFut(bufs).await
| ^^^^ this usage requires that `'1` must outlive `'2`
error: lifetime may not live long enough 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 { 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 `'2`
| | | | |
| | let's call the lifetime of this reference `'1` | let's call the lifetime of this reference `'1`
| assignment requires that `'1` must outlive `'2` LL | ListFut2(bufs).await
| ^^^^ this usage requires that `'1` must outlive `'2`
error: aborting due to 2 previous errors error: aborting due to 2 previous errors

View file

@ -5,7 +5,7 @@ LL | Self(&x);
| ^^ | ^^
| | | |
| borrowed value does not live long enough | 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 |
LL | } LL | }
| - `x` dropped here while still borrowed | - `x` dropped here while still borrowed

View file

@ -5,7 +5,7 @@ LL | let foo = Foo { x: &a };
| ^^ | ^^
| | | |
| borrowed value does not live long enough | 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 | loop { }
LL | } LL | }
| - `a` dropped here while still borrowed | - `a` dropped here while still borrowed

View file

@ -5,7 +5,7 @@ LL | SomeEnum::SomeVariant::<&'static u32> { t: &c };
| ^^ | ^^
| | | |
| borrowed value does not live long enough | borrowed value does not live long enough
| requires that `c` is borrowed for `'static` | this usage requires that `c` is borrowed for `'static`
LL | } LL | }
| - `c` dropped here while still borrowed | - `c` dropped here while still borrowed
@ -19,7 +19,7 @@ LL | SomeEnum::SomeVariant::<&'a u32> { t: &c };
| ^^ | ^^
| | | |
| borrowed value does not live long enough | borrowed value does not live long enough
| requires that `c` is borrowed for `'a` | this usage requires that `c` is borrowed for `'a`
LL | } LL | }
| - `c` dropped here while still borrowed | - `c` dropped here while still borrowed
@ -33,7 +33,7 @@ LL | SomeEnum::SomeVariant::<&'a u32> { t: &c };
| ^^ | ^^
| | | |
| borrowed value does not live long enough | borrowed value does not live long enough
| requires that `c` is borrowed for `'a` | this usage requires that `c` is borrowed for `'a`
LL | }; LL | };
| - `c` dropped here while still borrowed | - `c` dropped here while still borrowed

View file

@ -5,7 +5,7 @@ LL | SomeStruct::<&'static u32> { t: &c };
| ^^ | ^^
| | | |
| borrowed value does not live long enough | borrowed value does not live long enough
| requires that `c` is borrowed for `'static` | this usage requires that `c` is borrowed for `'static`
LL | } LL | }
| - `c` dropped here while still borrowed | - `c` dropped here while still borrowed
@ -19,7 +19,7 @@ LL | SomeStruct::<&'a u32> { t: &c };
| ^^ | ^^
| | | |
| borrowed value does not live long enough | borrowed value does not live long enough
| requires that `c` is borrowed for `'a` | this usage requires that `c` is borrowed for `'a`
LL | } LL | }
| - `c` dropped here while still borrowed | - `c` dropped here while still borrowed
@ -33,7 +33,7 @@ LL | SomeStruct::<&'a u32> { t: &c };
| ^^ | ^^
| | | |
| borrowed value does not live long enough | borrowed value does not live long enough
| requires that `c` is borrowed for `'a` | this usage requires that `c` is borrowed for `'a`
LL | }; LL | };
| - `c` dropped here while still borrowed | - `c` dropped here while still borrowed

View file

@ -5,7 +5,7 @@ LL | SomeEnum::SomeVariant::<&'static u32>(&c);
| ^^ | ^^
| | | |
| borrowed value does not live long enough | borrowed value does not live long enough
| requires that `c` is borrowed for `'static` | this usage requires that `c` is borrowed for `'static`
LL | } LL | }
| - `c` dropped here while still borrowed | - `c` dropped here while still borrowed
@ -19,7 +19,7 @@ LL | SomeEnum::SomeVariant::<&'a u32>(&c);
| ^^ | ^^
| | | |
| borrowed value does not live long enough | borrowed value does not live long enough
| requires that `c` is borrowed for `'a` | this usage requires that `c` is borrowed for `'a`
LL | } LL | }
| - `c` dropped here while still borrowed | - `c` dropped here while still borrowed
@ -33,7 +33,7 @@ LL | SomeEnum::SomeVariant::<&'a u32>(&c);
| ^^ | ^^
| | | |
| borrowed value does not live long enough | borrowed value does not live long enough
| requires that `c` is borrowed for `'a` | this usage requires that `c` is borrowed for `'a`
LL | }; LL | };
| - `c` dropped here while still borrowed | - `c` dropped here while still borrowed

View file

@ -5,7 +5,7 @@ LL | SomeStruct::<&'static u32>(&c);
| ^^ | ^^
| | | |
| borrowed value does not live long enough | borrowed value does not live long enough
| requires that `c` is borrowed for `'static` | this usage requires that `c` is borrowed for `'static`
LL | } LL | }
| - `c` dropped here while still borrowed | - `c` dropped here while still borrowed
@ -19,7 +19,7 @@ LL | SomeStruct::<&'a u32>(&c);
| ^^ | ^^
| | | |
| borrowed value does not live long enough | borrowed value does not live long enough
| requires that `c` is borrowed for `'a` | this usage requires that `c` is borrowed for `'a`
LL | } LL | }
| - `c` dropped here while still borrowed | - `c` dropped here while still borrowed
@ -33,7 +33,7 @@ LL | SomeStruct::<&'a u32>(&c);
| ^^ | ^^
| | | |
| borrowed value does not live long enough | borrowed value does not live long enough
| requires that `c` is borrowed for `'a` | this usage requires that `c` is borrowed for `'a`
LL | }; LL | };
| - `c` dropped here while still borrowed | - `c` dropped here while still borrowed

View file

@ -6,7 +6,7 @@ LL | fn bar<'a, 'b>(x: Cell<&'a u32>, y: Cell<&'b u32>) {
| | | |
| lifetime `'a` defined here | lifetime `'a` defined here
LL | Foo { x, y }; 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` = help: consider adding the following bound: `'a: 'b`