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.

Sign up for GitHub

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

Jump to bottom

Normalize possibly un-normalized GAT projections #93361

Closed
wants to merge 3 commits into from
Closed

Normalize possibly un-normalized GAT projections #93361

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
573a4da
Normalize possibly un-normalized GAT projections
compiler-errors Jan 27, 2022
55466d6
Fix bug (eq type to itself), add tests for GATs in Fn trait bounds
compiler-errors Jan 27, 2022
968012d
bless tests changed due to more projection normalization
compiler-errors Jan 27, 2022
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
View file
Open in desktop
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
View file
Open in desktop
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
105 changes: 32 additions & 73 deletions compiler/rustc_trait_selection/src/traits/select/confirmation.rs
View file
Open in desktop
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");
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");

obligations.extend(self.confirm_poly_trait_refs(
obligation.cause.clone(),
obligation.param_env,
obligation.predicate.to_poly_trait_ref(),
trait_ref,
)?);

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,
)
});

debug!(?closure_def_id, ?trait_ref, ?obligations, "confirm closure candidate obligations");
let mut nested = self.confirm_poly_trait_refs(obligation, trait_ref)?;

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,33 @@ 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 Normalized { obligations: mut nested, value: obligation_trait_ref } =
normalize_with_depth(
self,
obligation.param_env,
obligation.cause.clone(),
obligation.recursion_depth + 1,
obligation.predicate.to_poly_trait_ref(),
);
let expected_trait_ref = normalize_with_depth_to(
self,
obligation.param_env,
obligation.cause.clone(),
obligation.recursion_depth + 1,
obligation.predicate.to_poly_trait_ref(),
compiler-errors marked this conversation as resolved.
Show resolved Hide resolved
&mut nested,
);

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