Conversation
There was a problem hiding this comment.
this will need to a reference to the MIR
There was a problem hiding this comment.
To obtain the successor points, we will need to look at the Location. We can load the basic block information from the MIR via basic_blocks()[location.block]. Each block has a BasicBlockData.
If we compare the location.statement_index and see that it less than block_data.statements.len(), then there is just one successor: Location { statement_index: location.statement_index + 1, ..location }.
Otherwise, location.statement_index should be equal to block_data.statements.len(), which indicates that this location is the "terminator" -- that is, the final point in the block. In that case, the successors can be found by invoking block_data.terminator().successors() to get a list of successor blocks. Those can be converted into Location by pairing the BasicBlock with a statement index of 0 (first statement in the block.
a147e24 to
4f21ffa
Compare
There was a problem hiding this comment.
@nikomatsakis this is the only part I can't figure out. It seems we lose track of the free regions when we convert them all to region variables. See mod.rs line 49:
self.infcx.tcx.fold_regions(value, &mut false, |_region, _depth| {
self.regions.push(Region::default());
self.infcx.next_region_var(rustc_infer::MiscVariable(DUMMY_SP))
})where _region might be RegionKind::ReFree(FreeRegion). Also, I'm not sure if I can even use free_region_map like I have here at all. This was how (I think) it worked the last time I tried to get this information (many, many months ago), but it seems to have changed a lot since then. Is there a way to query this information from tcx or should we keep track of which regions are free regions in mod.rs and use that information here?
There was a problem hiding this comment.
Yeah, so, I was expecting us to not handle free regions yet. We are going to have to figure out (eventually) the set of free-regions in scope for a given MIR block -- though I do not think we will want to modify that particular code in question. That is, the fact that the old region inferencing code wound up assigning a free region is not of particular interest.
We will however want to look at the public "signature" of the function, since that is defined by the user. Well, modulo closures, where inference plays a role: doing a better job of handling closures is something we'll have to think about too.
There was a problem hiding this comment.
TL;DR I think i'd just leave this section as a "TODO" for now
There was a problem hiding this comment.
I'll move this into a function so it's a little cleaner.
There was a problem hiding this comment.
Creating a newtype index doesn't seem worth it to me here -- I think making a newtype'd index is only worth it if we are passing around these indices a lot. I don't envision us doing anything here but iterating through the errors, right?
There was a problem hiding this comment.
Probably we should include the location information here -- i.e., where this variable appears in the MIR.
nikomatsakis
left a comment
There was a problem hiding this comment.
Seems like a good start thus far.
|
@spastorino note the various tidy errors here |
|
☔ The latest upstream changes (presumably #45013) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@bors r+ |
|
📋 Looks like this PR is still in progress, ignoring approval |
|
@bors r+ |
|
📌 Commit 9603e24 has been approved by |
|
☀️ Test successful - status-appveyor, status-travis |
r? @nikomatsakis