-
Notifications
You must be signed in to change notification settings - Fork 13.3k
span_to_snippet is broken #8256
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
Comments
@cmr I can't compile that revision of rustdoc_ng with rust HEAD anymore. If you get a chance, can you try to reproduce it? (Don't worry about this too much; it's not high priority anyway.) |
@catamorphism sure, I'll try and get to this tomorrow. |
(The tomorrow that never came) |
…e end of the file. CodeMap.span_to_* perform a lookup of a BytePos(sp.hi), which lands into the next filemap if the last byte of range denoted by Span is also the last byte of the filemap, which results in ICEs or incorrect error reports. Example: ```` pub fn main() { let mut num = 3; let refe = &mut num; *refe = 5; println!("{}", num); }```` (note the empty line in the beginning and the absence of newline at the end) The above would have caused ICE when trying to report where "refe" borrow ends. The above without an empty line in the beginning would have reported borrow end to be the first line. Most probably, this is also responsible for (at least some occurrences of) issue rust-lang#8256. The issue is fixed by always adding a newline at the end of non-empty filemaps in case there isn't a new line there already.
CodeMap.span_to_* perform a lookup of a BytePos(sp.hi), which lands into the next filemap if the last byte of range denoted by Span is also the last byte of the filemap, which results in ICEs or incorrect error reports. Example: ```` pub fn main() { let mut num = 3; let refe = &mut num; *refe = 5; println!("{}", num); }```` (note the empty line in the beginning and the absence of newline at the end) The above would have caused ICE when trying to report where "refe" borrow ends. The above without an empty line in the beginning would have reported borrow end to be the first line. Most probably, this is also responsible for (at least some occurrences of) issue #8256. The issue is fixed by always adding a newline at the end of non-empty filemaps in case there isn't a new line there already.
I think I'm going to change this code to return |
(i encountered this when adding a revision to the syntax extension that implements |
This can be considered partial work on rust-lang#8256. The main observable change: macro expansion sometimes results in spans where `lo > hi`; so for now, when we have such a span, do not attempt to return a snippet result. (Longer term, we might think about whether we could still present a snippet for the cases where this arises, e.g. perhaps by showing the whole macro as the snippet, assuming that is the sole cause of such spans; or by somehow looking up the closest AST node that holds both `lo` and `hi`, and showing that.) As a drive-by, revised the API to return a `Result` rather than an `Option`, with better information-packed error value that should help us (and maybe also our users) identify the causes of such problems in the future. Ideally the call-sites that really want an actual snippet would be updated to catch the newly added `Err` case and print something meaningful about it, but that is not part of this PR.
…t, r=huonw This can be considered partial work on rust-lang#8256. The main observable change: macro expansion sometimes results in spans where `lo > hi`; so for now, when we have such a span, do not attempt to return a snippet result. (Longer term, we might think about whether we could still present a snippet for the cases where this arises, e.g. perhaps by showing the whole macro as the snippet, assuming that is the sole cause of such spans; or by somehow looking up the closest AST node that holds both `lo` and `hi`, and showing that.) As a drive-by, revised the API to return a `Result` rather than an `Option`, with better information-packed error value that should help us (and maybe also our users) identify the causes of such problems in the future. Ideally the call-sites that really want an actual snippet would be updated to catch the newly added `Err` case and print something meaningful about it, but that is not part of this PR.
Triage: it looks like @pnkfelix did some work, but I'm unaware of any further things. Is there a way we can get a reproduction of this issue? |
I feel like this should be closed since reproducing is hard, and I see that there have been some efforts (e.g., #21980, and a few more) to fix this. As such, I'm closing, but please reopen if we want to continue tracking this--preferably with some steps that could be taken to at least reproduce this. |
Passing it a span that I get from rustc fails with:
rust: task failed at 'assertion failed:
(left == right) && (right == left)
(left:{__field__: 9860}
, right:{__field__: 57921}
)', /home/cmr/hacking/rust-benching/src/libsyntax/codemap.rs:376To reproduce: build dd598e1 from cmr/rustdoc_ng, run it on libstd.
@jbclements maybe?
The text was updated successfully, but these errors were encountered: