Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deeply normalize item bounds in new solver #137000

Merged
merged 2 commits into from
Feb 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 22 additions & 42 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2105,18 +2105,21 @@ pub(super) fn check_type_bounds<'tcx>(
ObligationCause::new(impl_ty_span, impl_ty_def_id, code)
};

let mut obligations: Vec<_> = tcx
.explicit_item_bounds(trait_ty.def_id)
.iter_instantiated_copied(tcx, rebased_args)
.map(|(concrete_ty_bound, span)| {
debug!(?concrete_ty_bound);
traits::Obligation::new(tcx, mk_cause(span), param_env, concrete_ty_bound)
})
.collect();
let mut obligations: Vec<_> = util::elaborate(
tcx,
tcx.explicit_item_bounds(trait_ty.def_id).iter_instantiated_copied(tcx, rebased_args).map(
|(concrete_ty_bound, span)| {
debug!(?concrete_ty_bound);
traits::Obligation::new(tcx, mk_cause(span), param_env, concrete_ty_bound)
},
),
)
.collect();

// Only in a const implementation do we need to check that the `~const` item bounds hold.
if tcx.is_conditionally_const(impl_ty_def_id) {
obligations.extend(
obligations.extend(util::elaborate(
tcx,
tcx.explicit_implied_const_bounds(trait_ty.def_id)
.iter_instantiated_copied(tcx, rebased_args)
.map(|(c, span)| {
Expand All @@ -2127,34 +2130,27 @@ pub(super) fn check_type_bounds<'tcx>(
c.to_host_effect_clause(tcx, ty::BoundConstness::Maybe),
)
}),
);
));
}
debug!(item_bounds=?obligations);

// Normalize predicates with the assumption that the GAT may always normalize
// to its definition type. This should be the param-env we use to *prove* the
// predicate too, but we don't do that because of performance issues.
// See <https://github.com/rust-lang/rust/pull/117542#issue-1976337685>.
let trait_projection_ty = Ty::new_projection_from_args(tcx, trait_ty.def_id, rebased_args);
let impl_identity_ty = tcx.type_of(impl_ty.def_id).instantiate_identity();
let normalize_param_env = param_env_with_gat_bounds(tcx, impl_ty, impl_trait_ref);
for mut obligation in util::elaborate(tcx, obligations) {
let normalized_predicate = if infcx.next_trait_solver() {
obligation.predicate.fold_with(&mut ReplaceTy {
tcx,
from: trait_projection_ty,
to: impl_identity_ty,
})
} else {
ocx.normalize(&normalize_cause, normalize_param_env, obligation.predicate)
};
debug!(?normalized_predicate);
obligation.predicate = normalized_predicate;

ocx.register_obligation(obligation);
for obligation in &mut obligations {
match ocx.deeply_normalize(&normalize_cause, normalize_param_env, obligation.predicate) {
Ok(pred) => obligation.predicate = pred,
Err(e) => {
return Err(infcx.err_ctxt().report_fulfillment_errors(e));
}
}
}

// Check that all obligations are satisfied by the implementation's
// version.
ocx.register_obligations(obligations);
let errors = ocx.select_all_or_error();
if !errors.is_empty() {
let reported = infcx.err_ctxt().report_fulfillment_errors(errors);
Expand All @@ -2166,22 +2162,6 @@ pub(super) fn check_type_bounds<'tcx>(
ocx.resolve_regions_and_report_errors(impl_ty_def_id, param_env, assumed_wf_types)
}

struct ReplaceTy<'tcx> {
tcx: TyCtxt<'tcx>,
from: Ty<'tcx>,
to: Ty<'tcx>,
}

impl<'tcx> TypeFolder<TyCtxt<'tcx>> for ReplaceTy<'tcx> {
fn cx(&self) -> TyCtxt<'tcx> {
self.tcx
}

fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> {
if self.from == ty { self.to } else { ty.super_fold_with(self) }
}
}

/// Install projection predicates that allow GATs to project to their own
/// definition types. This is not allowed in general in cases of default
/// associated types in trait definitions, or when specialization is involved,
Expand Down
37 changes: 29 additions & 8 deletions compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,7 @@ where
return Ok(self.make_ambiguous_response_no_constraints(MaybeCause::Ambiguity));
};

let responses: Vec<_> = match proven_via {
match proven_via {
// Even when a trait bound has been proven using a where-bound, we
// still need to consider alias-bounds for normalization, see
// tests/ui/next-solver/alias-bound-shadowed-by-env.rs.
Expand All @@ -800,7 +800,7 @@ where
// constness checking. Doing so is *at least theoretically* breaking,
// see github.com/rust-lang/rust/issues/133044#issuecomment-2500709754
TraitGoalProvenVia::ParamEnv | TraitGoalProvenVia::AliasBound => {
let mut candidates_from_env: Vec<_> = candidates
let mut candidates_from_env_and_bounds: Vec<_> = candidates
.iter()
.filter(|c| {
matches!(
Expand All @@ -813,16 +813,37 @@ where

// If the trait goal has been proven by using the environment, we want to treat
// aliases as rigid if there are no applicable projection bounds in the environment.
if candidates_from_env.is_empty() {
if candidates_from_env_and_bounds.is_empty() {
if let Ok(response) = inject_normalize_to_rigid_candidate(self) {
candidates_from_env.push(response);
candidates_from_env_and_bounds.push(response);
}
}
candidates_from_env

if let Some(response) = self.try_merge_responses(&candidates_from_env_and_bounds) {
Ok(response)
} else {
self.flounder(&candidates_from_env_and_bounds)
}
}
TraitGoalProvenVia::Misc => candidates.iter().map(|c| c.result).collect(),
};
TraitGoalProvenVia::Misc => {
// Prefer "orphaned" param-env normalization predicates, which are used
// (for example, and ideally only) when proving item bounds for an impl.
let candidates_from_env: Vec<_> = candidates
.iter()
.filter(|c| matches!(c.source, CandidateSource::ParamEnv(_)))
.map(|c| c.result)
.collect();
if let Some(response) = self.try_merge_responses(&candidates_from_env) {
return Ok(response);
}

self.try_merge_responses(&responses).map_or_else(|| self.flounder(&responses), Ok)
let responses: Vec<_> = candidates.iter().map(|c| c.result).collect();
if let Some(response) = self.try_merge_responses(&responses) {
Ok(response)
} else {
self.flounder(&responses)
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0478]: lifetime bound not satisfied
--> $DIR/issue-91883.rs:30:24
--> $DIR/issue-91883.rs:34:24
|
LL | type Cursor<'tx>: Cursor<'tx>
| ----------------------------- definition of `Cursor` from trait
Expand All @@ -8,12 +8,12 @@ LL | type Cursor<'tx> = CursorImpl<'tx>;
| ^^^^^^^^^^^^^^^
|
note: lifetime parameter instantiated with the lifetime `'db` as defined here
--> $DIR/issue-91883.rs:29:6
--> $DIR/issue-91883.rs:33:6
|
LL | impl<'db> Transaction<'db> for TransactionImpl<'db> {
| ^^^
note: but lifetime parameter must outlive the lifetime `'tx` as defined here
--> $DIR/issue-91883.rs:30:17
--> $DIR/issue-91883.rs:34:17
|
LL | type Cursor<'tx> = CursorImpl<'tx>;
| ^^^
Expand Down
20 changes: 20 additions & 0 deletions tests/ui/generic-associated-types/issue-91883.next.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error[E0478]: lifetime bound not satisfied
--> $DIR/issue-91883.rs:34:24
|
LL | type Cursor<'tx> = CursorImpl<'tx>;
| ^^^^^^^^^^^^^^^
|
note: lifetime parameter instantiated with the lifetime `'db` as defined here
--> $DIR/issue-91883.rs:33:6
|
LL | impl<'db> Transaction<'db> for TransactionImpl<'db> {
| ^^^
note: but lifetime parameter must outlive the lifetime `'tx` as defined here
--> $DIR/issue-91883.rs:34:17
|
LL | type Cursor<'tx> = CursorImpl<'tx>;
| ^^^

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0478`.
4 changes: 4 additions & 0 deletions tests/ui/generic-associated-types/issue-91883.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
//@ revisions: current next
//@ ignore-compare-mode-next-solver (explicit revisions)
//@[next] compile-flags: -Znext-solver

use std::fmt::Debug;
use std::marker::PhantomData;

Expand Down
3 changes: 3 additions & 0 deletions tests/ui/impl-trait/in-trait/default-body-with-rpit.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
//@ edition:2021
//@ check-pass
//@ revisions: current next
//@ ignore-compare-mode-next-solver (explicit revisions)
//@[next] compile-flags: -Znext-solver

use std::fmt::Debug;

Expand Down
3 changes: 3 additions & 0 deletions tests/ui/impl-trait/in-trait/nested-rpitit-bounds.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
//@ check-pass
//@ revisions: current next
//@ ignore-compare-mode-next-solver (explicit revisions)
//@[next] compile-flags: -Znext-solver

use std::ops::Deref;

Expand Down
11 changes: 0 additions & 11 deletions tests/ui/traits/const-traits/predicate-entailment-passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,21 @@
#[const_trait] trait Bar {}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See below for why I ripped out the GATs from this test.

impl const Bar for () {}


#[const_trait] trait TildeConst {
type Bar<T> where T: ~const Bar;

fn foo<T>() where T: ~const Bar;
}
impl TildeConst for () {
type Bar<T> = () where T: Bar;

fn foo<T>() where T: Bar {}
}


#[const_trait] trait AlwaysConst {
type Bar<T> where T: const Bar;

fn foo<T>() where T: const Bar;
}
impl AlwaysConst for i32 {
type Bar<T> = () where T: Bar;

fn foo<T>() where T: Bar {}
}
impl const AlwaysConst for u32 {
type Bar<T> = () where T: ~const Bar;

fn foo<T>() where T: ~const Bar {}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,8 @@
error[E0275]: overflow evaluating the requirement `<T as Overflow>::Assoc: Sized`
error[E0275]: overflow evaluating the requirement `<T as Overflow>::Assoc == _`
--> $DIR/trait_ref_is_knowable-norm-overflow.rs:10:18
|
LL | type Assoc = <T as Overflow>::Assoc;
| ^^^^^^^^^^^^^^^^^^^^^^
|
note: required by a bound in `Overflow::Assoc`
--> $DIR/trait_ref_is_knowable-norm-overflow.rs:7:5
|
LL | type Assoc;
| ^^^^^^^^^^^ required by this bound in `Overflow::Assoc`
help: consider relaxing the implicit `Sized` restriction
|
LL | type Assoc: ?Sized;
| ++++++++

error[E0119]: conflicting implementations of trait `Trait`
--> $DIR/trait_ref_is_knowable-norm-overflow.rs:18:1
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/traits/next-solver/gat-wf.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//@ compile-flags: -Znext-solver

// Make sure that, like the old trait solver, we end up requiring that the WC of
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also the cause for the failures in tests/ui/traits/const-traits/predicate-entailment-passes.rs. Where clauses must be equal, which kinda makes the GATs in that test useless.

Copy link
Contributor

@lcnr lcnr Feb 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new solver test without enabling the new solver?

also, instead of adding a new test, maybe revisions and a comment in tests/ui/generic-associated-types/issue-91883.rs are enough?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda prefer an explicit test. 91833 is a mess. I'll add flag, tho, whoops.

// impl GAT matches that of the trait. This is not a restriction that we *need*,
// but is a side-effect of registering the where clauses when normalizing the GAT
// when proving it satisfies its item bounds.

trait Foo {
type T<'a>: Sized where Self: 'a;
}

impl Foo for &() {
type T<'a> = (); //~ the type `&()` does not fulfill the required lifetime
}

fn main() {}
15 changes: 15 additions & 0 deletions tests/ui/traits/next-solver/gat-wf.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0477]: the type `&()` does not fulfill the required lifetime
--> $DIR/gat-wf.rs:13:18
|
LL | type T<'a> = ();
| ^^
|
note: type must outlive the lifetime `'a` as defined here
--> $DIR/gat-wf.rs:13:12
|
LL | type T<'a> = ();
| ^^

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0477`.
Loading