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

Initial refactor of InferCtxt towards unified type checking state and per item side tables #26582

Merged
merged 5 commits into from
Jun 29, 2015

Conversation

jroesch
Copy link
Member

@jroesch jroesch commented Jun 26, 2015

This branch begins the work of unifying our type checking contexts into a single piece of state. The goal is to eventually have a single context that we can pass around instead of the fractured situation we currently have. There are still several things that must be done before beginning to make tables item local:

  • move FulfillmentContext into InferCtxt
  • modify SelectionContext to only take a single context argument
  • remove remaining typer impls
  • remove the ClosureTyper + Typer trait
  • do some renaming to make these things more applicable to their new roles

r? @nikomatsakis

As a side note there are a couple oddities that are temporary refactors that will be quickly cleaned up in a follow-up PR.

cc @eddyb @Aatch @arielb1 @nrc

@eddyb
Copy link
Member

eddyb commented Jun 26, 2015

Placing Tables inside InferCtxt is better than what I was considering.
We can then have all the nested inference-only type context be managed by InferCtxt, which would include write-back/lifting Tables<'local> to Tables<'global> by resolving all inference variables.

@bors
Copy link
Contributor

bors commented Jun 26, 2015

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

@nikomatsakis
Copy link
Contributor

r+ from me once we resolve nits, get things building

@jroesch jroesch force-pushed the infer-ctxt-refactor branch 3 times, most recently from 4025071 to 57f8253 Compare June 26, 2015 19:23
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 27, 2015

📌 Commit 57f8253 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Jun 27, 2015

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

@bors
Copy link
Contributor

bors commented Jun 27, 2015

🔒 Merge conflict

jroesch added 3 commits June 27, 2015 13:43
This first patch starts by moving around pieces of state related to
type checking. The goal is to slowly unify the type checking state
into a single typing context. This initial patch moves the
ParameterEnvironment into the InferCtxt and moves shared tables
from Inherited and ty::ctxt into their own struct Tables. This
is the foundational work to refactoring the type checker to
enable future evolution of the language and tooling.
@jroesch jroesch force-pushed the infer-ctxt-refactor branch from 57f8253 to fd412ab Compare June 27, 2015 21:51
cast_kinds: RefCell::new(NodeMap()),
}, f)
}

Copy link
Member

Choose a reason for hiding this comment

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

This looks like a rebase mishap (this function is gone on master, moved to ctxt::create_and_enter).

@jroesch jroesch force-pushed the infer-ctxt-refactor branch from fd412ab to 15bc4a3 Compare June 28, 2015 09:44
@eddyb
Copy link
Member

eddyb commented Jun 28, 2015

@bors r=nikomatsakis p=1

@bors
Copy link
Contributor

bors commented Jun 28, 2015

📌 Commit 15bc4a3 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Jun 28, 2015

⌛ Testing commit 15bc4a3 with merge 84855fe...

bors added a commit that referenced this pull request Jun 28, 2015
This branch begins the work of unifying our type checking contexts into a single piece of state. The goal is to eventually have a single context that we can pass around instead of the fractured situation we currently have. There are still several things that must be done before beginning to make tables item local:

- [ ] move FulfillmentContext into InferCtxt
- [ ] modify SelectionContext to only take a single context argument
- [ ] remove remaining typer impls 
- [ ] remove the ClosureTyper + Typer trait
- [ ] do some renaming to make these things more applicable to their new roles

r? @nikomatsakis 

As a side note there are a couple oddities that are temporary refactors that will be quickly cleaned up in a follow-up PR.

cc @eddyb @Aatch @arielb1 @nrc
@bors
Copy link
Contributor

bors commented Jun 28, 2015

💔 Test failed - auto-mac-32-opt

@eddyb
Copy link
Member

eddyb commented Jun 28, 2015

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Jun 28, 2015

📌 Commit 5c3753f has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Jun 29, 2015

⌛ Testing commit 5c3753f with merge a973e4c...

bors added a commit that referenced this pull request Jun 29, 2015
This branch begins the work of unifying our type checking contexts into a single piece of state. The goal is to eventually have a single context that we can pass around instead of the fractured situation we currently have. There are still several things that must be done before beginning to make tables item local:

- [ ] move FulfillmentContext into InferCtxt
- [ ] modify SelectionContext to only take a single context argument
- [ ] remove remaining typer impls 
- [ ] remove the ClosureTyper + Typer trait
- [ ] do some renaming to make these things more applicable to their new roles

r? @nikomatsakis 

As a side note there are a couple oddities that are temporary refactors that will be quickly cleaned up in a follow-up PR.

cc @eddyb @Aatch @arielb1 @nrc
@bors bors merged commit 5c3753f into rust-lang:master Jun 29, 2015
@jroesch jroesch deleted the infer-ctxt-refactor branch June 29, 2015 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants