Normalize trait ref before orphan check & consider ty params in alias types to be uncovered

Author: fmease

compiler/rustc_hir_analysis/messages.ftl

@@ -429,13 +429,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

@@ -2,11 +2,15 @@
 //! crate or pertains to a type defined in this crate.
 
 use crate::errors;
+
+use std::ops::ControlFlow;
+
+use rustc_data_structures::fx::FxIndexMap;
 use rustc_errors::ErrorGuaranteed;
-use rustc_hir as hir;
-use rustc_middle::ty::{self, AliasKind, Ty, TyCtxt, TypeVisitableExt};
+use rustc_middle::ty::{self, AliasKind, Ty, TyCtxt};
+use rustc_middle::ty::{TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor};
 use rustc_span::def_id::LocalDefId;
-use rustc_span::Span;
+use rustc_span::{Span, Symbol};
 use rustc_trait_selection::traits::{self, IsFirstInputType};
 
 #[instrument(skip(tcx), level = "debug")]
@@ -23,20 +27,17 @@ pub(crate) fn orphan_check_impl(
         Ok(()) => {}
         Err(err) => {
             let item = tcx.hir().expect_item(impl_def_id);
-            let hir::ItemKind::Impl(impl_) = item.kind else {
-                bug!("{:?} is not an impl: {:?}", impl_def_id, item);
-            };
-            let tr = impl_.of_trait.as_ref().unwrap();
-            let sp = tcx.def_span(impl_def_id);
+            let impl_ = item.expect_impl();
+            let hir_trait_ref = impl_.of_trait.as_ref().unwrap();
 
             emit_orphan_check_error(
                 tcx,
-                sp,
+                tcx.def_span(impl_def_id),
                 item.span,
-                tr.path.span,
+                hir_trait_ref.path.span,
                 trait_ref,
                 impl_.self_ty.span,
-                impl_.generics,
+                tcx.generics_of(impl_def_id),
                 err,
             )?
         }
@@ -277,7 +278,7 @@ fn emit_orphan_check_error<'tcx>(
     trait_span: Span,
     trait_ref: ty::TraitRef<'tcx>,
     self_ty_span: Span,
-    generics: &hir::Generics<'tcx>,
+    generics: &'tcx ty::Generics,
     err: traits::OrphanCheckErr<'tcx>,
 ) -> Result<!, ErrorGuaranteed> {
     let self_ty = trait_ref.self_ty();
@@ -404,23 +405,48 @@ 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;
-                }
+        // FIXME(fmease): The uncovered ty may contain infer vars.
+        // Somwhere we need to remap them to ty params.
+        traits::OrphanCheckErr::UncoveredTy(uncovered_ty, local_type) => {
+            let mut collector =
+                UncoveredTyParamCollector { tcx, generics, 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<'tcx> {
+                tcx: TyCtxt<'tcx>,
+                generics: &'tcx ty::Generics,
+                uncovered_params: FxIndexMap<Symbol, Span>,
             }
+
+            impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for UncoveredTyParamCollector<'tcx> {
+                type BreakTy = !;
+
+                fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
+                    if let ty::Param(param_ty) = *ty.kind() {
+                        let param_def = self.generics.param_at(param_ty.index as _, self.tcx);
+                        let span = self.tcx.def_ident_span(param_def.def_id).unwrap();
+                        self.uncovered_params.insert(param_def.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

@@ -1328,20 +1328,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/lib.rs

@@ -20,6 +20,7 @@
 #![feature(box_patterns)]
 #![feature(control_flow_enum)]
 #![feature(extract_if)]
+#![feature(if_let_guard)]
 #![feature(let_chains)]
 #![feature(option_take_if)]
 #![feature(never_type)]

compiler/rustc_trait_selection/src/traits/coherence.rs

@@ -12,15 +12,15 @@ use crate::solve::{deeply_normalize_for_diagnostics, inspect, FulfillmentCtxt};
 use crate::traits::engine::TraitEngineExt as _;
 use crate::traits::select::IntercrateAmbiguityCause;
 use crate::traits::structural_normalize::StructurallyNormalizeExt;
-use crate::traits::NormalizeExt;
 use crate::traits::SkipLeakCheck;
+use crate::traits::{NormalizeExt, ObligationCtxt};
 use crate::traits::{
     Obligation, ObligationCause, PredicateObligation, PredicateObligations, SelectionContext,
 };
 use rustc_data_structures::fx::FxIndexSet;
 use rustc_errors::{DiagnosticBuilder, EmissionGuarantee};
 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, TraitEngineExt};
 use rustc_middle::traits::query::NoSolution;
@@ -598,7 +598,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, Copy, Clone)]
@@ -630,7 +630,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);
@@ -641,7 +641,35 @@ 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, |raw_ty| {
+        let ty::Alias(..) = raw_ty.kind() else { return Ok(raw_ty) };
+
+        let infcx = tcx.infer_ctxt().intercrate(true).build();
+        let cause = ObligationCause::dummy();
+
+        let args = infcx.fresh_args_for_item(cause.span, impl_def_id);
+        let raw_ty = ty::EarlyBinder::bind(raw_ty).instantiate(tcx, args);
+
+        let ocx = ObligationCtxt::new(&infcx);
+        let ty = ocx.normalize(&cause, ty::ParamEnv::empty(), raw_ty);
+        let ty = infcx.resolve_vars_if_possible(ty);
+        let errors = ocx.select_where_possible();
+        if !errors.is_empty() {
+            return Ok(raw_ty);
+        }
+
+        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(Ok(ty))
+        } else {
+            Ok(ty)
+        }
+    })
+    .unwrap()
 }
 
 /// Checks whether a trait-ref is potentially implementable by a crate.
@@ -689,6 +717,9 @@ pub fn orphan_check(tcx: TyCtxt<'_>, impl_def_id: DefId) -> Result<(), OrphanChe
 ///    To check that a local impl follows the orphan rules, we check it in
 ///    InCrate::Local mode, using type parameters for the "generic" types.
 ///
+///    In InCrate::Local mode the orphan check succeeds if the current crate
+///    is definitely allowed to implement the given trait (no false positives).
+///
 /// 2. They ground negative reasoning for coherence. If a user wants to
 ///    write both a conditional blanket impl and a specific impl, we need to
 ///    make sure they do not overlap. For example, if we write
@@ -707,6 +738,9 @@ pub fn orphan_check(tcx: TyCtxt<'_>, impl_def_id: DefId) -> Result<(), OrphanChe
 ///    try to implement this trait-ref. To check for this, we use InCrate::Remote
 ///    mode. That is sound because we already know all the impls from known crates.
 ///
+///    In InCrate::Remote mode the orphan check succeeds if a foreign crate
+///    *could* implement the given trait (no false negatives).
+///
 /// 3. For non-`#[fundamental]` traits, they guarantee that parent crates can
 ///    add "non-blanket" impls without breaking negative reasoning in dependent
 ///    crates. This is the "rebalancing coherence" (RFC 1023) restriction.
@@ -747,16 +781,14 @@ 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::UncoveredTyParam(ty)) => {
             // Does there exist some local type after the `ParamTy`.
             checker.search_first_local_ty = true;
-            if let Some(OrphanCheckEarlyExit::LocalTy(local_ty)) =
-                trait_ref.visit_with(&mut checker).break_value()
-            {
-                Err(OrphanCheckErr::UncoveredTy(ty, Some(local_ty)))
-            } else {
-                Err(OrphanCheckErr::UncoveredTy(ty, None))
-            }
+            let local_ty = match trait_ref.visit_with(&mut checker).break_value() {
+                Some(OrphanCheckEarlyExit::LocalTy(local_ty)) => Some(local_ty),
+                _ => None,
+            };
+            Err(OrphanCheckErr::UncoveredTy(ty, local_ty))
         }
         ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(_)) => Ok(()),
     })
@@ -791,11 +823,14 @@ where
         ControlFlow::Continue(())
     }
 
-    fn found_param_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<OrphanCheckEarlyExit<'tcx, E>> {
+    fn found_uncovered_ty_param(
+        &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::UncoveredTyParam(t))
         }
     }
 
@@ -809,7 +844,7 @@ where
 
 enum OrphanCheckEarlyExit<'tcx, E> {
     NormalizationFailure(E),
-    ParamTy(Ty<'tcx>),
+    UncoveredTyParam(Ty<'tcx>),
     LocalTy(Ty<'tcx>),
 }
 
@@ -823,9 +858,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)),
         };
 
@@ -842,19 +877,44 @@ where
             | ty::Slice(..)
             | ty::RawPtr(..)
             | ty::Never
-            | ty::Tuple(..)
-            | ty::Alias(ty::Projection | ty::Inherent | ty::Weak, ..) => {
-                self.found_non_local_ty(ty)
-            }
-
-            ty::Param(..) => self.found_param_ty(ty),
+            | ty::Tuple(..) => self.found_non_local_ty(ty),
 
-            ty::Placeholder(..) | ty::Bound(..) | ty::Infer(..) => match self.in_crate {
-                InCrate::Local => self.found_non_local_ty(ty),
-                // The inference variable might be unified with a local
-                // type in that remote crate.
-                InCrate::Remote => ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty)),
+            ty::Param(..) => match self.in_crate {
+                InCrate::Local => self.found_uncovered_ty_param(ty),
+                InCrate::Remote => bug!("unexpected ty param"),
             },
+            ty::Placeholder(..) | ty::Bound(..) | ty::Infer(..) => {
+                match self.in_crate {
+                    InCrate::Local => self.found_uncovered_ty_param(ty),
+                    // The inference variable might be unified with a local
+                    // type in that remote crate.
+                    InCrate::Remote => ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty)),
+                }
+            }
+
+            ty::Alias(ty::Projection | ty::Inherent | ty::Weak, ..) => {
+                if ty.has_type_flags(ty::TypeFlags::HAS_TY_PARAM) {
+                    match self.in_crate {
+                        InCrate::Local => self.found_uncovered_ty_param(ty),
+                        InCrate::Remote => bug!("unexpected ty param"),
+                    }
+                } else if ty.has_type_flags(
+                    ty::TypeFlags::HAS_TY_PLACEHOLDER
+                        | ty::TypeFlags::HAS_TY_BOUND
+                        | ty::TypeFlags::HAS_TY_INFER,
+                ) {
+                    match self.in_crate {
+                        InCrate::Local => self.found_uncovered_ty_param(ty),
+                        InCrate::Remote => {
+                            // The inference variable might be unified with a local
+                            // type in that remote crate.
+                            ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty))
+                        }
+                    }
+                } else {
+                    ControlFlow::Continue(())
+                }
+            }
 
             // For fundamental types, we just look inside of them.
             ty::Ref(_, ty, _) => ty.visit_with(self),

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;
+}