Skip to content
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

Add ability to get lines/filename for Span in smir #116630

Merged
merged 2 commits into from
Oct 14, 2023

Conversation

ouz-a
Copy link
Contributor

@ouz-a ouz-a commented Oct 11, 2023

Wasn't sure about how to structure lines, so went with safest option, also I'm not sure why span_to_lines returns vec.

Addresses rust-lang/project-stable-mir#44

r? @oli-obk

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 11, 2023
@rustbot
Copy link
Collaborator

rustbot commented Oct 11, 2023

This PR changes Stable MIR

cc @oli-obk, @celinval, @spastorino

This PR changes Stable MIR

cc @oli-obk, @celinval, @spastorino

@oli-obk
Copy link
Contributor

oli-obk commented Oct 11, 2023

@celinval does this format contain what you need?

What are you planning on using these for? If it's just for diagnostics, does the opaque rendered version suffice?


#[derive(Clone, Copy, Debug)]
pub struct LineInfo {
pub line_index: usize,
Copy link
Contributor

Choose a reason for hiding this comment

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

document whether the first line is 0 or whether it is 1. Similarly for col

Copy link
Contributor Author

@ouz-a ouz-a Oct 11, 2023

Choose a reason for hiding this comment

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

made them start from 1 now

@rustbot
Copy link
Collaborator

rustbot commented Oct 11, 2023

This PR changes Stable MIR

cc @oli-obk, @celinval, @spastorino

This PR changes Stable MIR

cc @oli-obk, @celinval, @spastorino

Copy link
Contributor

@celinval celinval left a comment

Choose a reason for hiding this comment

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

Thanks @ouz-a!

)
}

fn get_lines(&self, span: &Span) -> Vec<stable_mir::ty::LineInfo> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest that you return something similar to span_to_location_info.

It basically returns the line + column for beginning and end of the span.

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 modified LineInfo struct so it looks like line,col:beg; line,col;end

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect! Thanks

compiler/rustc_smir/src/rustc_smir/mod.rs Show resolved Hide resolved
compiler/stable_mir/src/lib.rs Show resolved Hide resolved
@celinval
Copy link
Contributor

@oli-obk, we use the output for diagnostic but also in our vscode extension, code coverage and the trace generator. So ideally we would like to be able to map beginning and end of a span to the exact location in a local file.

@oli-obk
Copy link
Contributor

oli-obk commented Oct 13, 2023

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 13, 2023

📌 Commit d6a55d3 has been approved by oli-obk

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 13, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 13, 2023
Add ability to get lines/filename for Span in smir

Wasn't sure about how to structure lines, so went with safest option, also I'm not sure why `span_to_lines` returns `vec`.

Addresses rust-lang/project-stable-mir#44

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 14, 2023
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#116630 (Add ability to get lines/filename for Span in smir)
 - rust-lang#116644 (remove outdated bootstrap FIXME)
 - rust-lang#116695 (Fix a comment)
 - rust-lang#116696 (Misc improvements)
 - rust-lang#116704 (Fix AFIT lint message to mention pitfall)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 14, 2023
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#116630 (Add ability to get lines/filename for Span in smir)
 - rust-lang#116644 (remove outdated bootstrap FIXME)
 - rust-lang#116695 (Fix a comment)
 - rust-lang#116696 (Misc improvements)
 - rust-lang#116704 (Fix AFIT lint message to mention pitfall)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3890ba9 into rust-lang:master Oct 14, 2023
11 checks passed
@rustbot rustbot added this to the 1.75.0 milestone Oct 14, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 14, 2023
Rollup merge of rust-lang#116630 - ouz-a:smir_span_better, r=oli-obk

Add ability to get lines/filename for Span in smir

Wasn't sure about how to structure lines, so went with safest option, also I'm not sure why `span_to_lines` returns `vec`.

Addresses rust-lang/project-stable-mir#44

r? ``@oli-obk``
bors-ferrocene bot added a commit to ferrocene/ferrocene that referenced this pull request Oct 17, 2023
54: Pull upstream master 2023 10 17 r=pietroalbini a=Veykril

* rust-lang/rust#116196
* rust-lang/rust#116824
* rust-lang/rust#116822
* rust-lang/rust#116477
* rust-lang/rust#116826
* rust-lang/rust#116820
  * rust-lang/rust#116811
  * rust-lang/rust#116808
  * rust-lang/rust#116805
  * rust-lang/rust#116800
  * rust-lang/rust#116798
  * rust-lang/rust#116754
* rust-lang/rust#114370
* rust-lang/rust#116804
  * rust-lang/rust#116802
  * rust-lang/rust#116790
  * rust-lang/rust#116786
  * rust-lang/rust#116709
  * rust-lang/rust#116430
  * rust-lang/rust#116257
  * rust-lang/rust#114157
* rust-lang/rust#116731
* rust-lang/rust#116550
* rust-lang/rust#114330
* rust-lang/rust#116724
* rust-lang/rust#116782
  * rust-lang/rust#116776
  * rust-lang/rust#115955
  * rust-lang/rust#115196
* rust-lang/rust#116775
* rust-lang/rust#114589
* rust-lang/rust#113747
* rust-lang/rust#116772
  * rust-lang/rust#116771
  * rust-lang/rust#116760
  * rust-lang/rust#116755
  * rust-lang/rust#116732
  * rust-lang/rust#116522
  * rust-lang/rust#116341
  * rust-lang/rust#116172
* rust-lang/rust#110604
* rust-lang/rust#110729
* rust-lang/rust#116527
* rust-lang/rust#116688
* rust-lang/rust#116757
  * rust-lang/rust#116753
  * rust-lang/rust#116748
  * rust-lang/rust#116741
  * rust-lang/rust#116594
* rust-lang/rust#116691
* rust-lang/rust#116643
* rust-lang/rust#116683
* rust-lang/rust#116635
* rust-lang/rust#115515
* rust-lang/rust#116742
  * rust-lang/rust#116661
  * rust-lang/rust#116576
  * rust-lang/rust#116540
* rust-lang/rust#116352
* rust-lang/rust#116737
  * rust-lang/rust#116730
  * rust-lang/rust#116723
  * rust-lang/rust#116715
  * rust-lang/rust#116603
  * rust-lang/rust#116591
  * rust-lang/rust#115439
* rust-lang/rust#116264
* rust-lang/rust#116727
  * rust-lang/rust#116704
  * rust-lang/rust#116696
  * rust-lang/rust#116695
  * rust-lang/rust#116644
  * rust-lang/rust#116630
* rust-lang/rust#116728
  * rust-lang/rust#116689
  * rust-lang/rust#116679
  * rust-lang/rust#116618
  * rust-lang/rust#116577
  * rust-lang/rust#115653
* rust-lang/rust#116702
* rust-lang/rust#116015
* rust-lang/rust#115822
* rust-lang/rust#116407
* rust-lang/rust#115719
* rust-lang/rust#115524
* rust-lang/rust#116705
* rust-lang/rust#116645
* rust-lang/rust#116233
* rust-lang/rust#115108
* rust-lang/rust#116670
* rust-lang/rust#116676
* rust-lang/rust#116666



Co-authored-by: Benoît du Garreau <bdgdlm@outlook.com>
Co-authored-by: Colin Finck <colin@reactos.org>
Co-authored-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
Co-authored-by: Joshua Liebow-Feeser <joshlf@users.noreply.github.com>
Co-authored-by: León Orell Valerian Liehr <me@fmease.dev>
Co-authored-by: Trevor Gross <tmgross@umich.edu>
Co-authored-by: Evan Merlock <evan@merlock.dev>
Co-authored-by: joboet <jonasboettiger@icloud.com>
Co-authored-by: Ralf Jung <post@ralfj.de>
Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
Co-authored-by: Mark Rousskov <mark.simulacrum@gmail.com>
Co-authored-by: onur-ozkan <work@onurozkan.dev>
Co-authored-by: Nicholas Nethercote <n.nethercote@gmail.com>
Co-authored-by: The 8472 <git@infinite-source.de>
Co-authored-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Co-authored-by: reez12g <reez12g@gmail.com>
Co-authored-by: Jakub Beránek <berykubik@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants