Rollup merge of #112876 - compiler-errors:check-subst-compat-in-OpaqueTypeCollector, r=oli-obk

matthiaskrgr · web-flow · commit cc93c5fca098 · 2023-06-22T06:29:33.000+02:00
Don't substitute a GAT that has mismatched generics in `OpaqueTypeCollector` Fixes #111828 I didn't put up minimized UI tests for #112510 or #112873 because they'd minimize to literally the same code, but with different substs on the trait/impl. I don't think that warrants duplicate tests given the nature of the fix. r? `@oli-obk` ---- Side-note: I checked, and this isn't fixed by #112652 -- I think we discussed whether or not that PR fixed it either intentionally or by accident. The code here isn't really touched by that PR either as far as I can tell? Also, sorry, did some other drive-bys. Hope it doesn't make rebasing #112652 too difficult 😅
diff --git a/compiler/rustc_trait_selection/src/traits/mod.rs b/compiler/rustc_trait_selection/src/traits/mod.rs
@@ -65,12 +65,12 @@ pub use self::specialize::{
 pub use self::structural_match::search_for_structural_match_violation;
 pub use self::structural_normalize::StructurallyNormalizeExt;
 pub use self::util::elaborate;
-pub use self::util::{expand_trait_aliases, TraitAliasExpander};
-pub use self::util::{get_vtable_index_of_object_method, impl_item_is_final, upcast_choices};
 pub use self::util::{
-    supertrait_def_ids, supertraits, transitive_bounds, transitive_bounds_that_define_assoc_item,
-    SupertraitDefIds,
+    check_substs_compatible, supertrait_def_ids, supertraits, transitive_bounds,
+    transitive_bounds_that_define_assoc_item, SupertraitDefIds,
 };
+pub use self::util::{expand_trait_aliases, TraitAliasExpander};
+pub use self::util::{get_vtable_index_of_object_method, impl_item_is_final, upcast_choices};
 
 pub use self::chalk_fulfill::FulfillmentContext as ChalkFulfillmentContext;
 
diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs
@@ -1,5 +1,6 @@
 //! Code for projecting associated types out of trait references.
 
+use super::check_substs_compatible;
 use super::specialization_graph;
 use super::translate_substs;
 use super::util;
@@ -2378,47 +2379,6 @@ fn confirm_impl_candidate<'cx, 'tcx>(
     }
 }
 
-// Verify that the trait item and its implementation have compatible substs lists
-fn check_substs_compatible<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    assoc_item: ty::AssocItem,
-    substs: ty::SubstsRef<'tcx>,
-) -> bool {
-    fn check_substs_compatible_inner<'tcx>(
-        tcx: TyCtxt<'tcx>,
-        generics: &'tcx ty::Generics,
-        args: &'tcx [ty::GenericArg<'tcx>],
-    ) -> bool {
-        if generics.count() != args.len() {
-            return false;
-        }
-
-        let (parent_args, own_args) = args.split_at(generics.parent_count);
-
-        if let Some(parent) = generics.parent
-            && let parent_generics = tcx.generics_of(parent)
-            && !check_substs_compatible_inner(tcx, parent_generics, parent_args) {
-            return false;
-        }
-
-        for (param, arg) in std::iter::zip(&generics.params, own_args) {
-            match (&param.kind, arg.unpack()) {
-                (ty::GenericParamDefKind::Type { .. }, ty::GenericArgKind::Type(_))
-                | (ty::GenericParamDefKind::Lifetime, ty::GenericArgKind::Lifetime(_))
-                | (ty::GenericParamDefKind::Const { .. }, ty::GenericArgKind::Const(_)) => {}
-                _ => return false,
-            }
-        }
-
-        true
-    }
-
-    let generics = tcx.generics_of(assoc_item.def_id);
-    // Chop off any additional substs (RPITIT) substs
-    let substs = &substs[0..generics.count().min(substs.len())];
-    check_substs_compatible_inner(tcx, generics, substs)
-}
-
 fn confirm_impl_trait_in_trait_candidate<'tcx>(
     selcx: &mut SelectionContext<'_, 'tcx>,
     obligation: &ProjectionTyObligation<'tcx>,
diff --git a/compiler/rustc_trait_selection/src/traits/util.rs b/compiler/rustc_trait_selection/src/traits/util.rs
@@ -302,3 +302,44 @@ pub enum TupleArgumentsFlag {
     Yes,
     No,
 }
+
+// Verify that the trait item and its implementation have compatible substs lists
+pub fn check_substs_compatible<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    assoc_item: ty::AssocItem,
+    substs: ty::SubstsRef<'tcx>,
+) -> bool {
+    fn check_substs_compatible_inner<'tcx>(
+        tcx: TyCtxt<'tcx>,
+        generics: &'tcx ty::Generics,
+        args: &'tcx [ty::GenericArg<'tcx>],
+    ) -> bool {
+        if generics.count() != args.len() {
+            return false;
+        }
+
+        let (parent_args, own_args) = args.split_at(generics.parent_count);
+
+        if let Some(parent) = generics.parent
+            && let parent_generics = tcx.generics_of(parent)
+            && !check_substs_compatible_inner(tcx, parent_generics, parent_args) {
+            return false;
+        }
+
+        for (param, arg) in std::iter::zip(&generics.params, own_args) {
+            match (&param.kind, arg.unpack()) {
+                (ty::GenericParamDefKind::Type { .. }, ty::GenericArgKind::Type(_))
+                | (ty::GenericParamDefKind::Lifetime, ty::GenericArgKind::Lifetime(_))
+                | (ty::GenericParamDefKind::Const { .. }, ty::GenericArgKind::Const(_)) => {}
+                _ => return false,
+            }
+        }
+
+        true
+    }
+
+    let generics = tcx.generics_of(assoc_item.def_id);
+    // Chop off any additional substs (RPITIT) substs
+    let substs = &substs[0..generics.count().min(substs.len())];
+    check_substs_compatible_inner(tcx, generics, substs)
+}
diff --git a/compiler/rustc_ty_utils/src/opaque_types.rs b/compiler/rustc_ty_utils/src/opaque_types.rs
@@ -6,7 +6,7 @@ use rustc_middle::ty::util::{CheckRegions, NotUniqueParam};
 use rustc_middle::ty::{self, Ty, TyCtxt};
 use rustc_middle::ty::{TypeSuperVisitable, TypeVisitable, TypeVisitor};
 use rustc_span::Span;
-use rustc_type_ir::AliasKind;
+use rustc_trait_selection::traits::check_substs_compatible;
 use std::ops::ControlFlow;
 
 use crate::errors::{DuplicateArg, NotParam};
@@ -36,6 +36,15 @@ impl<'tcx> OpaqueTypeCollector<'tcx> {
         self.tcx.def_span(self.item)
     }
 
+    fn parent_trait_ref(&self) -> Option<ty::TraitRef<'tcx>> {
+        let parent = self.parent()?;
+        if matches!(self.tcx.def_kind(parent), DefKind::Impl { .. }) {
+            Some(self.tcx.impl_trait_ref(parent)?.subst_identity())
+        } else {
+            None
+        }
+    }
+
     fn parent(&self) -> Option<LocalDefId> {
         match self.tcx.def_kind(self.item) {
             DefKind::Fn => None,
@@ -56,7 +65,7 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for OpaqueTypeCollector<'tcx> {
     #[instrument(skip(self), ret, level = "trace")]
     fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<ErrorGuaranteed> {
         match t.kind() {
-            ty::Alias(AliasKind::Opaque, alias_ty) if alias_ty.def_id.is_local() => {
+            ty::Alias(ty::Opaque, alias_ty) if alias_ty.def_id.is_local() => {
                 if !self.seen.insert(alias_ty.def_id.expect_local()) {
                     return ControlFlow::Continue(());
                 }
@@ -98,37 +107,48 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for OpaqueTypeCollector<'tcx> {
                     }
                 }
             }
-            ty::Alias(AliasKind::Projection, alias_ty) => {
-                if let Some(parent) = self.parent() {
-                    trace!(?alias_ty);
-                    let (trait_ref, own_substs) = alias_ty.trait_ref_and_own_substs(self.tcx);
-
-                    trace!(?trait_ref, ?own_substs);
-                    // This avoids having to do normalization of `Self::AssocTy` by only
-                    // supporting the case of a method defining opaque types from assoc types
-                    // in the same impl block.
-                    if trait_ref.self_ty() == self.tcx.type_of(parent).subst_identity() {
-                        for assoc in self.tcx.associated_items(parent).in_definition_order() {
+            ty::Alias(ty::Projection, alias_ty) => {
+                // This avoids having to do normalization of `Self::AssocTy` by only
+                // supporting the case of a method defining opaque types from assoc types
+                // in the same impl block.
+                if let Some(parent_trait_ref) = self.parent_trait_ref() {
+                    // If the trait ref of the associated item and the impl differs,
+                    // then we can't use the impl's identity substitutions below, so
+                    // just skip.
+                    if alias_ty.trait_ref(self.tcx) == parent_trait_ref {
+                        let parent = self.parent().expect("we should have a parent here");
+
+                        for &assoc in self.tcx.associated_items(parent).in_definition_order() {
                             trace!(?assoc);
-                            if assoc.trait_item_def_id == Some(alias_ty.def_id) {
-                                // We reconstruct the generic args of the associated type within the impl
-                                // from the impl's generics and the generic args passed to the type via the
-                                // projection.
-                                let substs = ty::InternalSubsts::identity_for_item(
-                                    self.tcx,
-                                    parent.to_def_id(),
+                            if assoc.trait_item_def_id != Some(alias_ty.def_id) {
+                                continue;
+                            }
+
+                            // If the type is further specializable, then the type_of
+                            // is not actually correct below.
+                            if !assoc.defaultness(self.tcx).is_final() {
+                                continue;
+                            }
+
+                            let impl_substs = alias_ty.substs.rebase_onto(
+                                self.tcx,
+                                parent_trait_ref.def_id,
+                                ty::InternalSubsts::identity_for_item(self.tcx, parent),
+                            );
+
+                            if !check_substs_compatible(self.tcx, assoc, impl_substs) {
+                                self.tcx.sess.delay_span_bug(
+                                    self.tcx.def_span(assoc.def_id),
+                                    "item had incorrect substs",
                                 );
-                                trace!(?substs);
-                                let substs: Vec<_> =
-                                    substs.iter().chain(own_substs.iter().copied()).collect();
-                                trace!(?substs);
-                                // Find opaque types in this associated type.
-                                return self
-                                    .tcx
-                                    .type_of(assoc.def_id)
-                                    .subst(self.tcx, &substs)
-                                    .visit_with(self);
+                                return ControlFlow::Continue(());
                             }
+
+                            return self
+                                .tcx
+                                .type_of(assoc.def_id)
+                                .subst(self.tcx, impl_substs)
+                                .visit_with(self);
                         }
                     }
                 }
diff --git a/tests/ui/type-alias-impl-trait/impl-trait-in-type-alias-with-bad-substs.rs b/tests/ui/type-alias-impl-trait/impl-trait-in-type-alias-with-bad-substs.rs
@@ -0,0 +1,28 @@
+#![feature(impl_trait_in_assoc_type)]
+
+// We weren't checking that the trait and impl generics line up in the
+// normalization-shortcut code in `OpaqueTypeCollector`.
+
+use std::ops::Deref;
+
+trait Foo {
+    type Bar<'a>;
+
+    type Baz<'a>;
+
+    fn test<'a>() -> Self::Bar<'a>;
+}
+
+impl Foo for () {
+    type Bar<'a> = impl Deref<Target = Self::Baz<'a>>;
+
+    type Baz<T> = impl Sized;
+    //~^ ERROR type `Baz` has 1 type parameter but its trait declaration has 0 type parameters
+    //~| ERROR unconstrained opaque type
+
+    fn test<'a>() -> Self::Bar<'a> {
+        &()
+    }
+}
+
+fn main() {}
diff --git a/tests/ui/type-alias-impl-trait/impl-trait-in-type-alias-with-bad-substs.stderr b/tests/ui/type-alias-impl-trait/impl-trait-in-type-alias-with-bad-substs.stderr
@@ -0,0 +1,20 @@
+error[E0049]: type `Baz` has 1 type parameter but its trait declaration has 0 type parameters
+  --> $DIR/impl-trait-in-type-alias-with-bad-substs.rs:19:14
+   |
+LL |     type Baz<'a>;
+   |              -- expected 0 type parameters
+...
+LL |     type Baz<T> = impl Sized;
+   |              ^ found 1 type parameter
+
+error: unconstrained opaque type
+  --> $DIR/impl-trait-in-type-alias-with-bad-substs.rs:19:19
+   |
+LL |     type Baz<T> = impl Sized;
+   |                   ^^^^^^^^^^
+   |
+   = note: `Baz` must be used in combination with a concrete type within the same impl
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0049`.
diff --git a/tests/ui/type-alias-impl-trait/not-matching-trait-refs-isnt-defining.rs b/tests/ui/type-alias-impl-trait/not-matching-trait-refs-isnt-defining.rs
@@ -0,0 +1,33 @@
+#![feature(impl_trait_in_assoc_type)]
+
+trait Foo<T> {
+    type Assoc;
+
+    fn test() -> u32;
+}
+
+struct DefinesOpaque;
+impl Foo<DefinesOpaque> for () {
+    type Assoc = impl Sized;
+
+    // This test's return type is `u32`, *not* the opaque that is defined above.
+    // Previously we were only checking that the self type of the assoc matched,
+    // but this doesn't account for other impls with different trait substs.
+    fn test() -> <() as Foo<NoOpaques>>::Assoc {
+        let _: <Self as Foo<DefinesOpaque>>::Assoc = "";
+        //~^ ERROR mismatched types
+
+        1
+    }
+}
+
+struct NoOpaques;
+impl Foo<NoOpaques> for () {
+    type Assoc = u32;
+
+    fn test() -> u32 {
+        1
+    }
+}
+
+fn main() {}
diff --git a/tests/ui/type-alias-impl-trait/not-matching-trait-refs-isnt-defining.stderr b/tests/ui/type-alias-impl-trait/not-matching-trait-refs-isnt-defining.stderr
@@ -0,0 +1,22 @@
+error[E0308]: mismatched types
+  --> $DIR/not-matching-trait-refs-isnt-defining.rs:17:54
+   |
+LL |     type Assoc = impl Sized;
+   |                  ---------- the expected opaque type
+...
+LL |         let _: <Self as Foo<DefinesOpaque>>::Assoc = "";
+   |                -----------------------------------   ^^ expected opaque type, found `&str`
+   |                |
+   |                expected due to this
+   |
+   = note: expected opaque type `<() as Foo<DefinesOpaque>>::Assoc`
+                found reference `&'static str`
+note: this item must have the opaque type in its signature in order to be able to register hidden types
+  --> $DIR/not-matching-trait-refs-isnt-defining.rs:16:5
+   |
+LL |     fn test() -> <() as Foo<NoOpaques>>::Assoc {
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0308`.
