NLL infer #45155
NLL infer #45155
Conversation
| } | ||
|
|
||
| struct Dfs<'a, 'gcx: 'tcx + 'a, 'tcx: 'a> { | ||
| infcx: InferCtxt<'a, 'gcx, 'tcx>, |
There was a problem hiding this comment.
this will need to a reference to the MIR
|
|
||
| changed |= to_region.add_point(p); | ||
|
|
||
| let successor_points = self.infcx.tcx.successor_points(p); |
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.
Nashenas88
force-pushed
the
nll-infer
branch
2 times, most recently
from
a147e24
to
4f21ffa
Compare
| // If we reach the END point in the graph, then copy | ||
| // over any skolemized end points in the `from_region` | ||
| // and make sure they are included in the `to_region`. | ||
| for region_decl in self.infcx.tcx.tables.borrow().free_region_map() { |
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
| changed |= to_region.add_point(p); | ||
|
|
||
| let block_data = self.mir[p.block]; | ||
| let successor_points = if p.statement_index < block_data.statements.len() { |
There was a problem hiding this comment.
I'll move this into a function so it's a little cleaner.
| pub name: (), // TODO(nashenas88) RegionName | ||
| } | ||
|
|
||
| newtype_index!(InferenceErrorIndex); |
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?
| name: (), // TODO(nashenas88) RegionName | ||
| value: Region, | ||
| capped: bool, | ||
| } |
There was a problem hiding this comment.
Probably we should include the location information here -- i.e., where this variable appears in the MIR.
|
@spastorino note the various tidy errors here |
aidanhs
added
the
S-waiting-on-review
|
☔ 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 |
kennytm
added
S-waiting-on-bors
|
☀️ Test successful - status-appveyor, status-travis |
r? @nikomatsakis