Skip to content

MIR: Add pass that erases all regions right before trans #29886

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

Merged
merged 1 commit into from
Nov 18, 2015

Conversation

michaelwoerister
Copy link
Member

This change adds a MirPass erasing all early-bound regions from MIR, right before storing it in the MIR map. I've added some assertions at neuralgic points in trans::mir doing cheap checks whether region have actually been erased.

Here are some assumptions that I worked under:

  • AdtDef references stay untouched. It's the Substs accompanying them that need to be handled (e.g. in AggregateKind::Adt).
  • We can't really get rid of late-bound regions at this point because there is no version BareFnTy (for example) that comes without one. These still have to be handled on demand in trans.

Are this assumptions right?

r? @nikomatsakis

@nikomatsakis
Copy link
Contributor

@michaelwoerister

AdtDef references stay untouched. It's the Substs accompanying them that need to be handled (e.g. in AggregateKind::Adt).

Yes.

We can't really get rid of late-bound regions at this point because there is no version BareFnTy (for example) that comes without one. These still have to be handled on demand in trans.

Yes.

@nikomatsakis
Copy link
Contributor

Actually, thinking more on this, can we move this pass to later? (Vs doing it in a separate PR) It seems like we can easily modify the driver to pass an &mut mir_map to phase_4_translate_to_llvm (or just plain pass ownership, for that matter), and execute the "region clean" pass there.

@nikomatsakis
Copy link
Contributor

r+ if the nit about closure_substs is addressed and the pass is moved later in the pipeline.

@michaelwoerister
Copy link
Member Author

I've moved region cleaning to phase_4_translate_to_llvm.
Please see the comment on ClosureSubsts.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 18, 2015

📌 Commit 0ce26db has been approved by nikomatsakis

@bors
Copy link
Collaborator

bors commented Nov 18, 2015

⌛ Testing commit 0ce26db with merge 716a00d...

@bors
Copy link
Collaborator

bors commented Nov 18, 2015

💔 Test failed - auto-mac-64-opt

@michaelwoerister
Copy link
Member Author

@bors r=nikomatsakis

@bors
Copy link
Collaborator

bors commented Nov 18, 2015

📌 Commit c533902 has been approved by nikomatsakis

@bors
Copy link
Collaborator

bors commented Nov 18, 2015

⌛ Testing commit c533902 with merge 3c68f64...

bors added a commit that referenced this pull request Nov 18, 2015
…akis

This change adds a `MirPass` erasing all early-bound regions from MIR, right before storing it in the MIR map. I've added some assertions at neuralgic points in `trans::mir` doing cheap checks whether region have actually been erased.

Here are some assumptions that I worked under:
- AdtDef references stay untouched. It's the `Substs` accompanying them that need to be handled (e.g. in `AggregateKind::Adt`).
- We can't really get rid of late-bound regions at this point because there is no version `BareFnTy` (for example) that comes without one. These still have to be handled on demand in trans.

Are this assumptions right?

r? @nikomatsakis
@bors bors merged commit c533902 into rust-lang:master Nov 18, 2015
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.

3 participants