Bubble opaque types into aggregates to produce more precise diagnostics on aggregate constructors

compiler/rustc_borrowck/src/nll.rs

@@ -1,14 +1,15 @@
 //! The entry point of the NLL borrow checker.
 
 use rustc_data_structures::vec_map::VecMap;
+use rustc_hir::def_id::DefId;
 use rustc_index::vec::IndexVec;
 use rustc_infer::infer::InferCtxt;
 use rustc_middle::mir::{create_dump_file, dump_enabled, dump_mir, PassWhere};
 use rustc_middle::mir::{
     BasicBlock, Body, ClosureOutlivesSubject, ClosureRegionRequirements, LocalKind, Location,
     Promoted,
 };
-use rustc_middle::ty::{self, OpaqueHiddenType, OpaqueTypeKey, Region, RegionVid};
+use rustc_middle::ty::{self, OpaqueHiddenType, Region, RegionVid};
 use rustc_span::symbol::sym;
 use std::env;
 use std::fmt::Debug;
@@ -43,7 +44,7 @@ pub type PoloniusOutput = Output<RustcFacts>;
 /// closure requirements to propagate, and any generated errors.
 crate struct NllOutput<'tcx> {
     pub regioncx: RegionInferenceContext<'tcx>,
-    pub opaque_type_values: VecMap<OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>>,
+    pub opaque_type_values: VecMap<DefId, OpaqueHiddenType<'tcx>>,
     pub polonius_input: Option<Box<AllFacts>>,
     pub polonius_output: Option<Rc<PoloniusOutput>>,
     pub opt_closure_req: Option<ClosureRegionRequirements<'tcx>>,
@@ -372,7 +373,7 @@ pub(super) fn dump_annotation<'a, 'tcx>(
     body: &Body<'tcx>,
     regioncx: &RegionInferenceContext<'tcx>,
     closure_region_requirements: &Option<ClosureRegionRequirements<'_>>,
-    opaque_type_values: &VecMap<OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>>,
+    opaque_type_values: &VecMap<DefId, OpaqueHiddenType<'tcx>>,
     errors: &mut crate::error::BorrowckErrors<'tcx>,
 ) {
     let tcx = infcx.tcx;

compiler/rustc_borrowck/src/region_infer/opaque_types.rs

@@ -1,5 +1,6 @@
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::vec_map::VecMap;
+use rustc_hir::def_id::DefId;
 use rustc_hir::OpaqueTyOrigin;
 use rustc_infer::infer::InferCtxt;
 use rustc_middle::ty::subst::GenericArgKind;
@@ -54,8 +55,8 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         &self,
         infcx: &InferCtxt<'_, 'tcx>,
         opaque_ty_decls: VecMap<OpaqueTypeKey<'tcx>, (OpaqueHiddenType<'tcx>, OpaqueTyOrigin)>,
-    ) -> VecMap<OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>> {
-        let mut result: VecMap<OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>> = VecMap::new();
+    ) -> VecMap<DefId, OpaqueHiddenType<'tcx>> {
+        let mut result: VecMap<DefId, OpaqueHiddenType<'tcx>> = VecMap::new();
         for (opaque_type_key, (concrete_type, origin)) in opaque_ty_decls {
             let substs = opaque_type_key.substs;
             debug!(?concrete_type, ?substs);
@@ -124,21 +125,24 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             // back to the opaque type definition. E.g. we may have `OpaqueType<X, Y>` mapped to `(X, Y)`
             // and `OpaqueType<Y, X>` mapped to `(Y, X)`, and those are the same, but we only know that
             // once we convert the generic parameters to those of the opaque type.
-            if let Some(prev) = result.get_mut(&opaque_type_key) {
+            if let Some(prev) = result.get_mut(&opaque_type_key.def_id) {
                 if prev.ty != ty {
-                    let mut err = infcx.tcx.sess.struct_span_err(
-                        concrete_type.span,
-                        &format!("hidden type `{}` differed from previous `{}`", ty, prev.ty),
-                    );
-                    err.span_note(prev.span, "previous hidden type bound here");
-                    err.emit();
+                    if !ty.references_error() {
+                        prev.report_mismatch(
+                            &OpaqueHiddenType { ty, span: concrete_type.span },
+                            infcx.tcx,
+                        );
+                    }
                     prev.ty = infcx.tcx.ty_error();
                 }
                 // Pick a better span if there is one.
                 // FIXME(oli-obk): collect multiple spans for better diagnostics down the road.
                 prev.span = prev.span.substitute_dummy(concrete_type.span);
             } else {
-                result.insert(opaque_type_key, OpaqueHiddenType { ty, span: concrete_type.span });
+                result.insert(
+                    opaque_type_key.def_id,
+                    OpaqueHiddenType { ty, span: concrete_type.span },
+                );
             }
         }
         result

compiler/rustc_middle/src/mir/query.rs

@@ -9,7 +9,6 @@ use rustc_hir as hir;
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_index::bit_set::BitMatrix;
 use rustc_index::vec::IndexVec;
-use rustc_middle::ty::OpaqueTypeKey;
 use rustc_span::Span;
 use rustc_target::abi::VariantIdx;
 use smallvec::SmallVec;
@@ -242,7 +241,7 @@ pub struct BorrowCheckResult<'tcx> {
     /// All the opaque types that are restricted to concrete types
     /// by this function. Unlike the value in `TypeckResults`, this has
     /// unerased regions.
-    pub concrete_opaque_types: VecMap<OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>>,
+    pub concrete_opaque_types: VecMap<DefId, OpaqueHiddenType<'tcx>>,
     pub closure_requirements: Option<ClosureRegionRequirements<'tcx>>,
     pub used_mut_upvars: SmallVec<[Field; 8]>,
     pub tainted_by_errors: Option<ErrorGuaranteed>,

compiler/rustc_middle/src/ty/mod.rs

@@ -1112,6 +1112,26 @@ pub struct OpaqueHiddenType<'tcx> {
     pub ty: Ty<'tcx>,
 }
 
+impl<'tcx> OpaqueHiddenType<'tcx> {
+    pub fn report_mismatch(&self, other: &Self, tcx: TyCtxt<'tcx>) {
+        // Found different concrete types for the opaque type.
+        let mut err = tcx.sess.struct_span_err(
+            other.span,
+            "concrete type differs from previous defining opaque type use",
+        );
+        err.span_label(other.span, format!("expected `{}`, got `{}`", self.ty, other.ty));
+        if self.span == other.span {
+            err.span_label(
+                self.span,
+                "this expression supplies two conflicting concrete types for the same opaque type",
+            );
+        } else {
+            err.span_note(self.span, "previous use here");
+        }
+        err.emit();
+    }
+}
+
 rustc_index::newtype_index! {
     /// "Universes" are used during type- and trait-checking in the
     /// presence of `for<..>` binders to control what sets of names are

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

@@ -94,14 +94,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     ///
     /// Like `as_local_call_operand`, except that the argument will
     /// not be valid once `scope` ends.
+    #[instrument(level = "debug", skip(self))]
     crate fn as_operand(
         &mut self,
         mut block: BasicBlock,
         scope: Option<region::Scope>,
         expr: &Expr<'tcx>,
         local_info: Option<Box<LocalInfo<'tcx>>>,
     ) -> BlockAnd<Operand<'tcx>> {
-        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 {
@@ -113,7 +113,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         }
 
         let category = Category::of(&expr.kind).unwrap();
-        debug!("as_operand: category={:?} for={:?}", category, expr.kind);
+        debug!(?category, ?expr.kind);
         match category {
             Category::Constant => {
                 let constant = this.as_constant(expr);
@@ -129,13 +129,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         }
     }
 
+    #[instrument(level = "debug", skip(self))]
     crate fn as_call_operand(
         &mut self,
         mut block: BasicBlock,
         scope: Option<region::Scope>,
         expr: &Expr<'tcx>,
     ) -> BlockAnd<Operand<'tcx>> {
-        debug!("as_call_operand(block={:?}, expr={:?})", block, expr);
         let this = self;
 
         if let ExprKind::Scope { region_scope, lint_level, value } = expr.kind {

compiler/rustc_mir_build/src/build/expr/as_place.rs

@@ -420,15 +420,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         self.expr_as_place(block, expr, Mutability::Not, None)
     }
 
+    #[instrument(level = "debug", skip(self))]
     fn expr_as_place(
         &mut self,
         mut block: BasicBlock,
         expr: &Expr<'tcx>,
         mutability: Mutability,
         fake_borrow_temps: Option<&mut Vec<Local>>,
     ) -> BlockAnd<PlaceBuilder<'tcx>> {
-        debug!("expr_as_place(block={:?}, expr={:?}, mutability={:?})", block, expr, mutability);
-
         let this = self;
         let expr_span = expr.span;
         let source_info = this.source_info(expr_span);

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

@@ -31,14 +31,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     }
 
     /// Compile `expr`, yielding an rvalue.
+    #[instrument(level = "debug", skip(self))]
     crate fn as_rvalue(
         &mut self,
         mut block: BasicBlock,
         scope: Option<region::Scope>,
         expr: &Expr<'tcx>,
     ) -> BlockAnd<Rvalue<'tcx>> {
-        debug!("expr_as_rvalue(block={:?}, scope={:?}, expr={:?})", block, scope, expr);
-
         let this = self;
         let expr_span = expr.span;
         let source_info = this.source_info(expr_span);

compiler/rustc_mir_build/src/build/expr/as_temp.rs

@@ -6,10 +6,12 @@ use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_middle::middle::region;
 use rustc_middle::mir::*;
 use rustc_middle::thir::*;
+use rustc_middle::ty::Ty;
 
 impl<'a, 'tcx> Builder<'a, 'tcx> {
     /// Compile `expr` into a fresh temporary. This is used when building
     /// up rvalues so as to freeze the value that will be consumed.
+    #[instrument(level = "debug", skip(self))]
     crate fn as_temp(
         &mut self,
         block: BasicBlock,
@@ -20,7 +22,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         // this is the only place in mir building that we need to truly need to worry about
         // infinite recursion. Everything else does recurse, too, but it always gets broken up
         // at some point by inserting an intermediate temporary
-        ensure_sufficient_stack(|| self.as_temp_inner(block, temp_lifetime, expr, mutability))
+        ensure_sufficient_stack(|| {
+            self.as_temp_inner(block, temp_lifetime, expr, mutability, expr.ty)
+        })
     }
 
     fn as_temp_inner(
@@ -29,22 +33,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         temp_lifetime: Option<region::Scope>,
         expr: &Expr<'tcx>,
         mutability: Mutability,
+        expr_ty: Ty<'tcx>,
     ) -> BlockAnd<Local> {
-        debug!(
-            "as_temp(block={:?}, temp_lifetime={:?}, expr={:?}, mutability={:?})",
-            block, temp_lifetime, expr, mutability
-        );
         let this = self;
 
         let expr_span = expr.span;
         let source_info = this.source_info(expr_span);
         if let ExprKind::Scope { region_scope, lint_level, value } = expr.kind {
             return this.in_scope((region_scope, source_info), lint_level, |this| {
-                this.as_temp(block, temp_lifetime, &this.thir[value], mutability)
+                this.as_temp_inner(block, temp_lifetime, &this.thir[value], mutability, expr.ty)
             });
         }
 
-        let expr_ty = expr.ty;
         let temp = {
             let mut local_decl = LocalDecl::new(expr_ty, expr_span);
             if mutability == Mutability::Not {

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

@@ -15,14 +15,13 @@ use std::iter;
 impl<'a, 'tcx> Builder<'a, 'tcx> {
     /// Compile `expr`, storing the result into `destination`, which
     /// is assumed to be uninitialized.
+    #[instrument(level = "debug", skip(self))]
     crate fn expr_into_dest(
         &mut self,
         destination: Place<'tcx>,
         mut block: BasicBlock,
         expr: &Expr<'tcx>,
     ) -> BlockAnd<()> {
-        debug!("expr_into_dest(destination={:?}, block={:?}, expr={:?})", destination, block, expr);
-
         // since we frequently have to reference `self` from within a
         // closure, where `self` would be shadowed, it's easier to
         // just use the name `this` uniformly

compiler/rustc_mir_build/src/build/expr/stmt.rs

@@ -10,6 +10,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     /// (e.g., `some().code(&here());`) then `opt_stmt_span` is the
     /// span of that statement (including its semicolon, if any).
     /// The scope is used if a statement temporary must be dropped.
+    #[instrument(level = "debug", skip(self))]
     crate fn stmt_expr(
         &mut self,
         mut block: BasicBlock,

compiler/rustc_mir_build/src/thir/cx/expr.rs

@@ -1,3 +1,4 @@
+use crate::build::ExprCategory as Category;
 use crate::thir::cx::Cx;
 use crate::thir::util::UserAnnotatedTyHelpers;
 use rustc_data_structures::stack::ensure_sufficient_stack;
@@ -24,20 +25,26 @@ use rustc_target::abi::VariantIdx;
 impl<'tcx> Cx<'tcx> {
     crate fn mirror_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) -> ExprId {
         // `mirror_expr` is recursing very deep. Make sure the stack doesn't overflow.
-        ensure_sufficient_stack(|| self.mirror_expr_inner(expr))
+        ensure_sufficient_stack(|| self.mirror_expr_inner(expr, None))
     }
 
-    crate fn mirror_exprs(&mut self, exprs: &'tcx [hir::Expr<'tcx>]) -> Box<[ExprId]> {
-        exprs.iter().map(|expr| self.mirror_expr_inner(expr)).collect()
+    crate fn mirror_exprs(
+        &mut self,
+        exprs: impl Iterator<Item = (&'tcx hir::Expr<'tcx>, Option<Ty<'tcx>>)>,
+    ) -> Box<[ExprId]> {
+        exprs.map(|(expr, ty)| self.mirror_expr_inner(expr, ty)).collect()
     }
 
-    pub(super) fn mirror_expr_inner(&mut self, hir_expr: &'tcx hir::Expr<'tcx>) -> ExprId {
+    #[instrument(level = "debug", skip(self))]
+    pub(super) fn mirror_expr_inner(
+        &mut self,
+        hir_expr: &'tcx hir::Expr<'tcx>,
+        ty: Option<Ty<'tcx>>,
+    ) -> ExprId {
         let temp_lifetime = self.region_scope_tree.temporary_scope(hir_expr.hir_id.local_id);
         let expr_scope =
             region::Scope { id: hir_expr.hir_id.local_id, data: region::ScopeData::Node };
 
-        debug!("Expr::make_mirror(): id={}, span={:?}", hir_expr.hir_id, hir_expr.span);
-
         let mut expr = self.make_mirror_unadjusted(hir_expr);
 
         let adjustment_span = match self.adjustment_span {
@@ -47,12 +54,16 @@ impl<'tcx> Cx<'tcx> {
 
         // Now apply adjustments, if any.
         for adjustment in self.typeck_results.expr_adjustments(hir_expr) {
-            debug!("make_mirror: expr={:?} applying adjustment={:?}", expr, adjustment);
+            debug!(?expr, ?adjustment);
             let span = expr.span;
             expr =
                 self.apply_adjustment(hir_expr, expr, adjustment, adjustment_span.unwrap_or(span));
         }
 
+        if !matches!(Category::of(&expr.kind), Some(Category::Constant)) {
+            expr.ty = ty.unwrap_or(expr.ty);
+        }
+
         // Next, wrap this up in the expr's scope.
         expr = Expr {
             temp_lifetime,
@@ -172,7 +183,7 @@ impl<'tcx> Cx<'tcx> {
                 // is guaranteed to exist, since a method call always has a receiver.
                 let old_adjustment_span = self.adjustment_span.replace((args[0].hir_id, expr_span));
                 tracing::info!("Using method span: {:?}", expr.span);
-                let args = self.mirror_exprs(args);
+                let args = self.mirror_exprs(args.iter().zip(std::iter::repeat(None)));
                 self.adjustment_span = old_adjustment_span;
                 ExprKind::Call {
                     ty: expr.ty,
@@ -194,12 +205,16 @@ impl<'tcx> Cx<'tcx> {
 
                     let method = self.method_callee(expr, fun.span, None);
 
-                    let arg_tys = args.iter().map(|e| self.typeck_results().expr_ty_adjusted(e));
+                    let arg_tys: Vec<_> =
+                        args.iter().map(|e| self.typeck_results().expr_ty_adjusted(e)).collect();
                     let tupled_args = Expr {
-                        ty: self.tcx.mk_tup(arg_tys),
+                        ty: self.tcx.mk_tup(arg_tys.iter()),
                         temp_lifetime,
                         span: expr.span,
-                        kind: ExprKind::Tuple { fields: self.mirror_exprs(args) },
+                        kind: ExprKind::Tuple {
+                            fields: self
+                                .mirror_exprs(args.iter().zip(arg_tys.into_iter().map(Some))),
+                        },
                     };
                     let tupled_args = self.thir.exprs.push(tupled_args);
 
@@ -256,7 +271,7 @@ impl<'tcx> Cx<'tcx> {
                         ExprKind::Call {
                             ty: self.typeck_results().node_type(fun.hir_id),
                             fun: self.mirror_expr(fun),
-                            args: self.mirror_exprs(args),
+                            args: self.mirror_exprs(args.iter().zip(std::iter::repeat(None))),
                             from_hir_call: true,
                             fn_span: expr.span,
                         }
@@ -763,10 +778,17 @@ impl<'tcx> Cx<'tcx> {
                 ExprKind::Use { source: self.mirror_expr(source) }
             }
             hir::ExprKind::Box(ref value) => ExprKind::Box { value: self.mirror_expr(value) },
-            hir::ExprKind::Array(ref fields) => {
-                ExprKind::Array { fields: self.mirror_exprs(fields) }
-            }
-            hir::ExprKind::Tup(ref fields) => ExprKind::Tuple { fields: self.mirror_exprs(fields) },
+            hir::ExprKind::Array(ref fields) => ExprKind::Array {
+                fields: self.mirror_exprs(
+                    fields
+                        .iter()
+                        .zip(std::iter::repeat(Some(expr_ty.sequence_element_type(self.tcx)))),
+                ),
+            },
+            hir::ExprKind::Tup(ref fields) => ExprKind::Tuple {
+                fields: self
+                    .mirror_exprs(fields.iter().zip(expr_ty.tuple_fields().iter().map(Some))),
+            },
 
             hir::ExprKind::Yield(ref v, _) => ExprKind::Yield { value: self.mirror_expr(v) },
             hir::ExprKind::Err => unreachable!(),