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

Normalize possibly un-normalized GAT projections #93361

Closed
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
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/type_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ impl<'tcx> TypeVariableTable<'_, 'tcx> {
let index = self.values().push(TypeVariableData { origin });
assert_eq!(eq_key.vid.as_u32(), index as u32);

debug!("new_var(index={:?}, universe={:?}, origin={:?}", eq_key.vid, universe, origin,);
debug!("new_var(index={:?}, universe={:?}, origin={:?})", eq_key.vid, universe, origin);

eq_key.vid
}
Expand Down
13 changes: 12 additions & 1 deletion compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,18 @@ fn project_and_unify_type<'cx, 'tcx>(

let infcx = selcx.infcx();
// FIXME(associated_const_equality): Handle consts here as well as types.
let obligation_pred_ty = obligation.predicate.term.ty().unwrap();
// FIXME(compiler-errors): We need to normalize here until we properly handle
// GAT projection types that we can't normalize properly, see note in
// `rustc_trait_selection::traits::project`
Copy link
Member

Choose a reason for hiding this comment

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

This...is the note?

Copy link
Member Author

Choose a reason for hiding this comment

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

lmao sorry, copied this from another file and it's definitely ambiguous now.

I meant the note about trying to normalize GAT projections then treating them differently if they fail to normalize (not inserting a type variable because it would capture escaping bounds). I think I linked it in the PR description. I'll fix this comment when I get to my computer.

let obligation_pred_ty = normalize_with_depth_to(
selcx,
obligation.param_env,
obligation.cause.clone(),
obligation.recursion_depth + 1,
obligation.predicate.term.ty().unwrap(),
&mut obligations,
);

match infcx.at(&obligation.cause, obligation.param_env).eq(normalized_ty, obligation_pred_ty) {
Ok(InferOk { obligations: inferred_obligations, value: () }) => {
obligations.extend(inferred_obligations);
Expand Down
103 changes: 30 additions & 73 deletions compiler/rustc_trait_selection/src/traits/select/confirmation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,23 +555,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
)
.map_bound(|(trait_ref, _)| trait_ref);

let Normalized { value: trait_ref, mut obligations } = ensure_sufficient_stack(|| {
normalize_with_depth(
self,
obligation.param_env,
obligation.cause.clone(),
obligation.recursion_depth + 1,
trait_ref,
)
});

obligations.extend(self.confirm_poly_trait_refs(
obligation.cause.clone(),
obligation.param_env,
obligation.predicate.to_poly_trait_ref(),
trait_ref,
)?);
Ok(ImplSourceFnPointerData { fn_ty: self_ty, nested: obligations })
let nested = self.confirm_poly_trait_refs(obligation, trait_ref)?;
Ok(ImplSourceFnPointerData { fn_ty: self_ty, nested })
}

fn confirm_trait_alias_candidate(
Expand Down Expand Up @@ -618,26 +603,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
debug!(?obligation, ?generator_def_id, ?substs, "confirm_generator_candidate");

let trait_ref = self.generator_trait_ref_unnormalized(obligation, substs);
let Normalized { value: trait_ref, mut obligations } = ensure_sufficient_stack(|| {
normalize_with_depth(
self,
obligation.param_env,
obligation.cause.clone(),
obligation.recursion_depth + 1,
trait_ref,
)
});

debug!(?trait_ref, ?obligations, "generator candidate obligations");

obligations.extend(self.confirm_poly_trait_refs(
obligation.cause.clone(),
obligation.param_env,
obligation.predicate.to_poly_trait_ref(),
trait_ref,
)?);
let nested = self.confirm_poly_trait_refs(obligation, trait_ref)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this represent cleanup or a change in behavior? I guess we are now normalizing the obligation trait-ref as well? What are we doing that, exactly?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe it was previously an invariant that the obligation trait-ref was always normalized, and it seems like jack's PR also helps ensure that.

Copy link
Member Author

Choose a reason for hiding this comment

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

We actually landed this specific part of the PR in #94108. So this is change in behavior technically (and kinda cleanup, but mostly the former). @jackh726 and I thought that this (#94108) would offset the perf regression in #90887, but then we came to the conclusion that the perf regression in #90887 is probably noise.

#94108 probably doesn't need to be around after that PR lands. I can put up a revert once it lands, to see if we see perf regressions.

debug!(?trait_ref, ?nested, "generator candidate obligations");

Ok(ImplSourceGeneratorData { generator_def_id, substs, nested: obligations })
Ok(ImplSourceGeneratorData { generator_def_id, substs, nested })
}

#[instrument(skip(self), level = "debug")]
Expand All @@ -659,52 +629,23 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
_ => bug!("closure candidate for non-closure {:?}", obligation),
};

let obligation_predicate = obligation.predicate;
let Normalized { value: obligation_predicate, mut obligations } =
ensure_sufficient_stack(|| {
normalize_with_depth(
self,
obligation.param_env,
obligation.cause.clone(),
obligation.recursion_depth + 1,
obligation_predicate,
)
});

let trait_ref = self.closure_trait_ref_unnormalized(obligation, substs);
let Normalized { value: trait_ref, obligations: trait_ref_obligations } =
ensure_sufficient_stack(|| {
normalize_with_depth(
self,
obligation.param_env,
obligation.cause.clone(),
obligation.recursion_depth + 1,
trait_ref,
)
});
let mut nested = self.confirm_poly_trait_refs(obligation, trait_ref)?;

debug!(?closure_def_id, ?trait_ref, ?obligations, "confirm closure candidate obligations");

obligations.extend(trait_ref_obligations);
obligations.extend(self.confirm_poly_trait_refs(
obligation.cause.clone(),
obligation.param_env,
obligation_predicate.to_poly_trait_ref(),
trait_ref,
)?);
debug!(?closure_def_id, ?trait_ref, ?nested, "confirm closure candidate obligations");

// FIXME: Chalk

if !self.tcx().sess.opts.debugging_opts.chalk {
obligations.push(Obligation::new(
nested.push(Obligation::new(
obligation.cause.clone(),
obligation.param_env,
ty::Binder::dummy(ty::PredicateKind::ClosureKind(closure_def_id, substs, kind))
.to_predicate(self.tcx()),
));
}

Ok(ImplSourceClosureData { closure_def_id, substs, nested: obligations })
Ok(ImplSourceClosureData { closure_def_id, substs, nested })
}

/// In the case of closure types and fn pointers,
Expand Down Expand Up @@ -735,15 +676,31 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
#[instrument(skip(self), level = "trace")]
fn confirm_poly_trait_refs(
&mut self,
obligation_cause: ObligationCause<'tcx>,
obligation_param_env: ty::ParamEnv<'tcx>,
obligation_trait_ref: ty::PolyTraitRef<'tcx>,
obligation: &TraitObligation<'tcx>,
expected_trait_ref: ty::PolyTraitRef<'tcx>,
) -> Result<Vec<PredicateObligation<'tcx>>, SelectionError<'tcx>> {
let obligation_trait_ref = obligation.predicate.to_poly_trait_ref();
// Normalize the obligation and expected trait refs together, because why not
let Normalized { obligations: nested, value: (obligation_trait_ref, expected_trait_ref) } =
ensure_sufficient_stack(|| {
self.infcx.commit_unconditionally(|_| {
normalize_with_depth(
self,
obligation.param_env,
obligation.cause.clone(),
obligation.recursion_depth + 1,
(obligation_trait_ref, expected_trait_ref),
)
})
});

self.infcx
.at(&obligation_cause, obligation_param_env)
.at(&obligation.cause, obligation.param_env)
.sup(obligation_trait_ref, expected_trait_ref)
.map(|InferOk { obligations, .. }| obligations)
.map(|InferOk { mut obligations, .. }| {
obligations.extend(nested);
obligations
})
.map_err(|e| OutputTypeParameterMismatch(expected_trait_ref, obligation_trait_ref, e))
}

Expand Down
55 changes: 55 additions & 0 deletions src/test/ui/generic-associated-types/issue-93341.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// check-pass

#![feature(generic_associated_types)]
use std::marker::PhantomData;

pub struct Id<'id>(PhantomData<fn(&'id ()) -> &'id ()>);

fn new_id() -> Id<'static> {
Id(PhantomData)
}

pub trait HasLifetime where {
type AtLifetime<'a>;
}

pub struct ExistentialLifetime<S: HasLifetime>(S::AtLifetime<'static>);

impl<S: HasLifetime> ExistentialLifetime<S> {
pub fn new<F>(f: F) -> ExistentialLifetime<S>
where for<'id> F: FnOnce(Id<'id>) -> S::AtLifetime<'id> {
ExistentialLifetime(f(new_id()))
}
}


struct ExampleS<'id>(Id<'id>);

struct ExampleMarker;

impl HasLifetime for ExampleMarker {
type AtLifetime<'id> = ExampleS<'id>;
}


fn broken0() -> ExistentialLifetime<ExampleMarker> {
fn new_helper<'id>(id: Id<'id>) -> ExampleS<'id> {
ExampleS(id)
}

ExistentialLifetime::<ExampleMarker>::new(new_helper)
}

fn broken1() -> ExistentialLifetime<ExampleMarker> {
fn new_helper<'id>(id: Id<'id>) -> <ExampleMarker as HasLifetime>::AtLifetime<'id> {
ExampleS(id)
}

ExistentialLifetime::<ExampleMarker>::new(new_helper)
}

fn broken2() -> ExistentialLifetime<ExampleMarker> {
ExistentialLifetime::<ExampleMarker>::new(|id| ExampleS(id))
}

fn main() {}
57 changes: 57 additions & 0 deletions src/test/ui/generic-associated-types/issue-93342.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// check-pass

#![feature(generic_associated_types)]

use std::marker::PhantomData;

pub trait Scalar: 'static {
type RefType<'a>: ScalarRef<'a>;
}

pub trait ScalarRef<'a>: 'a {}

impl Scalar for i32 {
type RefType<'a> = i32;
}

impl Scalar for String {
type RefType<'a> = &'a str;
}

impl Scalar for bool {
type RefType<'a> = i32;
}

impl<'a> ScalarRef<'a> for bool {}

impl<'a> ScalarRef<'a> for i32 {}

impl<'a> ScalarRef<'a> for &'a str {}

fn str_contains(a: &str, b: &str) -> bool {
a.contains(b)
}

pub struct BinaryExpression<A: Scalar, B: Scalar, O: Scalar, F>
where
F: Fn(A::RefType<'_>, B::RefType<'_>) -> O,
{
f: F,
_phantom: PhantomData<(A, B, O)>,
}

impl<A: Scalar, B: Scalar, O: Scalar, F> BinaryExpression<A, B, O, F>
where
F: Fn(A::RefType<'_>, B::RefType<'_>) -> O,
{
pub fn new(f: F) -> Self {
Self {
f,
_phantom: PhantomData,
}
}
}

fn main() {
BinaryExpression::<String, String, bool, _>::new(str_contains);
}
4 changes: 2 additions & 2 deletions src/test/ui/higher-rank-trait-bounds/issue-60283.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// check-pass

pub trait Trait<'a> {
type Item;
}
Expand All @@ -15,6 +17,4 @@ where

fn main() {
foo((), drop)
//~^ ERROR type mismatch in function arguments
//~| ERROR size for values of type `<() as Trait<'_>>::Item` cannot be known at compilation time
}
42 changes: 0 additions & 42 deletions src/test/ui/higher-rank-trait-bounds/issue-60283.stderr

This file was deleted.

1 change: 0 additions & 1 deletion src/test/ui/hrtb/issue-62203-hrtb-ice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ trait Ty<'a> {

fn main() {
let v = Unit2.m(
//~^ ERROR type mismatch
L {
//~^ ERROR type mismatch
f : |x| { drop(x); Unit4 }
Expand Down
Loading