Skip to content

Non-lexical lifetimes region renumberer #43559

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 4 commits into from
Aug 11, 2017
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 88 additions & 13 deletions src/librustc_mir/transform/nll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,91 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use rustc::ty::TyCtxt;
use rustc::mir::Mir;
use rustc::mir::visit::MutVisitor;
use rustc::ty::TypeFoldable;
use rustc::ty::subst::Substs;
use rustc::ty::{Ty, TyCtxt, ClosureSubsts};
use rustc::mir::{Mir, Location, Rvalue, BasicBlock, Statement, StatementKind};
use rustc::mir::visit::{MutVisitor, Lookup};
use rustc::mir::transform::{MirPass, MirSource};
use rustc::infer::{self, InferCtxt};
use syntax_pos::Span;

#[allow(dead_code)]
struct NLLVisitor<'a, 'tcx: 'a> {
tcx: TyCtxt<'a, 'tcx, 'tcx>,
struct NLLVisitor<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> {
infcx: InferCtxt<'a, 'gcx, 'tcx>,
source: &'a Mir<'tcx>
}

impl<'a, 'tcx> NLLVisitor<'a, 'tcx> {
pub fn new(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Self {
impl<'a, 'gcx, 'tcx> NLLVisitor<'a, 'gcx, 'tcx> {
pub fn new(infcx: InferCtxt<'a, 'gcx, 'tcx>, source: &'a Mir<'tcx>) -> Self {
NLLVisitor {
tcx: tcx
infcx: infcx,
source: source,
}
}

fn renumber_regions<T>(&self, value: &T, span: Span) -> T where T: TypeFoldable<'tcx> {
self.infcx.tcx.fold_regions(value, &mut false, |_region, _depth| {
self.infcx.next_region_var(infer::MiscVariable(span))
})
}
}

impl<'a, 'tcx> MutVisitor<'tcx> for NLLVisitor<'a, 'tcx> {
// FIXME: Nashenas88: implement me!
fn span_from_location<'tcx>(source: &Mir<'tcx>, location: Location) -> Span {
source[location.block].statements[location.statement_index].source_info.span
}

impl<'a, 'gcx, 'tcx> MutVisitor<'tcx> for NLLVisitor<'a, 'gcx, 'tcx> {
fn visit_ty(&mut self, ty: &mut Ty<'tcx>, lookup: Lookup) {
let old_ty = *ty;
let span = match lookup {
Lookup::Loc(location) => span_from_location(self.source, location),
Lookup::Src(source_info) => source_info.span,
};
*ty = self.renumber_regions(&old_ty, span);
}

fn visit_substs(&mut self, substs: &mut &'tcx Substs<'tcx>, location: Location) {
*substs = self.renumber_regions(&{*substs}, span_from_location(self.source, location));
}

fn visit_rvalue(&mut self, rvalue: &mut Rvalue<'tcx>, location: Location) {
match *rvalue {
Rvalue::Ref(ref mut r, _, _) => {
let old_r = *r;
*r = self.renumber_regions(&old_r, span_from_location(self.source, location));
}
Rvalue::Use(..) |
Rvalue::Repeat(..) |
Rvalue::Len(..) |
Rvalue::Cast(..) |
Rvalue::BinaryOp(..) |
Rvalue::CheckedBinaryOp(..) |
Rvalue::UnaryOp(..) |
Rvalue::Discriminant(..) |
Rvalue::NullaryOp(..) |
Rvalue::Aggregate(..) => {
// These variants don't contain regions.
}
}
self.super_rvalue(rvalue, location);
}

fn visit_closure_substs(&mut self,
substs: &mut ClosureSubsts<'tcx>,
location: Location) {
*substs = self.renumber_regions(substs, span_from_location(self.source, location));
}

fn visit_statement(&mut self,
block: BasicBlock,
statement: &mut Statement<'tcx>,
location: Location) {
if let StatementKind::EndRegion(_) = statement.kind {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure if this should also be used from the erase_regions pass. Should the be changed, left as is or just removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

EndRegion is not going to be relevant with NLL, so I think this is OK

statement.kind = StatementKind::Nop;
}
self.super_statement(block, statement, location);
}
}

// MIR Pass for non-lexical lifetimes
Expand All @@ -38,10 +103,20 @@ impl MirPass for NLL {
tcx: TyCtxt<'a, 'tcx, 'tcx>,
_: MirSource,
mir: &mut Mir<'tcx>) {
if tcx.sess.opts.debugging_opts.nll {
if !tcx.sess.opts.debugging_opts.nll {
return;
}

tcx.infer_ctxt().enter(|infcx| {
// Clone mir so we can mutate it without disturbing the rest
// of the compiler
NLLVisitor::new(tcx).visit_mir(&mut mir.clone());
}
let mut renumbered_mir = mir.clone();

// Note that we're using the passed-in mir for the visitor. This is
Copy link
Contributor

Choose a reason for hiding this comment

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

That's ugly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Heh, it's either ugly... or genius. I'm not sure which. In any case, if it's only being used for the "origin data" of region variables, we could probably sidestep the question for a bit. For example, instead of using the proper span, we can use dummy-span, and track (for each region variable) the location. Or we can make a RegionVariableOrigin that includes the Location instead.

I'm sort of inclined to do the former -- just sidestep the existing inference infrastructure, and just try to remember, for each region variable index, what the location was for later.

Is there a reason that's a bad idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@arielb1 I agree it's ugly 😄 . That's why I added the comment so it could draw attention. The only part I thought worked well is that I avoid any additional copies of the Mir or redesign of its struct. Though the latter might have been (could be?) the better route at some point. I definitely think the existing version is hacky, though.

@nikomatsakis, when we plan to read these locations in future implementations, do you imagine that we'd need to have the Mir mutable as it is in this case? This is what caused me trouble, as I couldn't think of how to give the visitor structure the mutable Mir it needed while also referencing into other parts of the Mir immutably. If we only need the Mir immutable after the visitor pass (or at least when we try to lookup the Locations in the Mir), then I don't see any issue with what you proposed. Just a map from region variable index to Location/SourceInfo, right? (The SourceInfo is because not all Ty regions provide a Location.)

Copy link
Contributor

Choose a reason for hiding this comment

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

@Nashenas88 I do not expect us to be mutating the MIR as we perform the region inference -- it's more just using the MIR as an input.

// so we can lookup locations during traversal without worrying about
// maintaing both a mutable and immutable reference to the same object
let mut visitor = NLLVisitor::new(infcx, &mir);
visitor.visit_mir(&mut renumbered_mir);
})
}
}