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

"approximate check" for outlives relations can miss matches #54572

Closed
nikomatsakis opened this issue Sep 25, 2018 · 4 comments
Closed

"approximate check" for outlives relations can miss matches #54572

nikomatsakis opened this issue Sep 25, 2018 · 4 comments
Labels
A-NLL Area: Non-lexical lifetimes (NLL) NLL-complete Working towards the "valid code works" goal P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

Per this comment, the current system for handling outlives obligations relies on an "approximate" match for outlives relations that is actually kind of flawed in some edge cases, which can lead to us overconstraining inference:

It's not really meant to be conservative in "two dimensions". That is, projection_approx_declared_bounds_from_env ought to return anything that might potentially be useful. I suspect this is not presently true, though, around bound regions and a few other edge cases (e.g., I think we by and large ignore things like for<'a> T::Proj<'a>: 'a, which is really a bug, although a pre-existing one; we also are too conservative I think around equality of types like for<'a> fn(&'a u32) vs for<'b> fn(&'b u32). Also a pre-existing bug.)

I might be able to fix those cases where we are "overly" conservative though via a more thorough erasure. I think erasing regions today preserves the lifetime bounds, but we would ideally look for an "equality" check that just completely ignores region equality (even around binders). I was working towards that but never finished it.

Originally, I had intended to fix this by using the "type relation" code from the NLL type checker, but in a mode where it ignored all the outlives relations. I introduced the idea of an "outlives delegate" and so forth in preparation for that but then never took the last step, since "erasing obligations" seemed simpler and sufficient for the moment.

@nikomatsakis nikomatsakis added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-NLL Area: Non-lexical lifetimes (NLL) labels Sep 25, 2018
@pnkfelix pnkfelix added the NLL-complete Working towards the "valid code works" goal label Oct 2, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Oct 2, 2018

SInce this is about getting "valid code" to compile, tagging as NLL-complete. But @nikomatsakis notes that it does not feel that urgent, so we're also tagging as NLL-deferred.

@pnkfelix
Copy link
Member

Re-triaging for #56754. P-low.

@pnkfelix pnkfelix added P-low Low priority and removed NLL-deferred labels Dec 21, 2018
@aliemjay
Copy link
Member

aliemjay commented Mar 2, 2023

I believe the work intended here was implemented in #98109. We now support where for<'a> T::Proj<'a>: 'a and we have equality around binders via the test_type_match machinery.

Close?

@aliemjay
Copy link
Member

aliemjay commented May 1, 2023

Closing as fixed in #98109.

@aliemjay aliemjay closed this as completed May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NLL Area: Non-lexical lifetimes (NLL) NLL-complete Working towards the "valid code works" goal P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants