Skip to content

Commit ec56c44

Browse files
fmeasefmease
committed
Normalize trait ref before orphan check
1 parent 525c91d commit ec56c44

15 files changed

+340
-54
lines changed

‎compiler/rustc_hir_analysis/messages.ftl

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -370,13 +370,13 @@ hir_analysis_transparent_non_zero_sized_enum = the variant of a transparent {$de
370370
.label = needs at most one field with non-trivial size or alignment, but has {$field_count}
371371
.labels = this field has non-zero size or requires alignment
372372
373-
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}`)
374-
.label = type parameter `{$param_ty}` must be covered by another type when it appears before the first local type (`{$local_type}`)
373+
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}`)
374+
.label = type parameter `{$param}` must be covered by another type when it appears before the first local type (`{$local_type}`)
375375
.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
376376
.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
377377
378-
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}>`)
379-
.label = type parameter `{$param_ty}` must be used as the type parameter for some local type
378+
hir_analysis_ty_param_some = type parameter `{$param}` must be used as the type parameter for some local type (e.g., `MyStruct<{$param}>`)
379+
.label = type parameter `{$param}` must be used as the type parameter for some local type
380380
.note = implementing a foreign trait is only possible if at least one of the types for which it is implemented is local
381381
.only_note = only traits defined in the current crate can be implemented for a type parameter
382382

‎compiler/rustc_hir_analysis/src/coherence/orphan.rs

Lines changed: 47 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Orphan checker: every impl either implements a trait defined in this
22
//! crate or pertains to a type defined in this crate.
33
4-
use rustc_data_structures::fx::FxHashSet;
4+
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
55
use rustc_errors::{DelayDm, ErrorGuaranteed};
66
use rustc_hir as hir;
77
use rustc_middle::ty::util::CheckRegions;
@@ -12,7 +12,7 @@ use rustc_middle::ty::{
1212
};
1313
use rustc_session::lint;
1414
use rustc_span::def_id::{DefId, LocalDefId};
15-
use rustc_span::Span;
15+
use rustc_span::{Span, Symbol};
1616
use rustc_trait_selection::traits;
1717
use std::ops::ControlFlow;
1818

@@ -424,23 +424,55 @@ fn emit_orphan_check_error<'tcx>(
424424
};
425425
tcx.sess.emit_err(err_struct)
426426
}
427-
traits::OrphanCheckErr::UncoveredTy(param_ty, local_type) => {
428-
let mut sp = sp;
429-
for param in generics.params {
430-
if param.name.ident().to_string() == param_ty.to_string() {
431-
sp = param.span;
427+
traits::OrphanCheckErr::UncoveredTy(uncovered_ty, local_type) => {
428+
struct TyParamFinder {
429+
available_ty_params: FxHashMap<Symbol, Span>,
430+
ty_params: FxHashSet<(Symbol, Span)>,
431+
}
432+
433+
// FIXME(fmease): Add test for `<Type<T, U> as Trait>::Assoc` for `<T, U>`.
434+
impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for TyParamFinder {
435+
type BreakTy = ();
436+
437+
fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
438+
// FIXME: This doesn't respect macro hygiene.
439+
if let ty::Param(param_ty) = ty.kind()
440+
&& let Some(&span) = self.available_ty_params.get(&param_ty.name)
441+
{
442+
self.ty_params.insert((param_ty.name, span));
443+
return ControlFlow::Break(());
444+
}
445+
446+
ty.super_visit_with(self)
432447
}
433448
}
434449

435-
match local_type {
436-
Some(local_type) => tcx.sess.emit_err(errors::TyParamFirstLocal {
437-
span: sp,
438-
note: (),
439-
param_ty,
440-
local_type,
441-
}),
442-
None => tcx.sess.emit_err(errors::TyParamSome { span: sp, note: (), param_ty }),
450+
let mut visitor = TyParamFinder {
451+
available_ty_params: generics
452+
.params
453+
.iter()
454+
.filter(|param| matches!(param.kind, hir::GenericParamKind::Type { .. }))
455+
.map(|param| (param.name.ident().name, param.span))
456+
.collect(),
457+
ty_params: Default::default(),
458+
};
459+
460+
uncovered_ty.visit_with(&mut visitor);
461+
462+
let mut guar = None;
463+
for (param, span) in visitor.ty_params {
464+
guar = Some(match local_type {
465+
Some(local_type) => tcx.sess.emit_err(errors::TyParamFirstLocal {
466+
span,
467+
note: (),
468+
param,
469+
local_type,
470+
}),
471+
None => tcx.sess.emit_err(errors::TyParamSome { span, note: (), param }),
472+
});
443473
}
474+
475+
guar.unwrap_or_else(|| bug!("failed to find ty param in {uncovered_ty}"))
444476
}
445477
})
446478
}

‎compiler/rustc_hir_analysis/src/errors.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,20 +1198,20 @@ pub struct TyParamFirstLocal<'a> {
11981198
pub span: Span,
11991199
#[note(hir_analysis_case_note)]
12001200
pub note: (),
1201-
pub param_ty: Ty<'a>,
1201+
pub param: Symbol,
12021202
pub local_type: Ty<'a>,
12031203
}
12041204

12051205
#[derive(Diagnostic)]
12061206
#[diag(hir_analysis_ty_param_some, code = "E0210")]
12071207
#[note]
1208-
pub struct TyParamSome<'a> {
1208+
pub struct TyParamSome {
12091209
#[primary_span]
12101210
#[label]
12111211
pub span: Span,
12121212
#[note(hir_analysis_only_note)]
12131213
pub note: (),
1214-
pub param_ty: Ty<'a>,
1214+
pub param: Symbol,
12151215
}
12161216

12171217
#[derive(Diagnostic)]

‎compiler/rustc_trait_selection/src/traits/coherence.rs

Lines changed: 78 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use crate::traits::{
2121
use rustc_data_structures::fx::FxIndexSet;
2222
use rustc_errors::Diagnostic;
2323
use rustc_hir::def::DefKind;
24-
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
24+
use rustc_hir::def_id::DefId;
2525
use rustc_infer::infer::{DefineOpaqueTypes, InferCtxt, TyCtxtInferExt};
2626
use rustc_infer::traits::{util, TraitEngine};
2727
use rustc_middle::traits::query::NoSolution;
@@ -621,7 +621,7 @@ pub fn trait_ref_is_local_or_fundamental<'tcx>(
621621
tcx: TyCtxt<'tcx>,
622622
trait_ref: ty::TraitRef<'tcx>,
623623
) -> bool {
624-
trait_ref.def_id.krate == LOCAL_CRATE || tcx.has_attr(trait_ref.def_id, sym::fundamental)
624+
trait_ref.def_id.is_local() || tcx.has_attr(trait_ref.def_id, sym::fundamental)
625625
}
626626

627627
#[derive(Debug)]
@@ -638,7 +638,7 @@ pub enum OrphanCheckErr<'tcx> {
638638
/// 2. Some local type must appear in `Self`.
639639
#[instrument(level = "debug", skip(tcx), ret)]
640640
pub fn orphan_check(tcx: TyCtxt<'_>, impl_def_id: DefId) -> Result<(), OrphanCheckErr<'_>> {
641-
// We only except this routine to be invoked on implementations
641+
// We only accept this routine to be invoked on implementations
642642
// of a trait, not inherent implementations.
643643
let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap().instantiate_identity();
644644
debug!(?trait_ref);
@@ -649,7 +649,39 @@ pub fn orphan_check(tcx: TyCtxt<'_>, impl_def_id: DefId) -> Result<(), OrphanChe
649649
return Ok(());
650650
}
651651

652-
orphan_check_trait_ref::<!>(trait_ref, InCrate::Local, |ty| Ok(ty)).unwrap()
652+
orphan_check_trait_ref::<!>(trait_ref, InCrate::Local, |ty| {
653+
let delay_bug = || {
654+
tcx.sess.delay_span_bug(
655+
tcx.def_span(impl_def_id),
656+
format!(
657+
"orphan check: failed to normalize `{trait_ref}` while checking {impl_def_id:?}"
658+
),
659+
)
660+
};
661+
662+
let infcx = tcx.infer_ctxt().intercrate(true).build();
663+
let cause = ObligationCause::dummy();
664+
let param_env = tcx.param_env(impl_def_id);
665+
666+
let ocx = ObligationCtxt::new(&infcx);
667+
let ty = ocx.normalize(&cause, param_env, ty);
668+
let ty = infcx.resolve_vars_if_possible(ty);
669+
if !ocx.select_where_possible().is_empty() {
670+
delay_bug();
671+
}
672+
673+
if infcx.next_trait_solver()
674+
&& let ty::Alias(..) = ty.kind()
675+
{
676+
let mut fulfill_cx = <dyn TraitEngine<'_>>::new(&infcx);
677+
infcx.at(&cause, param_env).structurally_normalize(ty, &mut *fulfill_cx)
678+
.map(|ty| infcx.resolve_vars_if_possible(ty))
679+
.or_else(|_| Ok(Ty::new_error(tcx, delay_bug())))
680+
} else {
681+
Ok(ty)
682+
}
683+
})
684+
.unwrap()
653685
}
654686

655687
/// Checks whether a trait-ref is potentially implementable by a crate.
@@ -751,23 +783,37 @@ fn orphan_check_trait_ref<'tcx, E: Debug>(
751783
);
752784
}
753785

754-
let mut checker = OrphanChecker::new(in_crate, lazily_normalize_ty);
755-
Ok(match trait_ref.visit_with(&mut checker) {
756-
ControlFlow::Continue(()) => Err(OrphanCheckErr::NonLocalInputType(checker.non_local_tys)),
757-
ControlFlow::Break(OrphanCheckEarlyExit::NormalizationFailure(err)) => return Err(err),
758-
ControlFlow::Break(OrphanCheckEarlyExit::ParamTy(ty)) => {
759-
// Does there exist some local type after the `ParamTy`.
760-
checker.search_first_local_ty = true;
761-
if let Some(OrphanCheckEarlyExit::LocalTy(local_ty)) =
762-
trait_ref.visit_with(&mut checker).break_value()
763-
{
764-
Err(OrphanCheckErr::UncoveredTy(ty, Some(local_ty)))
765-
} else {
766-
Err(OrphanCheckErr::UncoveredTy(ty, None))
786+
fn check<'tcx, E: Debug>(
787+
trait_ref: ty::TraitRef<'tcx>,
788+
in_crate: InCrate,
789+
lazily_normalize_ty: impl FnMut(Ty<'tcx>) -> Result<Ty<'tcx>, E>,
790+
) -> Result<Result<(), OrphanCheckErr<'tcx>>, E> {
791+
let mut checker = OrphanChecker::new(in_crate, lazily_normalize_ty);
792+
793+
Ok(match trait_ref.visit_with(&mut checker) {
794+
ControlFlow::Continue(()) => {
795+
Err(OrphanCheckErr::NonLocalInputType(checker.non_local_tys))
767796
}
768-
}
769-
ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(_)) => Ok(()),
770-
})
797+
ControlFlow::Break(OrphanCheckEarlyExit::NormalizationFailure(err)) => return Err(err),
798+
ControlFlow::Break(OrphanCheckEarlyExit::UncoveredTy(ty)) => {
799+
// Does there exist some local type after the `ParamTy`.
800+
checker.search_first_local_ty = true;
801+
if let Some(OrphanCheckEarlyExit::LocalTy(local_ty)) =
802+
trait_ref.visit_with(&mut checker).break_value()
803+
{
804+
Err(OrphanCheckErr::UncoveredTy(ty, Some(local_ty)))
805+
} else {
806+
Err(OrphanCheckErr::UncoveredTy(ty, None))
807+
}
808+
}
809+
ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(_)) => Ok(()),
810+
})
811+
}
812+
813+
if let result @ Err(_) = check(trait_ref, in_crate, lazily_normalize_ty) {
814+
return result;
815+
}
816+
check(trait_ref, in_crate, |ty| Ok(ty))
771817
}
772818

773819
struct OrphanChecker<'tcx, F> {
@@ -799,11 +845,11 @@ where
799845
ControlFlow::Continue(())
800846
}
801847

802-
fn found_param_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<OrphanCheckEarlyExit<'tcx, E>> {
848+
fn found_uncovered_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<OrphanCheckEarlyExit<'tcx, E>> {
803849
if self.search_first_local_ty {
804850
ControlFlow::Continue(())
805851
} else {
806-
ControlFlow::Break(OrphanCheckEarlyExit::ParamTy(t))
852+
ControlFlow::Break(OrphanCheckEarlyExit::UncoveredTy(t))
807853
}
808854
}
809855

@@ -817,7 +863,7 @@ where
817863

818864
enum OrphanCheckEarlyExit<'tcx, E> {
819865
NormalizationFailure(E),
820-
ParamTy(Ty<'tcx>),
866+
UncoveredTy(Ty<'tcx>),
821867
LocalTy(Ty<'tcx>),
822868
}
823869

@@ -850,12 +896,17 @@ where
850896
| ty::Slice(..)
851897
| ty::RawPtr(..)
852898
| ty::Never
853-
| ty::Tuple(..)
854-
| ty::Alias(ty::Projection | ty::Inherent | ty::Weak, ..) => {
855-
self.found_non_local_ty(ty)
899+
| ty::Tuple(..) => self.found_non_local_ty(ty),
900+
901+
ty::Alias(ty::Projection | ty::Inherent | ty::Weak, ..) => {
902+
if ty.has_type_flags(ty::TypeFlags::HAS_TY_PARAM) {
903+
self.found_uncovered_ty(ty)
904+
} else {
905+
ControlFlow::Continue(())
906+
}
856907
}
857908

858-
ty::Param(..) => self.found_param_ty(ty),
909+
ty::Param(..) => self.found_uncovered_ty(ty),
859910

860911
ty::Placeholder(..) | ty::Bound(..) | ty::Infer(..) => match self.in_crate {
861912
InCrate::Local => self.found_non_local_ty(ty),

‎tests/ui/associated-types/issue-38821.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ pub trait Column: Expression {}
2222

2323
#[derive(Debug, Copy, Clone)]
2424
//~^ ERROR the trait bound `<Col as Expression>::SqlType: NotNull` is not satisfied
25+
//~| ERROR the trait bound `<Col as Expression>::SqlType: NotNull` is not satisfied
26+
//~| ERROR the trait bound `<Col as Expression>::SqlType: NotNull` is not satisfied
2527
pub enum ColumnInsertValue<Col, Expr> where
2628
Col: Column,
2729
Expr: Expression<SqlType=<Col::SqlType as IntoNullable>::Nullable>,

‎tests/ui/associated-types/issue-38821.stderr

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,22 @@
1+
error[E0277]: the trait bound `<Col as Expression>::SqlType: NotNull` is not satisfied
2+
--> $DIR/issue-38821.rs:23:10
3+
|
4+
LL | #[derive(Debug, Copy, Clone)]
5+
| ^^^^^ the trait `NotNull` is not implemented for `<Col as Expression>::SqlType`
6+
|
7+
note: required for `<Col as Expression>::SqlType` to implement `IntoNullable`
8+
--> $DIR/issue-38821.rs:9:18
9+
|
10+
LL | impl<T: NotNull> IntoNullable for T {
11+
| ------- ^^^^^^^^^^^^ ^
12+
| |
13+
| unsatisfied trait bound introduced here
14+
= note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)
15+
help: consider further restricting the associated type
16+
|
17+
LL | Expr: Expression<SqlType=<Col::SqlType as IntoNullable>::Nullable>, <Col as Expression>::SqlType: NotNull,
18+
| +++++++++++++++++++++++++++++++++++++++
19+
120
error[E0277]: the trait bound `<Col as Expression>::SqlType: NotNull` is not satisfied
221
--> $DIR/issue-38821.rs:23:17
322
|
@@ -17,6 +36,25 @@ help: consider further restricting the associated type
1736
LL | Expr: Expression<SqlType=<Col::SqlType as IntoNullable>::Nullable>, <Col as Expression>::SqlType: NotNull,
1837
| +++++++++++++++++++++++++++++++++++++++
1938

20-
error: aborting due to previous error
39+
error[E0277]: the trait bound `<Col as Expression>::SqlType: NotNull` is not satisfied
40+
--> $DIR/issue-38821.rs:23:23
41+
|
42+
LL | #[derive(Debug, Copy, Clone)]
43+
| ^^^^^ the trait `NotNull` is not implemented for `<Col as Expression>::SqlType`
44+
|
45+
note: required for `<Col as Expression>::SqlType` to implement `IntoNullable`
46+
--> $DIR/issue-38821.rs:9:18
47+
|
48+
LL | impl<T: NotNull> IntoNullable for T {
49+
| ------- ^^^^^^^^^^^^ ^
50+
| |
51+
| unsatisfied trait bound introduced here
52+
= note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info)
53+
help: consider further restricting the associated type
54+
|
55+
LL | Expr: Expression<SqlType=<Col::SqlType as IntoNullable>::Nullable>, <Col as Expression>::SqlType: NotNull,
56+
| +++++++++++++++++++++++++++++++++++++++
57+
58+
error: aborting due to 3 previous errors
2159

2260
For more information about this error, try `rustc --explain E0277`.

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
pub trait Trait0<T, U, V> {}
2+
pub trait Trait1<T, U> {}

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Projections can cover type parameters if they normalize to a (local) type that covers them.
2+
// This ensures that we don't perform an overly strict check on
3+
// projections like in closed PR #100555 which did a syntactic
4+
// check for type parameters in projections without normalizing
5+
// first which would've lead to real-word regressions.
6+
7+
// check-pass
8+
// revisions: classic next
9+
//[next] compile-flags: -Ztrait-solver=next
10+
11+
// aux-crate:foreign=parametrized-trait.rs
12+
// edition:2021
13+
14+
trait Project { type Output; }
15+
16+
impl<T> Project for T {
17+
type Output = Local;
18+
}
19+
20+
struct Local;
21+
22+
impl<T> foreign::Trait1<Local, T> for <T as Project>::Output {}
23+
24+
fn main() {}

0 commit comments

Comments
 (0)