Skip to content

Commit 3feddf7

Browse files
Aaron1011Aaron1011
committed
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 rust-lang#57374)
1 parent 497ee32 commit 3feddf7

File tree

14 files changed

+71
-36
lines changed

14 files changed

+71
-36
lines changed

‎compiler/rustc_borrowck/src/diagnostics/region_errors.rs

+1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ impl ConstraintDescription for ConstraintCategory {
4040
ConstraintCategory::CopyBound => "copying this value ",
4141
ConstraintCategory::OpaqueType => "opaque type ",
4242
ConstraintCategory::ClosureUpvar(_) => "closure capture ",
43+
ConstraintCategory::Usage => "this usage ",
4344
ConstraintCategory::Boring
4445
| ConstraintCategory::BoringNoLocation
4546
| ConstraintCategory::Internal => "",

‎compiler/rustc_borrowck/src/type_check/mod.rs

+13
Original file line numberDiff line numberDiff line change
@@ -1386,11 +1386,24 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
13861386
ConstraintCategory::Return(ReturnConstraint::Normal)
13871387
}
13881388
}
1389+
Some(l)
1390+
if matches!(
1391+
body.local_decls[l].local_info,
1392+
Some(box LocalInfo::AggregateTemp)
1393+
) =>
1394+
{
1395+
ConstraintCategory::Usage
1396+
}
13891397
Some(l) if !body.local_decls[l].is_user_variable() => {
13901398
ConstraintCategory::Boring
13911399
}
13921400
_ => ConstraintCategory::Assignment,
13931401
};
1402+
debug!(
1403+
"assignment category: {:?} {:?}",
1404+
category,
1405+
place.as_local().map(|l| &body.local_decls[l])
1406+
);
13941407

13951408
let place_ty = place.ty(body, tcx).ty;
13961409
let place_ty = self.normalize(place_ty, location);

‎compiler/rustc_middle/src/mir/mod.rs

+3
Original file line numberDiff line numberDiff line change
@@ -993,6 +993,9 @@ pub enum LocalInfo<'tcx> {
993993
StaticRef { def_id: DefId, is_thread_local: bool },
994994
/// A temporary created that references the const with the given `DefId`
995995
ConstRef { def_id: DefId },
996+
/// A temporary created during the creation of an aggregate
997+
/// (e.g. a temporary for `foo` in `MyStruct { my_field: foo }`)
998+
AggregateTemp,
996999
}
9971000

9981001
impl<'tcx> LocalDecl<'tcx> {

‎compiler/rustc_middle/src/mir/query.rs

+3-5
Original file line numberDiff line numberDiff line change
@@ -332,17 +332,15 @@ pub enum ConstraintCategory {
332332
CopyBound,
333333
SizedBound,
334334
Assignment,
335+
/// A constraint that came from a usage of a variable (e.g. in an ADT expression
336+
/// like `Foo { field: my_val }`)
337+
Usage,
335338
OpaqueType,
336339
ClosureUpvar(hir::HirId),
337340

338341
/// A "boring" constraint (caused by the given location) is one that
339342
/// the user probably doesn't want to see described in diagnostics,
340343
/// because it is kind of an artifact of the type system setup.
341-
/// Example: `x = Foo { field: y }` technically creates
342-
/// intermediate regions representing the "type of `Foo { field: y
343-
/// }`", and data flows from `y` into those variables, but they
344-
/// are not very interesting. The assignment into `x` on the other
345-
/// hand might be.
346344
Boring,
347345
// Boring and applicable everywhere.
348346
BoringNoLocation,

‎compiler/rustc_mir_build/src/build/expr/as_operand.rs

+13-4
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
2020
expr: &Expr<'tcx>,
2121
) -> BlockAnd<Operand<'tcx>> {
2222
let local_scope = self.local_scope();
23-
self.as_operand(block, Some(local_scope), expr)
23+
self.as_operand(block, Some(local_scope), expr, None)
2424
}
2525

2626
/// 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> {
8585
/// temporary `tmp = x`, so that we capture the value of `x` at
8686
/// this time.
8787
///
88+
/// If we end up needing to create a temporary, then we will use
89+
/// `local_info` as its `LocalInfo`, unless `as_temporary`
90+
/// has already assigned it a non-`None` `LocalInfo`.
91+
/// Normally, you should use `None` for `local_info`
92+
///
8893
/// The operand is known to be live until the end of `scope`.
8994
///
9095
/// Like `as_local_call_operand`, except that the argument will
@@ -94,15 +99,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
9499
mut block: BasicBlock,
95100
scope: Option<region::Scope>,
96101
expr: &Expr<'tcx>,
102+
local_info: Option<Box<LocalInfo<'tcx>>>,
97103
) -> BlockAnd<Operand<'tcx>> {
98-
debug!("as_operand(block={:?}, expr={:?})", block, expr);
104+
debug!("as_operand(block={:?}, expr={:?} local_info={:?})", block, expr, local_info);
99105
let this = self;
100106

