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

Refactor propagation to be more explicit #32730

Closed
nikomatsakis opened this issue Apr 4, 2016 · 9 comments
Closed

Refactor propagation to be more explicit #32730

nikomatsakis opened this issue Apr 4, 2016 · 9 comments
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup.

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Apr 4, 2016

In conjunction with @soltanmm, and with an aim towards enabling lazy normalization, HR-caching, specialization's lifetime dispatch rules, and a number of other things, we should refactor the way we propagate obligations and mutable state through the inference context.

The general idea is to make the result of unification also carry a list of obligations. This can be used by lazy normalizing to carry the results of normalization, but we should also (eventually) move all mutation of the region inference state out and use it to carry region edges that result. This will help simplify the skolemization code, with the lifetime dispatch logic, and with more advanced caching. It might also make sense to eventually move the unification of type variables too, so that unify is a pure function.

At the moment, this issue exists mainly to serve as a placeholder for FIXME, but I hope to expand with a bit more design and also some checklist of to-do items. :)

bors added a commit that referenced this issue Apr 13, 2016
Replace consider_unification_despite_ambiguity with new obligation variant

Is work towards #32730. Addresses part one of #32286. Addresses #24210 and #26046 to some degree.

r? @nikomatsakis
@soltanmm
Copy link

soltanmm commented Jul 5, 2016

A note (both for myself to walk back to and I guess as a minor update):
Turns out that the obligation plumbing is incomplete AFAICT; switching up the variance appears to drop obligations on the floor. So, that'll happen in another PR before the originally planned next-step of propagating region inference obligations for the Sub and Equate relations over regions.

@jroesch
Copy link
Member

jroesch commented Aug 23, 2016

cc me

@nikomatsakis
Copy link
Contributor Author

@soltanmm I have a branch that is using the plumbing now to handle all region obligations. It's not quite right for higher-ranked stuff yet -- but seems to basically work:

failures:
[compile-fail] compile-fail/hr-subtype.rs
[compile-fail] compile-fail/lifetime-bound-will-change-warning.rs
[compile-fail] compile-fail/object-lifetime-default-from-rptr-box-error.rs
[compile-fail] compile-fail/object-lifetime-default-from-rptr-struct-error.rs
[compile-fail] compile-fail/object-lifetime-default-mybox.rs
[compile-fail] compile-fail/regions-bounds.rs
[compile-fail] compile-fail/regions-fn-subtyping-return-static.rs
[compile-fail] compile-fail/regions-infer-at-fn-not-param.rs
[compile-fail] compile-fail/regions-infer-contravariance-due-to-decl.rs
[compile-fail] compile-fail/regions-infer-covariance-due-to-decl.rs
[compile-fail] compile-fail/regions-infer-not-param.rs
[compile-fail] compile-fail/regions-infer-paramd-indirect.rs
[compile-fail] compile-fail/regions-trait-1.rs
[compile-fail] compile-fail/regions-variance-contravariant-use-covariant-in-second-position.rs
[compile-fail] compile-fail/regions-variance-contravariant-use-covariant.rs
[compile-fail] compile-fail/regions-variance-covariant-use-contravariant.rs
[compile-fail] compile-fail/trait-matching-lifetimes.rs
[compile-fail] compile-fail/variance-use-contravariant-struct-1.rs
[compile-fail] compile-fail/variance-use-covariant-struct-1.rs

test result: FAILED. 2465 passed; 19 failed; 13 ignored; 0 measured

@nikomatsakis
Copy link
Contributor Author

@nikomatsakis
Copy link
Contributor Author

I've been plugging away on the higher-ranked code. It's coming along, but it's a bit trickier than I naively expected (e.g., I forgot about plug_leaks).

I'm thinking more and more I need to writeup the overall plan here. I still think we're on the right track, but the end-game is complicated. (e.g., in order to make lazy norm and higher-ranked stuff work -- which is of course one of the main motivations -- is going to require some refactoring to better handle higher-ranked reasoning in general.)

Sorry, I know this is vague, just don't have time for such a write-up this second.

bors added a commit that referenced this issue Nov 6, 2016
[4/n] rustc: harden against InferOk having obligations in more cases.

_This is part of a series ([prev](#37402) | [next](#37408)) of patches designed to rework rustc into an out-of-order on-demand pipeline model for both better feature support (e.g. [MIR-based](https://github.com/solson/miri) early constant evaluation) and incremental execution of compiler passes (e.g. type-checking), with beneficial consequences to IDE support as well.
If any motivation is unclear, please ask for additional PR description clarifications or code comments._

<hr>

This adds more asserts that `InferOk` results have no obligations, pending completion of #32730.

Each of these could accidentally drop obligations on the floor if they start getting produced by unification, and a future change does just that, in order to produce a "shallow success" (hopefully leading to ambiguities during trait selection), _without_ the possibility of an eventual success - mostly guarded by ICEs for now.
nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Nov 15, 2016
Or at least, more properly. I think I left one or two FIXMEs still in
there.

cc rust-lang#32730
nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Apr 19, 2017
cc rust-lang#32730 -- I left exactly one instance where I wasn't sure of the
right behavior.
@Mark-Simulacrum
Copy link
Member

@nikomatsakis Could you provide an update on this?

@soltanmm
Copy link

IIRC all the reasons to do this are similarly addressed by efforts in Chalk and its eventual integration (though I'm saying this as someone who's been out of the loop for a while).

@nikomatsakis
Copy link
Contributor Author

nikomatsakis commented May 24, 2017

@Mark-Simulacrum getting there, still some FIXMEs related to this issue. I hope to be resolving them soon. (ps @soltanmm, we should chat, I don't have your new e-mail)

@Mark-Simulacrum Mark-Simulacrum added C-cleanup Category: PRs that clean code up or issues documenting cleanup. and removed A-refactor labels Jul 24, 2017
@nikomatsakis
Copy link
Contributor Author

I'd call this done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup.
Projects
None yet
Development

No branches or pull requests

4 participants