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

WIP: Trust parameter env when projecting GATs #98742

Closed
wants to merge 2 commits into from

Conversation

nikomatsakis
Copy link
Contributor

While investigating the behavior for GAT implied bounds, I realized that we were enforcing stricter requirements when projecting than I believe we should -- in particular, if the where-clause says "this is true for all 'a", we should believe it.

This fixes a lot of unnecessary errors from GATs, though not all (see the new test I added, which shows why this is still sound and reasonable).

However, it also may introduce a backwards incompatibility hazard, albeit a very unlikely one, in that we are going to modify the meaning of this where-clause, and hence some functions are accepted now that won't be accepted later, even though they aren't callable today.

r? @jackh726

If the param-env tells you that, forall 'a, `T::Item<'a> = u32`,
we can believe it.
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 30, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 30, 2022
`call-forall` shows the expected behavior: the where-clause proves
too much, so we can't prove it.

`backwards-incompat` shows the problem with that: we can construct
types that we ought not to be able to construct.
@nikomatsakis
Copy link
Contributor Author

Thinking on it more, as the backwards-incompat suggests, this behavior isn't more right -- that is, I think we should be trusting the where-clause, but we have the wrong where-clause, so... in a way the current code enforces something closer to the final behavior (but not completely). Anyway, prob shouldn't land, but it was a good exercise for me.

@bors
Copy link
Contributor

bors commented Jul 1, 2022

☔ The latest upstream changes (presumably #93967) made this pull request unmergeable. Please resolve the merge conflicts.

@jackh726 jackh726 added S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 10, 2022
@Dylan-DPC
Copy link
Member

Closing this as it was an experiment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants