long literal expressions should be truncated #125581
Comments
lolbinarycat
added
A-diagnostics
Yeah that's unacceptable, shoving a string behind an |
workingjubilee
added
the
A-macros
|
@lolbinarycat wait, apparently this doesn't happen with can you produce an example that produces the bad diagnostics you describe with |
|
longconst1.rs: const FOO: &[&str] = include!("longconst2.rs");longconst2.rs ["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"]then do |
|
here's the terminal output. surprisingly alacritty actually doesn't turn the line wraps into hard newlines when you copy the selection. |
|
it looks like the syntax error actually shows as being inside the included file, which makes sense. i couldn't tell before because it was cut off. |
|
...okay there's... still some kinda diagnostic bug here but I don't know what lol. |
|
like I would expect this suggestion: help: consider borrowing here
|
1 | const FOO: &[&str] = &include!("longconst2.rs");
| + |
|
it's grabbing the source position of the it's impossible for the suggestions to be 100% accurate, what i think is more important is making sure the programmer can see the details of the error. |
|
in case it wasn't obvious why the current behavior is a problem, here's a more accurate representation of what you would see in your terminal. and remember, this is a mild example. it can get so much worse. very long error message |
|
I did take a look at the error in my actual terminal and honestly it didn't look that bad (i.e. that was not really an "accurate representation of what you would see in your terminal", for all terminals anyways, and "more" feels debatable?). Force-wrapping your terminal to the width of a punch card and then including 420-character strings in your source is an... interesting combination of decisions. |
|
I was curious how this is handled for multiple line errors and it does indeed partially elide these cases: Even when source describes a single literal: But there the linebreaks provide the natural, well, breaking points, so the heuristic of when to start "horizontal" elision seems like it will be more complicated (due to the aforementioned variable terminal width). |
|
Ah, right, rust/compiler/rustc_errors/src/emitter.rs Line 41 in cdc509f There's... still quite a few heuristics indeed. Hmm, we do seem to have partial support for horizontal elision, but only on the start of a line, if I'm reading this correctly...? |
|
hmm, i just realized the diagnostics code doesn't recive an AST.. that does make things quite a bit trickier. |
|
Well, it looks at spans. This has benefits, in that it prevents losing track of where in the actual code (and not the abstractifications of it) the error occurs. rustc's errors are often surfaced from deep inside borrow-checking or const-eval, so remembering spans all the way through helps prevent looking at fake code that doesn't exist, or pointing at the wrong thing. You're right that sometimes looking at the abstractification is kinda what we want. I don't think it makes resolving this issue impossible, though. It just might make the implementation... funny. More likely to induce grief are the includes, I think: rust/compiler/rustc_builtin_macros/src/source_util.rs Lines 105 to 182 in cdc509f |
|
there's also a midground solution to consider: if a line is extremely long (say, a lot of older compilers only print souce code locations, so i don't think it would be so bad, and it would definitely be an improvement over the current situation, since you could actually see the errors. only tricky part might be getting the "help" diagnostics to print a source code location where you could change something. |
|
Hmm, my experience with learning Rust is I had no idea what the file:line:column things meant. |
That is correct, we trim the beginning and end of long lines, but we don't even attempt to cut the middle of long lines. I didn't implement that originally because 1) it would have been too much logic to land at once, 2) I wasn't sure what the heuristic should have been (note that this logic could also trigger with multiple long expressions on the same line), 3) was concerned about mucking around too much with the code as written by end users. The machinery for keeping track of this is already all there: you need to account for multibyte chars (to avoid splitting one of them when cutting the code string up), to keep track of the trimmings so the underline keeps working, to write a very clear Edit: One more thing, I think that whatever is done for this should be done on the
It's not a bad idea, but I don't think it is the best course of action. We already mangle users' code a bit for long lines, we should either lean into that, or remove what we have already and use that new solution for those cases too. I lean towards leaning in. Also note that we currently don't apply this logic for suggestions, because I deemed likely to confuse people, but maybe I was wrong and we should also trim suggestions as well. That might require additional impl work. |
estebank
added
the
D-verbose
Hmmm. While the emitter reasons about things in terms of chars, as it should, I think we should prefer to elide entire tokens at a time. That's also justified by only doing this if the code runs wildly "off the page"... if someone overhangs by a few dozen bytes, that would still render "normally", but if we determine the token is effectively useless to the output then we should discard it. |
|
there's always the option of discarding the line entirely, which might be good enough considering not many people will have lines 1000s of chars long. or, alternatively, we could discard the ends of very long lines, as well as their beginning. this way you can still have the alignment-based help messages, while not flooding the screen, and also without significant increases to code complexity. |
I'm not sure I agree for two reasons. One is stylistic: providing partial context is useful to help identify what we might be pointing at. If you have
Agree.
It might be that the span covers more than a single token. It could be quite a complex expression that might be hard to find a natural cutting place for without AST analysis. I think that the "show a bit of the start and a bit of the end" has worked well enough as a heuristic in the past, and we should use it here too.
We do, the problem is for long spans, not long lines in general: |
in that case i think it would make sense to keep to the behavior of only omitting at the start and the end. otherwise diagnostics could end up with an ellipsis in 3 places (such as if it consists of a short span, a very long span, then another short span), and that seems needlessly confusing. i propose a simple change: if, after omitting all leading and trailing spans, the line is still extremely long, the end of the target span should also be omitted (only enough to make the line a reasonable length). |
…al width
When encountering a single line span that is wider than the terminal, we keep context at the start and end of the span but otherwise remove the code from the middle. This is somewhat independent from whether the left and right margins of the output have been trimmed as well.
```
error[E0308]: mismatched types
--> $DIR/long-span.rs:6:15
|
LL | ... = [0, 0, 0, 0, ..., 0, 0];
| ^^^^^^^^^^^^^...^^^^^^^ expected `u8`, found `[{integer}; 1681]`
```
Address part of rust-lang#137680 (missing handling of the long suggestion). Fix rust-lang#125581.
…al width
When encountering a single line span that is wider than the terminal, we keep context at the start and end of the span but otherwise remove the code from the middle. This is somewhat independent from whether the left and right margins of the output have been trimmed as well.
```
error[E0308]: mismatched types
--> $DIR/long-span.rs:6:15
|
LL | ... = [0, 0, 0, 0, ..., 0, 0];
| ^^^^^^^^^^^^^...^^^^^^^ expected `u8`, found `[{integer}; 1681]`
```
Address part of rust-lang#137680 (missing handling of the long suggestion). Fix rust-lang#125581.
|
I think that #137757 will address everything but the suggestions.
|
On long spans, trim the middle of them to make them fit in the terminal width
When encountering a single line span that is wider than the terminal, we keep context at the start and end of the span but otherwise remove the code from the middle. This is somewhat independent from whether the left and right margins of the output have been trimmed as well.
```
error[E0308]: mismatched types
--> $DIR/long-span.rs:6:15
|
LL | ... = [0, 0, 0, 0, ..., 0, 0];
| ^^^^^^^^^^^^^...^^^^^^^ expected `u8`, found `[{integer}; 1681]`
```
Address part of rust-lang#137680 (missing handling of the long suggestion). Fix rust-lang#125581.
---
Change the way that underline positions are calculated by delaying using the "visual" column position until the last possible moment, instead using the "file"/byte position in the file, and then calculating visual positioning as late as possible. This should make the underlines more resilient to non-1-width unicode chars.
Unfortunately, as part of this change (which fixes some visual bugs) comes with the loss of some eager tab codepoint handling, but the output remains legible despite some minor regression on the "margin trimming" logic.
---
`-Zteach` is perma-unstable, barely used, the highlighting logic buggy and the flag being passed around is tech-debt. We should likely remove `-Zteach` in its entirely.
On long spans, trim the middle of them to make them fit in the terminal width
When encountering a single line span that is wider than the terminal, we keep context at the start and end of the span but otherwise remove the code from the middle. This is somewhat independent from whether the left and right margins of the output have been trimmed as well.
```
error[E0308]: mismatched types
--> $DIR/long-span.rs:6:15
|
LL | ... = [0, 0, 0, 0, ..., 0, 0];
| ^^^^^^^^^^^^^...^^^^^^^ expected `u8`, found `[{integer}; 1681]`
```
Address part of rust-lang#137680 (missing handling of the long suggestion). Fix rust-lang#125581.
---
Change the way that underline positions are calculated by delaying using the "visual" column position until the last possible moment, instead using the "file"/byte position in the file, and then calculating visual positioning as late as possible. This should make the underlines more resilient to non-1-width unicode chars.
Unfortunately, as part of this change (which fixes some visual bugs) comes with the loss of some eager tab codepoint handling, but the output remains legible despite some minor regression on the "margin trimming" logic.
---
`-Zteach` is perma-unstable, barely used, the highlighting logic buggy and the flag being passed around is tech-debt. We should likely remove `-Zteach` in its entirely.
Rollup merge of rust-lang#137757 - estebank:trim-spans, r=davidtwco On long spans, trim the middle of them to make them fit in the terminal width When encountering a single line span that is wider than the terminal, we keep context at the start and end of the span but otherwise remove the code from the middle. This is somewhat independent from whether the left and right margins of the output have been trimmed as well. ``` error[E0308]: mismatched types --> $DIR/long-span.rs:6:15 | LL | ... = [0, 0, 0, 0, ..., 0, 0]; | ^^^^^^^^^^^^^...^^^^^^^ expected `u8`, found `[{integer}; 1681]` ``` Address part of rust-lang#137680 (missing handling of the long suggestion). Fix rust-lang#125581. --- Change the way that underline positions are calculated by delaying using the "visual" column position until the last possible moment, instead using the "file"/byte position in the file, and then calculating visual positioning as late as possible. This should make the underlines more resilient to non-1-width unicode chars. Unfortunately, as part of this change (which fixes some visual bugs) comes with the loss of some eager tab codepoint handling, but the output remains legible despite some minor regression on the "margin trimming" logic. --- `-Zteach` is perma-unstable, barely used, the highlighting logic buggy and the flag being passed around is tech-debt. We should likely remove `-Zteach` in its entirely.
Code
Current output
Desired output
Rationale and extra context
because of the terminal line wrapping several times above and below the error message, it can be very difficult to find the actual details of the error (in this case, the expected and actual types)
this gets much worse if you are trying to include a generated constant expression into your code, as the
include!will actually be expanded in the error message, meaning rustc might bury the important details of the error between hundreds or thousands of lines of irrelevant autogenerated code.Other cases
No response
Rust Version
Anything else?
No response
The text was updated successfully, but these errors were encountered: