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.

Sign up for GitHub

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

Jump to bottom

Add support to span #31

Closed
celinval opened this issue Sep 1, 2023 · 7 comments
Closed

Add support to span #31

celinval opened this issue Sep 1, 2023 · 7 comments
Assignees
@ouz-a

Comments

@celinval
Copy link
Contributor

celinval commented Sep 1, 2023 •
edited by rustbot

Users should be able to retrieve the code span of SMIR items.
@ouz-a
Copy link

ouz-a commented Sep 4, 2023

@rustbot claim

@oli-obk
Copy link
Contributor

oli-obk commented Sep 4, 2023

how should we expose it. If we expose it directly (have PathBuf, line/col info and expansion info directly in the smir::Span struct), that may get expensive. If we expose it indirectly, we can do it like .kind() on smir::Ty, computing it only when requested and otherwise just storing an index.

@celinval
Copy link
Contributor Author

celinval commented Sep 4, 2023

We should encapsulate the path, so no one relies on having direct access to them. We probably want to intern the pathbuf for the file, so we don't keep replicating it.

@ouz-a
Copy link

ouz-a commented Sep 9, 2023

Should we return rustc_span::Span or stable_mir::Span, agree with oli here exposing indirectly will suffice for most needs.

@ouz-a ouz-a mentioned this issue Sep 9, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 9, 2023
@bors
Auto merge of rust-lang#115703 - ouz-a:smir_span, r=oli-obk
c1b38c7 
Add function that returns span of an item in smir

Addressees rust-lang/project-stable-mir#31

Maybe we should change `Span = Opaque` into something else, and then return `String` with newly added function, I don't think it matters that much though, since we are not storing `Span` anywhere.

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 9, 2023
@bors
Auto merge of rust-lang#115703 - ouz-a:smir_span, r=oli-obk
8ed4537 
Add function that returns span of an item in smir

Addressees rust-lang/project-stable-mir#31

Maybe we should change `Span = Opaque` into something else, and then return `String` with newly added function, I don't think it matters that much though, since we are not storing `Span` anywhere.

r? `@oli-obk`
@ouz-a
Copy link

ouz-a commented Sep 11, 2023

can we close this now ?

@oli-obk
Copy link
Contributor

oli-obk commented Sep 11, 2023

Right now the spans are opaque and thus not very useful. We should add a way to retrieve file, line, col and expansion information.

@ouz-a ouz-a mentioned this issue Sep 12, 2023
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 14, 2023
@matthiaskrgr
Rollup merge of rust-lang#115772 - ouz-a:smir_span2, r=oli-obk
134e3a7 
Improve Span in smir

Addressing rust-lang/project-stable-mir#31

r? `@oli-obk`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 14, 2023
@matthiaskrgr
Rollup merge of rust-lang#115772 - ouz-a:smir_span2, r=oli-obk
4f90a52 
Improve Span in smir

Addressing rust-lang/project-stable-mir#31

r? ``@oli-obk``
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 14, 2023
@rust-timer
Unrolled build for rust-lang#115772
26ce33b 
Rollup merge of rust-lang#115772 - ouz-a:smir_span2, r=oli-obk

Improve Span in smir

Addressing rust-lang/project-stable-mir#31

r? ``@oli-obk``
@celinval
Copy link
Contributor Author

I believe we can close this now, right? Feel free to reopen if there's more work to be done here. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ouz-a ouz-a

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@oli-obk @celinval @ouz-a