Normalize trait ref before orphan check

Author: fmease

compiler/rustc_hir_analysis/messages.ftl

@@ -401,13 +401,13 @@ hir_analysis_transparent_non_zero_sized_enum = the variant of a transparent {$de
     .label = needs at most one field with non-trivial size or alignment, but has {$field_count}
     .labels = this field has non-zero size or requires alignment
 
-hir_analysis_ty_param_first_local = type parameter `{$param_ty}` must be covered by another type when it appears before the first local type (`{$local_type}`)
-    .label = type parameter `{$param_ty}` must be covered by another type when it appears before the first local type (`{$local_type}`)
+hir_analysis_ty_param_first_local = type parameter `{$param}` must be covered by another type when it appears before the first local type (`{$local_type}`)
+    .label = type parameter `{$param}` must be covered by another type when it appears before the first local type (`{$local_type}`)
     .note = implementing a foreign trait is only possible if at least one of the types for which it is implemented is local, and no uncovered type parameters appear before that first local type
     .case_note = in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last
 
-hir_analysis_ty_param_some = type parameter `{$param_ty}` must be used as the type parameter for some local type (e.g., `MyStruct<{$param_ty}>`)
-    .label = type parameter `{$param_ty}` must be used as the type parameter for some local type
+hir_analysis_ty_param_some = type parameter `{$param}` must be used as the type parameter for some local type (e.g., `MyStruct<{$param}>`)
+    .label = type parameter `{$param}` must be used as the type parameter for some local type
     .note = implementing a foreign trait is only possible if at least one of the types for which it is implemented is local
     .only_note = only traits defined in the current crate can be implemented for a type parameter
 

compiler/rustc_hir_analysis/src/coherence/orphan.rs

@@ -1,7 +1,7 @@
 //! Orphan checker: every impl either implements a trait defined in this
 //! crate or pertains to a type defined in this crate.
 
-use rustc_data_structures::fx::FxHashSet;
+use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
 use rustc_errors::{DelayDm, ErrorGuaranteed};
 use rustc_hir as hir;
 use rustc_middle::ty::util::CheckRegions;
@@ -12,7 +12,7 @@ use rustc_middle::ty::{
 };
 use rustc_session::lint;
 use rustc_span::def_id::{DefId, LocalDefId};
-use rustc_span::Span;
+use rustc_span::{Span, Symbol};
 use rustc_trait_selection::traits;
 use std::ops::ControlFlow;
 
@@ -424,23 +424,55 @@ fn emit_orphan_check_error<'tcx>(
             };
             tcx.dcx().emit_err(err_struct)
         }
-        traits::OrphanCheckErr::UncoveredTy(param_ty, local_type) => {
-            let mut sp = sp;
-            for param in generics.params {
-                if param.name.ident().to_string() == param_ty.to_string() {
-                    sp = param.span;
-                }
+        traits::OrphanCheckErr::UncoveredTy(uncovered_ty, local_type) => {
+            let mut collector = UncoveredTyParamCollector {
+                available_params: generics
+                    .params
+                    .iter()
+                    .filter(|param| matches!(param.kind, hir::GenericParamKind::Type { .. }))
+                    .map(|param| (param.name.ident().name, param.span))
+                    .collect(),
+                uncovered_params: Default::default(),
+            };
+            uncovered_ty.visit_with(&mut collector);
+
+            let mut reported = None;
+            for (param, span) in collector.uncovered_params {
+                reported.get_or_insert(match local_type {
+                    Some(local_type) => tcx.dcx().emit_err(errors::TyParamFirstLocal {
+                        span,
+                        note: (),
+                        param,
+                        local_type,
+                    }),
+                    None => tcx.dcx().emit_err(errors::TyParamSome { span, note: (), param }),
+                });
             }
 
-            match local_type {
-                Some(local_type) => tcx.dcx().emit_err(errors::TyParamFirstLocal {
-                    span: sp,
-                    note: (),
-                    param_ty,
-                    local_type,
-                }),
-                None => tcx.dcx().emit_err(errors::TyParamSome { span: sp, note: (), param_ty }),
+            struct UncoveredTyParamCollector {
+                available_params: FxHashMap<Symbol, Span>,
+                uncovered_params: FxIndexSet<(Symbol, Span)>,
+            }
+
+            impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for UncoveredTyParamCollector {
+                type BreakTy = !;
+
+                fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
+                    // FIXME: This doesn't respect macro hygiene I think. We can't look up the param
+                    // in the `hir::Generics.params` by its index since the index is relative to the
+                    // `ty::Generics.params`.
+                    if let ty::Param(param) = *ty.kind()
+                        && let Some(&span) = self.available_params.get(&param.name)
+                    {
+                        self.uncovered_params.insert((param.name, span));
+                        ControlFlow::Continue(())
+                    } else {
+                        ty.super_visit_with(self)
+                    }
+                }
             }
+
+            reported.unwrap_or_else(|| bug!("failed to find ty param in `{uncovered_ty}`"))
         }
     })
 }

compiler/rustc_hir_analysis/src/errors.rs

@@ -1279,20 +1279,20 @@ pub struct TyParamFirstLocal<'a> {
     pub span: Span,
     #[note(hir_analysis_case_note)]
     pub note: (),
-    pub param_ty: Ty<'a>,
+    pub param: Symbol,
     pub local_type: Ty<'a>,
 }
 
 #[derive(Diagnostic)]
 #[diag(hir_analysis_ty_param_some, code = "E0210")]
 #[note]
-pub struct TyParamSome<'a> {
+pub struct TyParamSome {
     #[primary_span]
     #[label]
     pub span: Span,
     #[note(hir_analysis_only_note)]
     pub note: (),
-    pub param_ty: Ty<'a>,
+    pub param: Symbol,
 }
 
 #[derive(Diagnostic)]

compiler/rustc_trait_selection/src/traits/coherence.rs

@@ -21,7 +21,7 @@ use crate::traits::{
 use rustc_data_structures::fx::FxIndexSet;
 use rustc_errors::Diagnostic;
 use rustc_hir::def::DefKind;
-use rustc_hir::def_id::{DefId, LOCAL_CRATE};
+use rustc_hir::def_id::DefId;
 use rustc_infer::infer::{DefineOpaqueTypes, InferCtxt, TyCtxtInferExt};
 use rustc_infer::traits::{util, TraitEngine};
 use rustc_middle::traits::query::NoSolution;
@@ -592,7 +592,7 @@ pub fn trait_ref_is_local_or_fundamental<'tcx>(
     tcx: TyCtxt<'tcx>,
     trait_ref: ty::TraitRef<'tcx>,
 ) -> bool {
-    trait_ref.def_id.krate == LOCAL_CRATE || tcx.has_attr(trait_ref.def_id, sym::fundamental)
+    trait_ref.def_id.is_local() || tcx.has_attr(trait_ref.def_id, sym::fundamental)
 }
 
 #[derive(Debug)]
@@ -609,7 +609,7 @@ pub enum OrphanCheckErr<'tcx> {
 /// 2. Some local type must appear in `Self`.
 #[instrument(level = "debug", skip(tcx), ret)]
 pub fn orphan_check(tcx: TyCtxt<'_>, impl_def_id: DefId) -> Result<(), OrphanCheckErr<'_>> {
-    // We only except this routine to be invoked on implementations
+    // We only accept this routine to be invoked on implementations
     // of a trait, not inherent implementations.
     let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap().instantiate_identity();
     debug!(?trait_ref);
@@ -620,7 +620,42 @@ pub fn orphan_check(tcx: TyCtxt<'_>, impl_def_id: DefId) -> Result<(), OrphanChe
         return Ok(());
     }
 
-    orphan_check_trait_ref::<!>(trait_ref, InCrate::Local, |ty| Ok(ty)).unwrap()
+    orphan_check_trait_ref::<!>(trait_ref, InCrate::Local, |ty| {
+        let ty::Alias(..) = ty.kind() else { return Ok(ty) };
+
+        let delay_bug = || {
+            tcx.dcx().span_delayed_bug(
+                tcx.def_span(impl_def_id),
+                format!(
+                    "orphan check: failed to normalize `{trait_ref}` while checking {impl_def_id:?}"
+                ),
+            )
+        };
+
+        let infcx = tcx.infer_ctxt().intercrate(true).build();
+        let cause = ObligationCause::dummy();
+
+        let ocx = ObligationCtxt::new(&infcx);
+        let args = infcx.fresh_args_for_item(cause.span, impl_def_id);
+        let ty = ty::EarlyBinder::bind(ty).instantiate(tcx, args);
+        let ty = ocx.normalize(&cause, ty::ParamEnv::empty(), ty);
+        let ty = infcx.resolve_vars_if_possible(ty);
+        if !ocx.select_where_possible().is_empty() {
+            delay_bug();
+        }
+
+        if infcx.next_trait_solver() {
+            let mut fulfill_cx = <dyn TraitEngine<'_>>::new(&infcx);
+            infcx
+                .at(&cause, ty::ParamEnv::empty())
+                .structurally_normalize(ty, &mut *fulfill_cx)
+                .map(|ty| infcx.resolve_vars_if_possible(ty))
+                .or_else(|_| Ok(Ty::new_error(tcx, delay_bug())))
+        } else {
+            Ok(ty)
+        }
+    })
+    .unwrap()
 }
 
 /// Checks whether a trait-ref is potentially implementable by a crate.
@@ -726,7 +761,7 @@ fn orphan_check_trait_ref<'tcx, E: Debug>(
     Ok(match trait_ref.visit_with(&mut checker) {
         ControlFlow::Continue(()) => Err(OrphanCheckErr::NonLocalInputType(checker.non_local_tys)),
         ControlFlow::Break(OrphanCheckEarlyExit::NormalizationFailure(err)) => return Err(err),
-        ControlFlow::Break(OrphanCheckEarlyExit::ParamTy(ty)) => {
+        ControlFlow::Break(OrphanCheckEarlyExit::UncoveredTy(ty)) => {
             // Does there exist some local type after the `ParamTy`.
             checker.search_first_local_ty = true;
             if let Some(OrphanCheckEarlyExit::LocalTy(local_ty)) =
@@ -770,11 +805,11 @@ where
         ControlFlow::Continue(())
     }
 
-    fn found_param_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<OrphanCheckEarlyExit<'tcx, E>> {
+    fn found_uncovered_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<OrphanCheckEarlyExit<'tcx, E>> {
         if self.search_first_local_ty {
             ControlFlow::Continue(())
         } else {
-            ControlFlow::Break(OrphanCheckEarlyExit::ParamTy(t))
+            ControlFlow::Break(OrphanCheckEarlyExit::UncoveredTy(t))
         }
     }
 
@@ -788,7 +823,7 @@ where
 
 enum OrphanCheckEarlyExit<'tcx, E> {
     NormalizationFailure(E),
-    ParamTy(Ty<'tcx>),
+    UncoveredTy(Ty<'tcx>),
     LocalTy(Ty<'tcx>),
 }
 
@@ -802,9 +837,9 @@ where
     }
 
     fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
-        // Need to lazily normalize here in with `-Znext-solver=coherence`.
         let ty = match (self.lazily_normalize_ty)(ty) {
-            Ok(ty) => ty,
+            Ok(norm_ty) if norm_ty.is_ty_or_numeric_infer() => ty,
+            Ok(norm_ty) => norm_ty,
             Err(err) => return ControlFlow::Break(OrphanCheckEarlyExit::NormalizationFailure(err)),
         };
 
@@ -821,12 +856,17 @@ where
             | ty::Slice(..)
             | ty::RawPtr(..)
             | ty::Never
-            | ty::Tuple(..)
-            | ty::Alias(ty::Projection | ty::Inherent | ty::Weak, ..) => {
-                self.found_non_local_ty(ty)
+            | ty::Tuple(..) => self.found_non_local_ty(ty),
+
+            ty::Alias(ty::Projection | ty::Inherent | ty::Weak, ..) => {
+                if ty.has_type_flags(ty::TypeFlags::HAS_TY_PARAM) {
+                    self.found_uncovered_ty(ty)
+                } else {
+                    ControlFlow::Continue(())
+                }
             }
 
-            ty::Param(..) => self.found_param_ty(ty),
+            ty::Param(..) => self.found_uncovered_ty(ty),
 
             ty::Placeholder(..) | ty::Bound(..) | ty::Infer(..) => match self.in_crate {
                 InCrate::Local => self.found_non_local_ty(ty),

tests/ui/coherence/auxiliary/parametrized-trait.rs

@@ -0,0 +1,2 @@
+pub trait Trait0<T, U, V> {}
+pub trait Trait1<T, U> {}

tests/ui/coherence/auxiliary/trait-with-assoc-ty.rs

@@ -0,0 +1,3 @@
+pub trait Trait {
+    type Assoc;
+}

tests/ui/coherence/orphan-check-nested-projections.rs

@@ -0,0 +1,23 @@
+// This used to ICE in an earlier iteration of #117164. Minimized from crate `proqnt`.
+
+// check-pass
+// revisions: classic next
+//[next] compile-flags: -Znext-solver
+// aux-crate:dep=trait-with-assoc-ty.rs
+// edition: 2021
+
+pub(crate) trait Trait<T> {
+    type Assoc;
+}
+
+pub(crate) struct Type<T, U, V>(T, U, V);
+
+impl<T, U> dep::Trait for Type<T, <<T as dep::Trait>::Assoc as Trait<U>>::Assoc, U>
+where
+    T: dep::Trait,
+    <T as dep::Trait>::Assoc: Trait<U>,
+{
+    type Assoc = U;
+}
+
+fn main() {}

tests/ui/coherence/orphan-check-projection-does-cover.classic.stderr

@@ -0,0 +1,12 @@
+error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
+  --> $DIR/orphan-check-projection-does-cover.rs:23:6
+   |
+LL | impl<T> foreign::Trait1<Local, T> for <T as Project>::Output {}
+   |      ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
+   |
+   = note: implementing a foreign trait is only possible if at least one of the types for which it is implemented is local, and no uncovered type parameters appear before that first local type
+   = note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0210`.

tests/ui/coherence/orphan-check-projection-does-cover.next.stderr

@@ -0,0 +1,12 @@
+error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
+  --> $DIR/orphan-check-projection-does-cover.rs:23:6
+   |
+LL | impl<T> foreign::Trait1<Local, T> for <T as Project>::Output {}
+   |      ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
+   |
+   = note: implementing a foreign trait is only possible if at least one of the types for which it is implemented is local, and no uncovered type parameters appear before that first local type
+   = note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0210`.

tests/ui/coherence/orphan-check-projection-does-cover.rs

@@ -0,0 +1,25 @@
+// Projections can cover type parameters if they normalize to a (local) type that covers them.
+// This ensures that we don't perform an overly strict check on
+// projections like in closed PR #100555 which did a syntactic
+// check for type parameters in projections without normalizing
+// first which would've lead to real-word regressions.
+
+// FIXME: check-pass
+// known-bug: unknown
+// revisions: classic next
+//[next] compile-flags: -Znext-solver
+
+// aux-crate:foreign=parametrized-trait.rs
+// edition:2021
+
+trait Project { type Output; }
+
+impl<T> Project for T {
+    type Output = Local;
+}
+
+struct Local;
+
+impl<T> foreign::Trait1<Local, T> for <T as Project>::Output {}
+
+fn main() {}

tests/ui/coherence/orphan-check-projection-doesnt-cover.classic.stderr

@@ -0,0 +1,30 @@
+error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
+  --> $DIR/orphan-check-projection-doesnt-cover.rs:21:6
+   |
+LL | impl<T> foreign::Trait0<Local, T, ()> for <T as Identity>::Output {}
+   |      ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
+   |
+   = note: implementing a foreign trait is only possible if at least one of the types for which it is implemented is local, and no uncovered type parameters appear before that first local type
+   = note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last
+
+error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
+  --> $DIR/orphan-check-projection-doesnt-cover.rs:24:6
+   |
+LL | impl<T> foreign::Trait0<<T as Identity>::Output, Local, T> for Option<T> {}
+   |      ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
+   |
+   = note: implementing a foreign trait is only possible if at least one of the types for which it is implemented is local, and no uncovered type parameters appear before that first local type
+   = note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last
+
+error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
+  --> $DIR/orphan-check-projection-doesnt-cover.rs:36:6
+   |
+LL | impl<T: Deferred> foreign::Trait1<Local, T> for <T as Deferred>::Output {}
+   |      ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
+   |
+   = note: implementing a foreign trait is only possible if at least one of the types for which it is implemented is local, and no uncovered type parameters appear before that first local type
+   = note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last
+
+error: aborting due to 3 previous errors
+
+For more information about this error, try `rustc --explain E0210`.