Provide positional information when visiting ty, substs and closure_substs in MIR

[Nashenas88]

This will enable the region renumbering portion of #43234 (non-lexical lifetimes). @nikomatsakis's current plan here shows that we need spans of the original code to create new region variables, e.g. self.infcx.next_region_var(infer::MiscVariable(span)). The current visitor impls did not pass positional information (Location in some, Span and SourceInfo for others) for all types. I did not expand this to all visits, just the ones necessary for the above-mentioned plan.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @arielb1 (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[Nashenas88]

This is the one part I wasn't sure about. I could have just passed Spans as the third parameter to visit_ty, but this leaves the visit implementations a little flexible for future uses. Also, is this the best place for this enum?

[arielb1]

I'll just pass a Span.

[Nashenas88]

The one travis failure looks like a network issue. It never even finished the git processes.

[arielb1]

r+ with a Span or maybe a SourceInfo passed (with ARGUMENT_VISIBILITY_SCOPE for the return type). We can always go to passing something more complicated if we need to.

[Nashenas88]

Sorry, I made a mistake when I said I could have just passed a Span. I had my current work in mind, where the visitor struct held onto a Mir struct. I need the Mir struct in order to turn the Locations into a Span. The Location struct just holds onto indices for vectors inside the Mir struct. Since the traits don't have this, I can't convert the Locations to Spans without maintaining a copy or the original Mir struct and referencing it in all places that call visit_ty. I also didn't understand your comment about ARGUMENT_VISIBILITY_SCOPE. Where does that come from?

[carols10cents]

ping @arielb1, @Nashenas88 has some questions for you!

[arielb1]

@Nashenas88

Then maybe also pass the SourceInfo around in all visitor methods? Sounds like it would be useful in other cases too (e.g. currently MIR typeck does a hack to regenerate the SourceInfo).

ARGUMENT_VISIBILITY_SCOPE is the scope of argument patterns etc - you only have a span, so you can have the scope be ARGUMENT_VISIBILITY_SCOPE.

[Nashenas88]

@arielb1 I'm not sure I follow. SourceInfo data is stored within nested vectors in a Mir struct. In order to access those, I need the indices within Location. In order to get the SourceInfo in all cases, I need both Mir and Location or just a Span as you mentioned above. I could simplify the enum I have to just Location and SourceInfo given your clarification on ARGUMENT_VISIBILITY_SCOPE (thanks!!), though I'm not sure I could simplify this much further without passing around the Mir struct. That would pose a challenge given the code is already passing around borrows for the items within the mir, and the only way it exposes access to the vectors is with the top-level Mir struct. The vectors inside are private.

[Nashenas88]

@arielb1 would it suffice to reduce the enum to Location and SourceInfo?

[arielb1]

Ok

@bors r+

[bors]

📌 Commit 4b9acad has been approved by arielb1

[bors]

⌛ Testing commit 4b9acad with merge e2b5d7e...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing e2b5d7e to master...