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

Various refactorings towards universe-based regions #45657

Conversation

nikomatsakis
Copy link
Contributor

These are a variety of refactorings that I did on my lazy normalization. This code is not expected to change the behavior in any particular way, just pave the way for using universes more uniformly in place of the existing "leak check" approach (and, in turn, for normalization).

One of the notable things that this branch does is to adopt the unification table from the ena project in rust-lang-nursery, which is of course branched from the compiler sources.

@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@nikomatsakis
Copy link
Contributor Author

r? @arielb1

@rust-highfive rust-highfive assigned arielb1 and unassigned eddyb Oct 31, 2017
@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 31, 2017
@bors
Copy link
Contributor

bors commented Nov 5, 2017

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

@nikomatsakis nikomatsakis force-pushed the chalkify-universe-refactorings-1 branch from f74f4d9 to edff811 Compare November 7, 2017 22:16
// bug; it was introduced when we added the `eq_relations`
// table, but it's hard to create rust code that triggers it.)
//
// We could tell the table to prefer lower vids, and then we would
Copy link
Contributor

@arielb1 arielb1 Nov 9, 2017

Choose a reason for hiding this comment

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

We could also store the minimum vid of a node's childrenonce-children on each tree node and propagate that when we unify. This would allow us to find the lowest vid of a subtree by looking at the root node's vid, while being O(α(n)) to maintain.

@bors
Copy link
Contributor

bors commented Nov 12, 2017

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

@@ -285,6 +285,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
let element_tys_iter = (0..max_len).map(|_| self.next_ty_var(
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks ugly - would introduce a next_global_ty_var function on fcx.

@@ -1250,6 +1332,17 @@ pub struct ParamEnv<'tcx> {
/// want `Reveal::All` -- note that this is always paired with an
/// empty environment. To get that, use `ParamEnv::reveal()`.
pub reveal: traits::Reveal,

Copy link
Contributor

Choose a reason for hiding this comment

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

I find ParamEnv containing a universe rather unsatisfying - it doesn't fit in my mental model.

@nikomatsakis
Copy link
Contributor Author

So @arielb1 and I had a chat on #rustc. We said some interesting stuff. Some notes:

  • It'd be nice if we can use universes as a "better leak-check".
  • In principle, we could generate a fresh universe (rather than keeping a stack) each time we enter a binder, which would mean that we don't need to track the universe in the parameter environment.
    • Later, we can canonicalize universes anyway, since the only thing that really matters is the relative order in which they were created.
  • We do want to be a bit careful with how we integrate universes into caching and so forth anyway.

I have to think over just how I want to adjust this PR. I think I will close it for the time being and mull things over.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants