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 obligations before matching them against a candidate #109115

Closed
wants to merge 1 commit into from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Mar 14, 2023

Noticed again during #108860

When we are matching an impl against a predicate obligation, we normalize the impl predicate, but not the obligation predicate, causing errors like trait bound for<'a> <Type as Trait>::Assoc<'a>: OtherTrait is not satisfied, even though we have all the information available to figure out that the associated type is a type that implements OtherTrait.

r? types

fixes #88460
fixes #90950
fixes #89196

@oli-obk oli-obk added I-types-nominated Nominated for discussion during a types team meeting. T-types Relevant to the types team, which will review and decide on the PR/issue. labels Mar 14, 2023
@rustbot
Copy link
Collaborator

rustbot commented Mar 14, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jackh726 (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 14, 2023
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

we tend to assume that obligation is already normalized. fulfill has an explicit normalization step, maybe we should also explicitly normalize at the start of evaluate instead? That should supersede both this PR and #103695

@oli-obk
Copy link
Contributor Author

oli-obk commented Mar 14, 2023

I guess we can leave this for the new trait solver.

The issue is with the existing normalization during process_obligation. This PR does the normalization after replacing bound vars with inference vars, where it trivially succeeds (by replacing the projection with an inference var). So... I guess the right solution would be to look into

// If there are escaping bound vars, we temporarily replace the
// bound vars with placeholders. Note though, that in the case
// that we still can't project for whatever reason (e.g. self
// type isn't known enough), we *can't* register an obligation
// and return an inference variable (since then that obligation
// would have bound vars and that's a can of worms). Instead,
// we just give up and fall back to pretending like we never tried!
//
// Note: this isn't necessarily the final approach here; we may
// want to figure out how to register obligations with escaping vars
// or handle this some other way.
after #108860

@aliemjay
Copy link
Member

Doesn't this have the same problems as @BoxyUwU's attempt at deferred projection equality? particularly the type enum regression.

@jackh726
Copy link
Member

jackh726 commented Apr 9, 2023

I'm really not sure what the state of this is. r? @lcnr

@oli-obk
Copy link
Contributor Author

oli-obk commented May 8, 2023

Closing this PR as it's a very lazy (as in, the work I did) approach to the problem that has unclear effects on other parts of trait solving. We'll wait for the new solver to land and then try again.

@oli-obk oli-obk closed this May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-types-nominated Nominated for discussion during a types team meeting. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
5 participants