Add an ability to convert between Span and visit::Location

[artemagvanian]

AFAIK, there is no way to create a Location from a Span because its only field is private. This makes it impossible to use visitor methods like visit_statement or visit_terminator.

This PR adds an implementation forFrom<Span> for Location to fix this.

r? @celinval

[rustbot]

This PR changes Stable MIR

cc @oli-obk, @celinval, @ouz-a

[celinval]

Sorry for the delay. I'm not sure we want to necessary convert Span to Location. Just to give a bit of background, the Location was basically added as a placeholder for us to expand later on based on the emerging use cases. So we decided to keep things opaque.

Span is not as precise as we may want the location to be.

The compiler MIR Visitor, for example, is a combination of the index of the basic block and the index of the instruction within the basic block [1].

I would be more comfortable creating an interface that allows you to create a Location, such as:

pub fn new(body: &Body, bb: BasicBlockIdx, idx: usize) -> Location {}

Maybe you can provide a bit more background on your use case and see if that would work for you.

[artemagvanian]

Thanks for the detailed explanation!

I am working on an MIR transformation in Stable MIR, which could be significantly simplified if we traversed the basic block in the reverse direction. However, the only option to achieve that seems to be to implement visit_statement and visit_terminator methods and then call them manually while iterating over the statements in reverse. Not being able to create Location from Span or otherwise prevents calling those methods directly.

Currently, we were able to circumvent this by visiting the basic block to collect instructions that need instrumentation and then applying the instrumentation in reverse order. However, it seems that being able to call visit_statement and visit_terminator directly is something worth implementing at some point.

[celinval]

Would my suggestion work in this case? Maybe we can add a few methods:

/// Location of the first instruction of the BB
fn basic_block_location(body: &Body, bb: &BasicBlockIdx);
/// Location of the statement at the given index
fn statement_location(body: &Body, bb: &BasicBlockIdx, stmt_indx: usize);
/// Location of the terminator 
fn terminator_location(body: &Body, bb: &BasicBlockIdx);

Feel free to tweak with the idea / naming.

[artemagvanian]

I think it would. I believe Location is only well-defined for the latter two methods as of now. I will make the changes to the PR.

[celinval]

Awesome! Thanks

@bors r+ rollup

[bors]

📌 Commit 515f5ac has been approved by celinval

It is now in the queue for this repository.