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

[1/n] Move the MIR map into the type context. #37400

Merged
merged 2 commits into from
Oct 30, 2016
Merged

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Oct 25, 2016

This is part of a series (prev | next) of patches designed to rework rustc into an out-of-order on-demand pipeline model for both better feature support (e.g. MIR-based 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.


The first commit reorganizes the rustc::mir module to contain the MIR types directly without an extraneous repr module which serves no practical purpose but is rather an eyesore.

The second commit performs the actual move of the MIR map into the type context, for the purposes of future integration with requesting analysis/lowering by-products through TyCtxt.

Local Mir bodies need to be mutated by passes (hence RefCell), and at least one pass (qualify_consts) needs simultaneous access to multiple Mir bodies (hence arena-allocation).
Mir bodies loaded from other crates appear as if immutably borrowed (by .borrow()-ing one Ref and subsequently "leaking" it) to avoid, at least dynamically, any possibility of their local mutation.

One caveat is that lint passes can now snoop at the MIR (helpful) or even mutate it (dangerous).
However, lints are unstable anyway and we can find a way to deal with this in due time.
Future work will result in a tighter API, potentially hiding mutation completely outside of MIR passes.

@rust-highfive
Copy link
Collaborator

r? @pnkfelix

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

adt_defs: TypedArena<ty::AdtDefData<'tcx, 'tcx>>,
trait_def: TypedArena<ty::TraitDef<'tcx>>,
adt_def: TypedArena<ty::AdtDefData<'tcx, 'tcx>>,
mir: TypedArena<RefCell<Mir<'tcx>>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

it was probably inevitable...if sad...

let mir = self.alloc_mir(mir);

// Perma-borrow MIR from extern crates to prevent mutation.
mem::forget(mir.borrow());
Copy link
Contributor

Choose a reason for hiding this comment

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

heh, cute.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 25, 2016

📌 Commit 0848ae2 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Oct 27, 2016

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

@eddyb
Copy link
Member Author

eddyb commented Oct 28, 2016

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Oct 28, 2016

📌 Commit 52d08e2 has been approved by nikomatsakis

@eddyb
Copy link
Member Author

eddyb commented Oct 28, 2016

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Oct 28, 2016

📌 Commit e34792b has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Oct 30, 2016

⌛ Testing commit e34792b with merge ef6f743...

bors added a commit that referenced this pull request Oct 30, 2016
[1/n] Move the MIR map into the type context.

*This is part of a series ([prev]() | [next](#37401)) 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>

The first commit reorganizes the `rustc::mir` module to contain the MIR types directly without an extraneous `repr` module which serves no practical purpose but is rather an eyesore.

The second commit performs the actual move of the MIR map into the type context, for the purposes of future integration with requesting analysis/lowering by-products through `TyCtxt`.

Local `Mir` bodies need to be mutated by passes (hence `RefCell`), and at least one pass (`qualify_consts`) needs simultaneous access to multiple `Mir` bodies (hence arena-allocation).
`Mir` bodies loaded from other crates appear as if immutably borrowed (by `.borrow()`-ing one `Ref` and subsequently "leaking" it) to avoid, at least dynamically, *any* possibility of their local mutation.

One caveat is that lint passes can now snoop at the MIR (helpful) or even mutate it (dangerous).
However, lints are unstable anyway and we can find a way to deal with this in due time.
Future work will result in a tighter API, potentially hiding mutation *completely* outside of MIR passes.
bors added a commit that referenced this pull request Oct 30, 2016
[2/n] rustc_metadata: move is_extern_item to trans.

*This is part of a series ([prev](#37400) | [next](#37402)) 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>

Minor cleanup missed by #36551: `is_extern_item` is one of, if not the only `CrateStore` method who takes a `TyCtxt` but doesn't produce something cached in it, and such methods are going away.
@bors bors merged commit e34792b into rust-lang:master Oct 30, 2016
@eddyb eddyb mentioned this pull request Nov 4, 2016
plietar added a commit to plietar/miri that referenced this pull request Nov 4, 2016
The most significant change comes from rust-lang/rust#37400, which
removes the need to maintain our node cache of MIR nodes.
@eddyb eddyb deleted the lazy-1 branch November 9, 2016 16:57
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.

5 participants