Fix soundness bug with type alias impl trait

compiler/rustc_mir/src/borrow_check/type_check/mod.rs

@@ -1307,7 +1307,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
                         };
                         debug!("opaque_defn_ty = {:?}", opaque_defn_ty);
                         let subst_opaque_defn_ty =
-                            opaque_defn_ty.concrete_type.subst(tcx, opaque_decl.substs);
+                            opaque_defn_ty.concrete_type.subst(tcx, opaque_decl.substs[0]);
                         let renumbered_opaque_defn_ty =
                             renumber::renumber_regions(infcx, subst_opaque_defn_ty);
 
@@ -1328,7 +1328,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
                                 opaque_def_id,
                                 ty::ResolvedOpaqueTy {
                                     concrete_type: renumbered_opaque_defn_ty,
-                                    substs: opaque_decl.substs,
+                                    substs: opaque_decl.substs[0],
                                 },
                             ));
                         } else {

compiler/rustc_trait_selection/src/opaque_types.rs

@@ -13,6 +13,7 @@ use rustc_middle::ty::fold::{BottomUpFolder, TypeFoldable, TypeFolder, TypeVisit
 use rustc_middle::ty::subst::{GenericArg, GenericArgKind, InternalSubsts, Subst, SubstsRef};
 use rustc_middle::ty::{self, Ty, TyCtxt};
 use rustc_span::Span;
+use smallvec::{smallvec, SmallVec};
 
 use std::ops::ControlFlow;
 
@@ -21,7 +22,7 @@ pub type OpaqueTypeMap<'tcx> = DefIdMap<OpaqueTypeDecl<'tcx>>;
 /// Information about the opaque types whose values we
 /// are inferring in this function (these are the `impl Trait` that
 /// appear in the return type).
-#[derive(Copy, Clone, Debug)]
+#[derive(Clone, Debug)]
 pub struct OpaqueTypeDecl<'tcx> {
     /// The opaque type (`ty::Opaque`) for this declaration.
     pub opaque_type: Ty<'tcx>,
@@ -37,7 +38,11 @@ pub struct OpaqueTypeDecl<'tcx> {
     ///     fn foo<'a, 'b, T>() -> Foo<'a, T>
     ///
     /// then `substs` would be `['a, T]`.
-    pub substs: SubstsRef<'tcx>,
+    ///
+    /// In case there are multiple conflicting substs an error has already
+    /// been reported, but we still store the additional substs here in order
+    /// to allow for better diagnostics later.
+    pub substs: SmallVec<[SubstsRef<'tcx>; 1]>,
 
     /// The span of this particular definition of the opaque type. So
     /// for example:
@@ -95,6 +100,7 @@ pub struct OpaqueTypeDecl<'tcx> {
 }
 
 /// Whether member constraints should be generated for all opaque types
+#[derive(Debug)]
 pub enum GenerateMemberConstraints {
     /// The default, used by typeck
     WhenRequired,
@@ -183,6 +189,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
     ///   obligations
     /// - `value` -- the value within which we are instantiating opaque types
     /// - `value_span` -- the span where the value came from, used in error reporting
+    #[instrument(level = "debug", skip(self))]
+    // FIXME(oli-obk): this function is invoked twice: once with the crate root, and then for each body that
+    // actually could be a defining use. It is unclear to me why we run all of it twice. Figure out what
+    // happens and document that or fix it.
     fn instantiate_opaque_types<T: TypeFoldable<'tcx>>(
         &self,
         parent_def_id: LocalDefId,
@@ -191,11 +201,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         value: T,
         value_span: Span,
     ) -> InferOk<'tcx, (T, OpaqueTypeMap<'tcx>)> {
-        debug!(
-            "instantiate_opaque_types(value={:?}, parent_def_id={:?}, body_id={:?}, \
-             param_env={:?}, value_span={:?})",
-            value, parent_def_id, body_id, param_env, value_span,
-        );
         let mut instantiator = Instantiator {
             infcx: self,
             parent_def_id,
@@ -389,22 +394,19 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
     }
 
     /// See `constrain_opaque_types` for documentation.
+    #[instrument(level = "debug", skip(self, free_region_relations))]
     fn constrain_opaque_type<FRR: FreeRegionRelations<'tcx>>(
         &self,
         def_id: DefId,
         opaque_defn: &OpaqueTypeDecl<'tcx>,
         mode: GenerateMemberConstraints,
         free_region_relations: &FRR,
     ) {
-        debug!("constrain_opaque_type()");
-        debug!("constrain_opaque_type: def_id={:?}", def_id);
-        debug!("constrain_opaque_type: opaque_defn={:#?}", opaque_defn);
-
         let tcx = self.tcx;
 
         let concrete_ty = self.resolve_vars_if_possible(opaque_defn.concrete_ty);
 
-        debug!("constrain_opaque_type: concrete_ty={:?}", concrete_ty);
+        debug!(?concrete_ty);
 
         let first_own_region = match opaque_defn.origin {
             hir::OpaqueTyOrigin::FnReturn | hir::OpaqueTyOrigin::AsyncFn => {
@@ -432,11 +434,11 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         // If there are required region bounds, we can use them.
         if opaque_defn.has_required_region_bounds {
             let bounds = tcx.explicit_item_bounds(def_id);
-            debug!("constrain_opaque_type: predicates: {:#?}", bounds);
+            debug!(?bounds, "predicates");
             let bounds: Vec<_> =
-                bounds.iter().map(|(bound, _)| bound.subst(tcx, opaque_defn.substs)).collect();
-            debug!("constrain_opaque_type: bounds={:#?}", bounds);
-            let opaque_type = tcx.mk_opaque(def_id, opaque_defn.substs);
+                bounds.iter().map(|(bound, _)| bound.subst(tcx, opaque_defn.substs[0])).collect();
+            debug!(?bounds);
+            let opaque_type = tcx.mk_opaque(def_id, opaque_defn.substs[0]);
 
             let required_region_bounds =
                 required_region_bounds(tcx, opaque_type, bounds.into_iter());
@@ -462,15 +464,15 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         // second.
         let mut least_region = None;
 
-        for subst_arg in &opaque_defn.substs[first_own_region..] {
+        for subst_arg in &opaque_defn.substs[0][first_own_region..] {
             let subst_region = match subst_arg.unpack() {
                 GenericArgKind::Lifetime(r) => r,
                 GenericArgKind::Type(_) | GenericArgKind::Const(_) => continue,
             };
 
             // Compute the least upper bound of it with the other regions.
-            debug!("constrain_opaque_types: least_region={:?}", least_region);
-            debug!("constrain_opaque_types: subst_region={:?}", subst_region);
+            debug!(?least_region);
+            debug!(?subst_region);
             match least_region {
                 None => least_region = Some(subst_region),
                 Some(lr) => {
@@ -535,7 +537,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         // type can be equal to any of the region parameters of the
         // opaque type definition.
         let choice_regions: Lrc<Vec<ty::Region<'tcx>>> = Lrc::new(
-            opaque_defn.substs[first_own_region..]
+            opaque_defn.substs[0][first_own_region..]
                 .iter()
                 .filter_map(|arg| match arg.unpack() {
                     GenericArgKind::Lifetime(r) => Some(r),
@@ -997,8 +999,8 @@ struct Instantiator<'a, 'tcx> {
 }
 
 impl<'a, 'tcx> Instantiator<'a, 'tcx> {
+    #[instrument(level = "debug", skip(self))]
     fn instantiate_opaque_types_in_map<T: TypeFoldable<'tcx>>(&mut self, value: T) -> T {
-        debug!("instantiate_opaque_types_in_map(value={:?})", value);
         let tcx = self.infcx.tcx;
         value.fold_with(&mut BottomUpFolder {
             tcx,
@@ -1075,12 +1077,7 @@ impl<'a, 'tcx> Instantiator<'a, 'tcx> {
                             return self.fold_opaque_ty(ty, def_id.to_def_id(), substs, origin);
                         }
 
-                        debug!(
-                            "instantiate_opaque_types_in_map: \
-                             encountered opaque outside its definition scope \
-                             def_id={:?}",
-                            def_id,
-                        );
+                        debug!(?def_id, "encountered opaque outside its definition scope");
                     }
                 }
 
@@ -1091,6 +1088,7 @@ impl<'a, 'tcx> Instantiator<'a, 'tcx> {
         })
     }
 
+    #[instrument(level = "debug", skip(self))]
     fn fold_opaque_ty(
         &mut self,
         ty: Ty<'tcx>,
@@ -1101,21 +1099,45 @@ impl<'a, 'tcx> Instantiator<'a, 'tcx> {
         let infcx = self.infcx;
         let tcx = infcx.tcx;
 
-        debug!("instantiate_opaque_types: Opaque(def_id={:?}, substs={:?})", def_id, substs);
-
         // Use the same type variable if the exact same opaque type appears more
-        // than once in the return type (e.g., if it's passed to a type alias).
-        if let Some(opaque_defn) = self.opaque_types.get(&def_id) {
-            debug!("instantiate_opaque_types: returning concrete ty {:?}", opaque_defn.concrete_ty);
-            return opaque_defn.concrete_ty;
+        // than once in a function (e.g., if it's passed to a type alias).
+        if let Some(opaque_defn) = self.opaque_types.get_mut(&def_id) {
+            debug!(?opaque_defn, "found already known concrete type");
+            if opaque_defn.substs.contains(&substs) {
+                // Already seen this concrete type
+                return opaque_defn.concrete_ty;
+            } else {
+                // Don't emit multiple errors for the same set of substs
+                opaque_defn.substs.push(substs);
+                opaque_defn.concrete_ty = tcx.ty_error_with_message(
+                    self.value_span,
+                    "defining use generics differ from previous defining use",
+                );
+                tcx.sess
+                    .struct_span_err(
+                        self.value_span,
+                        &format!(
+                            "defining use generics `{:?}` differ from previous defining use",
+                            substs
+                        ),
+                    )
+                    .span_note(
+                        opaque_defn.definition_span,
+                        &format!(
+                            "previous defining use with different generics `{:?}` found here",
+                            opaque_defn.substs[0]
+                        ),
+                    )
+                    .emit();
+            }
         }
         let span = tcx.def_span(def_id);
-        debug!("fold_opaque_ty {:?} {:?}", self.value_span, span);
+        debug!(?self.value_span, ?span);
         let ty_var = infcx
             .next_ty_var(TypeVariableOrigin { kind: TypeVariableOriginKind::TypeInference, span });
 
         let item_bounds = tcx.explicit_item_bounds(def_id);
-        debug!("instantiate_opaque_types: bounds={:#?}", item_bounds);
+        debug!(?item_bounds);
         let bounds: Vec<_> =
             item_bounds.iter().map(|(bound, _)| bound.subst(tcx, substs)).collect();
 
@@ -1124,16 +1146,16 @@ impl<'a, 'tcx> Instantiator<'a, 'tcx> {
             infcx.partially_normalize_associated_types_in(span, self.body_id, param_env, bounds);
         self.obligations.extend(obligations);
 
-        debug!("instantiate_opaque_types: bounds={:?}", bounds);
+        debug!(?bounds);
 
         let required_region_bounds = required_region_bounds(tcx, ty, bounds.iter().copied());
-        debug!("instantiate_opaque_types: required_region_bounds={:?}", required_region_bounds);
+        debug!(?required_region_bounds);
 
         // Make sure that we are in fact defining the *entire* type
         // (e.g., `type Foo<T: Bound> = impl Bar;` needs to be
         // defined by a function like `fn foo<T: Bound>() -> Foo<T>`).
-        debug!("instantiate_opaque_types: param_env={:#?}", self.param_env,);
-        debug!("instantiate_opaque_types: generics={:#?}", tcx.generics_of(def_id),);
+        debug!(?self.param_env,);
+        debug!(generics= ?tcx.generics_of(def_id),);
 
         // Ideally, we'd get the span where *this specific `ty` came
         // from*, but right now we just use the span from the overall
@@ -1142,18 +1164,22 @@ impl<'a, 'tcx> Instantiator<'a, 'tcx> {
         // Foo, impl Bar)`.
         let definition_span = self.value_span;
 
-        self.opaque_types.insert(
-            def_id,
-            OpaqueTypeDecl {
-                opaque_type: ty,
-                substs,
-                definition_span,
-                concrete_ty: ty_var,
-                has_required_region_bounds: !required_region_bounds.is_empty(),
-                origin,
-            },
-        );
-        debug!("instantiate_opaque_types: ty_var={:?}", ty_var);
+        // We only keep the first concrete type var, as we will already error
+        // out if there are multiple due to the conflicting obligations
+        if !self.opaque_types.contains_key(&def_id) {
+            self.opaque_types.insert(
+                def_id,
+                OpaqueTypeDecl {
+                    opaque_type: ty,
+                    substs: smallvec![substs],
+                    definition_span,
+                    concrete_ty: ty_var,
+                    has_required_region_bounds: !required_region_bounds.is_empty(),
+                    origin,
+                },
+            );
+        }
+        debug!(?ty_var);
 
         for predicate in &bounds {
             if let ty::PredicateKind::Projection(projection) = predicate.kind().skip_binder() {

compiler/rustc_typeck/src/check/check.rs

@@ -741,7 +741,7 @@ fn check_opaque_meets_bounds<'tcx>(
         for (def_id, opaque_defn) in opaque_type_map {
             match infcx
                 .at(&misc_cause, param_env)
-                .eq(opaque_defn.concrete_ty, tcx.type_of(def_id).subst(tcx, opaque_defn.substs))
+                .eq(opaque_defn.concrete_ty, tcx.type_of(def_id).subst(tcx, opaque_defn.substs[0]))
             {
                 Ok(infer_ok) => inh.register_infer_ok_obligations(infer_ok),
                 Err(ty_err) => tcx.sess.delay_span_bug(

compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs

@@ -399,8 +399,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     }
                 }
             }
-            let _ = opaque_types.insert(ty, decl);
             let _ = opaque_types_vars.insert(decl.concrete_ty, decl.opaque_type);
+            let _ = opaque_types.insert(ty, decl);
         }
 
         value

compiler/rustc_typeck/src/check/wfcheck.rs

@@ -915,19 +915,19 @@ fn check_fn_or_method<'fcx, 'tcx>(
 /// fn b<T>() -> Foo<T, u32> { .. }
 /// ```
 ///
+#[instrument(level = "debug", skip(tcx, fcx))]
 fn check_opaque_types<'fcx, 'tcx>(
     tcx: TyCtxt<'tcx>,
     fcx: &FnCtxt<'fcx, 'tcx>,
     fn_def_id: LocalDefId,
     span: Span,
     ty: Ty<'tcx>,
 ) {
-    trace!("check_opaque_types(ty={:?})", ty);
     ty.fold_with(&mut ty::fold::BottomUpFolder {
         tcx: fcx.tcx,
         ty_op: |ty| {
             if let ty::Opaque(def_id, substs) = *ty.kind() {
-                trace!("check_opaque_types: opaque_ty, {:?}, {:?}", def_id, substs);
+                trace!(?def_id, ?substs, "opaque type");
                 let generics = tcx.generics_of(def_id);
 
                 let opaque_hir_id = if let Some(local_id) = def_id.as_local() {
@@ -965,7 +965,7 @@ fn check_opaque_types<'fcx, 'tcx>(
                 if !may_define_opaque_type(tcx, fn_def_id, opaque_hir_id) {
                     return ty;
                 }
-                trace!("check_opaque_types: may define, generics={:#?}", generics);
+                trace!(?generics, "may define");
                 let mut seen_params: FxHashMap<_, Vec<_>> = FxHashMap::default();
                 for (i, arg) in substs.iter().enumerate() {
                     let arg_is_param = match arg.unpack() {

compiler/rustc_typeck/src/check/writeback.rs

@@ -515,7 +515,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
             // figures out the concrete type with `U`, but the stored type is with `T`.
             let definition_ty = self.fcx.infer_opaque_definition_from_instantiation(
                 def_id,
-                opaque_defn.substs,
+                opaque_defn.substs[0],
                 instantiated_ty,
                 span,
             );
@@ -535,7 +535,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
                 }
             }
 
-            if !opaque_defn.substs.needs_infer() {
+            if !opaque_defn.substs[0].needs_infer() {
                 // We only want to add an entry into `concrete_opaque_types`
                 // if we actually found a defining usage of this opaque type.
                 // Otherwise, we do nothing - we'll either find a defining usage
@@ -544,12 +544,15 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
                 if !skip_add {
                     let new = ty::ResolvedOpaqueTy {
                         concrete_type: definition_ty,
-                        substs: opaque_defn.substs,
+                        substs: opaque_defn.substs[0],
                     };
 
+                    debug!(?def_id, ?new, "inserting opaque type resolution");
                     let old = self.typeck_results.concrete_opaque_types.insert(def_id, new);
                     if let Some(old) = old {
-                        if old.concrete_type != definition_ty || old.substs != opaque_defn.substs {
+                        debug!(?old, "duplicate insertion");
+                        if old.concrete_type != definition_ty || old.substs != opaque_defn.substs[0]
+                        {
                             span_bug!(
                                 span,
                                 "`visit_opaque_types` tried to write different types for the same \

compiler/rustc_typeck/src/collect/type_of.rs

@@ -540,7 +540,10 @@ fn find_opaque_ty_constraints(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Ty<'_> {
                 }
 
                 if let Some((prev_span, prev_ty)) = self.found {
-                    if *concrete_type != prev_ty {
+                    if *concrete_type != prev_ty
+                        && !concrete_type.references_error()
+                        && !prev_ty.references_error()
+                    {
                         debug!("find_opaque_ty_constraints: span={:?}", span);
                         // Found different concrete types for the opaque type.
                         let mut err = self.tcx.sess.struct_span_err(