101107
if let ExprKind::Scope { region_scope, lint_level, value } = expr.kind {
102108
let source_info = this.source_info(expr.span);
103109
let region_scope = (region_scope, source_info);
104110
return this.in_scope(region_scope, lint_level, |this| {
105-
this.as_operand(block, scope, &this.thir[value])
111+
this.as_operand(block, scope, &this.thir[value], local_info)
106112
});
107113
}
108114

@@ -115,6 +121,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
115121
}
116122
Category::Place | Category::Rvalue(..) => {
117123
let operand = unpack!(block = this.as_temp(block, scope, expr, Mutability::Mut));
124+
if this.local_decls[operand].local_info.is_none() {
125+
this.local_decls[operand].local_info = local_info;
126+
}
118127
block.and(Operand::Move(Place::from(operand)))
119128
}
120129
}
@@ -167,6 +176,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
167176
}
168177
}
169178

170-
this.as_operand(block, scope, expr)
179+
this.as_operand(block, scope, expr, None)
171180
}
172181
}

‎compiler/rustc_mir_build/src/build/expr/as_rvalue.rs

+14-10
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
5252
}
5353
ExprKind::Repeat { value, count } => {
5454
let value_operand =
55-
unpack!(block = this.as_operand(block, scope, &this.thir[value]));
55+
unpack!(block = this.as_operand(block, scope, &this.thir[value], None));
5656
block.and(Rvalue::Repeat(value_operand, count))
5757
}
5858
ExprKind::Binary { op, lhs, rhs } => {
59-
let lhs = unpack!(block = this.as_operand(block, scope, &this.thir[lhs]));
60-
let rhs = unpack!(block = this.as_operand(block, scope, &this.thir[rhs]));
59+
let lhs = unpack!(block = this.as_operand(block, scope, &this.thir[lhs], None));
60+
let rhs = unpack!(block = this.as_operand(block, scope, &this.thir[rhs], None));
6161
this.build_binary_op(block, op, expr_span, expr.ty, lhs, rhs)
6262
}
6363
ExprKind::Unary { op, arg } => {
64-
let arg = unpack!(block = this.as_operand(block, scope, &this.thir[arg]));
64+
let arg = unpack!(block = this.as_operand(block, scope, &this.thir[arg], None));
6565
// Check for -MIN on signed integers
6666
if this.check_overflow && op == UnOp::Neg && expr.ty.is_signed() {
6767
let bool_ty = this.tcx.types.bool;
@@ -116,11 +116,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
116116
block.and(Rvalue::Use(Operand::Move(Place::from(result))))
117117
}
118118
ExprKind::Cast { source } => {
119-
let source = unpack!(block = this.as_operand(block, scope, &this.thir[source]));
119+
let source =
120+
unpack!(block = this.as_operand(block, scope, &this.thir[source], None));
120121
block.and(Rvalue::Cast(CastKind::Misc, source, expr.ty))
121122
}
122123
ExprKind::Pointer { cast, source } => {
123-
let source = unpack!(block = this.as_operand(block, scope, &this.thir[source]));
124+
let source =
125+
unpack!(block = this.as_operand(block, scope, &this.thir[source], None));
124126
block.and(Rvalue::Cast(CastKind::Pointer(cast), source, expr.ty))
125127
}
126128
ExprKind::Array { ref fields } => {
@@ -155,7 +157,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
155157
let fields: Vec<_> = fields
156158
.into_iter()
157159
.copied()
158-
.map(|f| unpack!(block = this.as_operand(block, scope, &this.thir[f])))
160+
.map(|f| unpack!(block = this.as_operand(block, scope, &this.thir[f], None)))
159161
.collect();
160162

161163
block.and(Rvalue::Aggregate(Box::new(AggregateKind::Array(el_ty)), fields))
@@ -166,7 +168,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
166168
let fields: Vec<_> = fields
167169
.into_iter()
168170
.copied()
169-
.map(|f| unpack!(block = this.as_operand(block, scope, &this.thir[f])))
171+
.map(|f| unpack!(block = this.as_operand(block, scope, &this.thir[f], None)))
170172
.collect();
171173

172174
block.and(Rvalue::Aggregate(Box::new(AggregateKind::Tuple), fields))
@@ -242,7 +244,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
242244
&this.thir[arg],
243245
)
244246
),
245-
_ => unpack!(block = this.as_operand(block, scope, upvar)),
247+
_ => {
248+
unpack!(block = this.as_operand(block, scope, upvar, None))
249+
}
246250
}
247251
}
248252
}
@@ -304,7 +308,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
304308
Category::of(&expr.kind),
305309
Some(Category::Rvalue(RvalueFunc::AsRvalue))
306310
));
307-
let operand = unpack!(block = this.as_operand(block, scope, expr));
311+
let operand = unpack!(block = this.as_operand(block, scope, expr, None));
308312
block.and(Rvalue::Use(operand))
309313
}
310314
}

