Skip to content

/rust

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

Conversation

Reviewers

@nikomatsakis nikomatsakis

Assignees

@pnkfelix pnkfelix

Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@eddyb @rust-highfive @nikomatsakis @bors @pnkfelix
@eddyb
Copy link
Member

eddyb commented Oct 25, 2016 •
edited

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 rust-highfive assigned pnkfelix Oct 25, 2016

@rust-highfive

This comment has been minimized.

Sign in to view
Copy link
Collaborator

rust-highfive commented Oct 25, 2016

r? @pnkfelix

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

@eddyb eddyb referenced this pull request Oct 25, 2016

Merged

[2/n] rustc_metadata: move is_extern_item to trans. #37401

@eddyb eddyb force-pushed the eddyb:lazy-1 branch from c4f4ecd to 0848ae2 Oct 25, 2016

@nikomatsakis

nikomatsakis approved these changes Oct 25, 2016
View changes

src/librustc/ty/context.rs
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>>>,

This comment has been minimized.

Sign in to view
Copy link
@nikomatsakis

nikomatsakis Oct 25, 2016

Contributor

it was probably inevitable...if sad...
src/librustc/ty/mod.rs
let mir = self.alloc_mir(mir);

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

This comment has been minimized.

Sign in to view
Copy link
@nikomatsakis

nikomatsakis Oct 25, 2016

Contributor

heh, cute.
@nikomatsakis

This comment has been minimized.

Sign in to view
Copy link
Contributor

nikomatsakis commented Oct 25, 2016

@bors r+
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Oct 25, 2016

📌 Commit 0848ae2 has been approved by nikomatsakis
@bors

This comment has been minimized.

Sign in to view
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 eddyb force-pushed the eddyb:lazy-1 branch from 0848ae2 to 52d08e2 Oct 28, 2016

@eddyb

This comment has been minimized.

Sign in to view
Copy link
Member Author

eddyb commented Oct 28, 2016

@bors r=nikomatsakis
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Oct 28, 2016

📌 Commit 52d08e2 has been approved by nikomatsakis

@eddyb eddyb force-pushed the eddyb:lazy-1 branch from 52d08e2 to e34792b Oct 28, 2016

@eddyb

This comment has been minimized.

Sign in to view
Copy link
Member Author

eddyb commented Oct 28, 2016

@bors r=nikomatsakis
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Oct 28, 2016

📌 Commit e34792b has been approved by nikomatsakis
@bors

This comment has been minimized.

Sign in to view
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

@bors
Auto merge of #37400 - eddyb:lazy-1, r=nikomatsakis 
[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.
Loading status checks…
ef6f743
@bors

bors added a commit that referenced this pull request Oct 30, 2016

@bors
Auto merge of #37401 - eddyb:lazy-2, r=nikomatsakis 
[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.
Loading status checks…
1238266

@bors bors merged commit e34792b into rust-lang:master Oct 30, 2016
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

This was referenced Oct 30, 2016

Merged
Closed
Merged
Closed
Merged
Merged
Merged

@nnethercote nnethercote referenced this pull request Oct 31, 2016

Merged

Do not clone Mir unnecessarily #37489

@eddyb eddyb referenced this pull request Nov 4, 2016

Open

Look at the output of -Z save-analysis-json #4

@eddyb eddyb referenced this pull request Nov 4, 2016

Merged

Graph Changes #37535

plietar added a commit to plietar/miri that referenced this pull request Nov 4, 2016

@plietar
Fix build for latest nightly. 
The most significant change comes from rust-lang/rust#37400, which
removes the need to maintain our node cache of MIR nodes.
f75d4fb

@eddyb eddyb deleted the eddyb:lazy-1 branch Nov 9, 2016

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.