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

support transient inference contexts in the SLG solver #97

Merged
merged 11 commits into from Mar 19, 2018

Conversation

Projects
None yet
1 participant
@nikomatsakis
Copy link
Collaborator

nikomatsakis commented Mar 16, 2018

This PR shifts the way that the SLG trait works to support "transient" inference contexts. In the older code, each strand in the SLG solver had an associated inference context that it would carry with it. When we created new strands, we would fork this inference context (using clone). (My intention was to move eventually to a persistent data structure for this, although it's not obvious that this would be faster.)

However, the approach of storing an inference context per stand was pretty incompatible with how rustc's inference contexts work. They are always bound to some closure, so you can't store them in the heap, return, and then come back and use them later. The reason for this is that rustc creates an arena per inference context to store the temporary types, and then throws this arena away when you return from the closure.

Originally, I hoped to address this by changing Rust's API to use a "Rental-like" interface. But this proved more challenging than I anticipated. I made some progress, but I kept hitting annoying edge cases in the trait system: notably, the lack of generic associated types and the shortcomings of normalization under binders. Ironically, two of the bugs I most hope to fix via this move to Chalk! It seemed to me that this route was not going anywhere.

So this PR takes a different tack. The SLG solver trait now never gives ownership of an inference context away; instead, it invokes a closure with a &mut dyn InferenceTable dyn-trait. This means that the callee can only use that inference table during the closure and cannot allow it to "escape" onto the heap (the anonymous lifetime ensures that).

The solver in turn then distinguishes between strands that are "at rest", waiting in some table, and the current strand. A strand-at-rest is called a CanonicalStrand, and it is stored in canonical form. When we pick up a strand to run with it, we create a fresh inference context, instantiate its variables, and then start using it.

As implemented, this is probably fairly inefficient. We have to do a lot of substitution on a pretty regular basis. But I'm mostly interested in pushing through until we have something that works, then I think we can come back and revisit some of these integration questions. See for example the last commit, which suggests that it might be worthwhile to push hard on just making inference contexts potentially really light weight (and also optimizing canonicalization).

nikomatsakis added some commits Mar 14, 2018

have `resolvent_clause` produce canonical ex-clause
`resolvent_clause` is now side-effect-free as well, removing the need to
clone the inference table.
supply inference table via a closure callback
This is a bit annoying from a borrowck perspective, but nothing a little
clone can't fix. =) Cloning shouldn't be a problem in rustc because the
types involved are almost always interned and Copy.
pass `InferenceTable` via `dyn Trait`
This will (eventually) allow it to contain arbitrary lifetimes.
change to use an `&mut dyn InferenceTable<C>` instead
Strand now carries a lifetime `'table`

@nikomatsakis nikomatsakis requested a review from scalexm Mar 16, 2018

pointless micro-optimization: clone instead of substitute
When we create a fresh inference context, we always map all free
variables to themselves, so we don't need to substitute.
@nikomatsakis

This comment has been minimized.

Copy link
Collaborator Author

nikomatsakis commented Mar 16, 2018

See also this internals thread for more discussion.

@nikomatsakis nikomatsakis merged commit 7cf55a6 into rust-lang:master Mar 19, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@nikomatsakis

This comment has been minimized.

Copy link
Collaborator Author

nikomatsakis commented Mar 19, 2018

After some discussion with @scalexm, gonna land this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.