about summary refs log tree commit diff
path: root/compiler
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-09-20 00:53:13 +0000
committerbors <bors@rust-lang.org>2021-09-20 00:53:13 +0000
commit0c0826c3e272840cf35c13a8a4debd3b5a7ad09c (patch)
treed8aab1d02da660bce6177b11d69525e8f07ba318 /compiler
parent91198820d7e697def79177c022b5e98b3d482ddc (diff)
parenta41a13f775dfbc73b7c99e92619fd301ffcc4a53 (diff)
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)
Diffstat (limited to 'compiler')
-rw-r--r--compiler/rustc_borrowck/src/diagnostics/region_errors.rs1
-rw-r--r--compiler/rustc_borrowck/src/type_check/mod.rs13
-rw-r--r--compiler/rustc_middle/src/mir/mod.rs3
-rw-r--r--compiler/rustc_middle/src/mir/query.rs8
-rw-r--r--compiler/rustc_mir_build/src/build/expr/as_operand.rs17
-rw-r--r--compiler/rustc_mir_build/src/build/expr/as_rvalue.rs24
-rw-r--r--compiler/rustc_mir_build/src/build/expr/into.rs11
7 files changed, 56 insertions, 21 deletions
diff --git a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs
index 57d2a3c5ce9..0761d63c665 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 41c004ea596..5ccf3806025 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 e8d30034dc4..3e9c02ee268 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 567f65e83d9..b003a504691 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 bbb2f89fda9..b627b0763a2 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<Operand<'tcx>> {
         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<region::Scope>,
         expr: &Expr<'tcx>,
+        local_info: Option<Box<LocalInfo<'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;
 
         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 be0d5d2f1b2..4b40faaf195 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 e30e758e637..1803a18441c 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,