‎compiler/rustc_mir_build/src/build/expr/into.rs

+9-2
Original file line numberDiff line numberDiff line change
@@ -326,10 +326,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
326326
let fields_map: FxHashMap<_, _> = fields
327327
.into_iter()
328328
.map(|f| {
329+
let local_info = Box::new(LocalInfo::AggregateTemp);
329330
(
330331
f.name,
331332
unpack!(
332-
block = this.as_operand(block, Some(scope), &this.thir[f.expr])
333+
block = this.as_operand(
334+
block,
335+
Some(scope),
336+
&this.thir[f.expr],
337+
Some(local_info)
338+
)
333339
),
334340
)
335341
})
@@ -508,7 +514,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
508514

509515
ExprKind::Yield { value } => {
510516
let scope = this.local_scope();
511-
let value = unpack!(block = this.as_operand(block, Some(scope), &this.thir[value]));
517+
let value =
518+
unpack!(block = this.as_operand(block, Some(scope), &this.thir[value], None));
512519
let resume = this.cfg.start_new_block();
513520
this.cfg.terminate(
514521
block,

‎src/test/ui/issues/issue-61882-2.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | Self(&x);
55
| ^^
66
| |
77
| borrowed value does not live long enough
8-
| requires that `x` is borrowed for `'static`
8+
| this usage requires that `x` is borrowed for `'static`
99
LL |
1010
LL | }
1111
| - `x` dropped here while still borrowed

‎src/test/ui/nll/issue-46036.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | let foo = Foo { x: &a };
55
| ^^
66
| |
77
| borrowed value does not live long enough
8-
| requires that `a` is borrowed for `'static`
8+
| this usage requires that `a` is borrowed for `'static`
99
LL | loop { }
1010
LL | }
1111
| - `a` dropped here while still borrowed

‎src/test/ui/nll/user-annotations/adt-brace-enums.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | SomeEnum::SomeVariant::<&'static u32> { t: &c };
55
| ^^
66
| |
77
| borrowed value does not live long enough
8-
| requires that `c` is borrowed for `'static`
8+
| this usage requires that `c` is borrowed for `'static`
99
LL | }
1010
| - `c` dropped here while still borrowed
1111

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

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

‎src/test/ui/nll/user-annotations/adt-brace-structs.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | SomeStruct::<&'static u32> { t: &c };
55
| ^^
66
| |
77
| borrowed value does not live long enough
8-
| requires that `c` is borrowed for `'static`
8+
| this usage requires that `c` is borrowed for `'static`
99
LL | }
1010
| - `c` dropped here while still borrowed
1111

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

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

‎src/test/ui/nll/user-annotations/adt-tuple-enums.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | SomeEnum::SomeVariant::<&'static u32>(&c);
55
| ^^
66
| |
77
| borrowed value does not live long enough
8-
| requires that `c` is borrowed for `'static`
8+
| this usage requires that `c` is borrowed for `'static`
99
LL | }
1010
| - `c` dropped here while still borrowed
1111

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

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

‎src/test/ui/nll/user-annotations/adt-tuple-struct.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | SomeStruct::<&'static u32>(&c);
55
| ^^
66
| |
77
| borrowed value does not live long enough
8-
| requires that `c` is borrowed for `'static`
8+
| this usage requires that `c` is borrowed for `'static`
99
LL | }
1010
| - `c` dropped here while still borrowed
1111

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

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

‎src/test/ui/nll/where_clauses_in_structs.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ LL | fn bar<'a, 'b>(x: Cell<&'a u32>, y: Cell<&'b u32>) {
66
| |
77
| lifetime `'a` defined here
88
LL | Foo { x, y };
9-
| ^ requires that `'a` must outlive `'b`
9+
| ^ this usage requires that `'a` must outlive `'b`
1010
|
1111
= help: consider adding the following bound: `'a: 'b`
1212

0 commit comments

Comments
 (0)