Use more slice patterns inside the compiler
Nothing super noteworthy. Just replacing the common 'fragile' pattern of "length check followed by indexing or unwrap" with slice patterns for legibility and 'robustness'.
r? ghost
use stable sort to sort multipart diagnostics
I think a stable sort should be used to sort the different parts of a multipart selection. The current unstable sort uses the text of the suggestion as a tie-breaker. That just doesn't seem right, and the order of the input is a better choice I think, because it gives the diagnostic author more control.
This came up when I was building a suggestion where
```rust
fn foo() {}
```
must be turned into an unsafe function, and an attribute must be added
```rust
#[target_feature(enable = "...")]
unsafe fn foo() {}
```
In this example, the two suggestions occur at the same position, but the order is extremely important: unsafe must come after the attribute. But the situation changes if there is a pub/pub(crate), and if the unsafe is already present. It just out that because of the suggestion text, there is no way for me to order the suggestions correctly.
This change probably should be tested, but are there tests of the diagnostics code itself in the tests?
r? ```@estebank```
Some `const { }` asserts for #128200
The correctness of code in #128200 relies on an array being sorted (so that it can be used in binary search later), which is currently enforced with `// tidy-alphabetical` (and characters being written in `\u{XXXX}` form), as well as lack of duplicate entries with conflicting keys, which is not currently enforced.
This PR changes it to using a `const{ }` assertion (and also checks for duplicate entries). Sadly, we cannot use the recently-stabilized `is_sorted_by_key` here, because it is not const (but it would not allow us to check for uniqueness anyways). Instead, let's write a manual loop.
Alternative approach (perfect hash function): #128463
r? `@ghost`
On short error format, append primary span label to message
The `error-format=short` output only displays the path, error code and main error message all in the same line. We now add the primary span label as well after the error message, to provide more context.
The `error-format=short` output only displays the path, error code and
main error message all in the same line. We now add the primary span label
as well after the error message, to provide more context.
Change output normalization logic to be linear against size of output
Modify the rendered output normalization routine to scan each character *once* and construct a `String` to be printed out to the terminal *once*, instead of using `String::replace` in a loop multiple times. The output doesn't change, but the time spent to prepare a diagnostic is now faster (or rather, closer to what it was before #127528).
When a suggestion part is for already present code, do not highlight it. If after that there are no highlights left, do not show the suggestion at all.
Fix clippy lint suggestion incorrectly treated as `span_help`.
Scan strings to be normalized for printing in a linear scan and collect
the resulting `String` only once.
Use a binary search when looking for chars to be replaced, instead of a
`HashMap::get`.
Replace ASCII control chars with Unicode Control Pictures
Replace ASCII control chars like `CR` with Unicode Control Pictures like `␍`:
```
error: bare CR not allowed in doc-comment
--> $DIR/lex-bare-cr-string-literal-doc-comment.rs:3:32
|
LL | /// doc comment with bare CR: '␍'
| ^
```
Centralize the checking of unicode char width for the purposes of CLI display in one place. Account for the new replacements. Remove unneeded tracking of "zero-width" unicode chars, as we calculate these in the `SourceMap` as needed now.
We already point these out quite aggressively, telling people not to use them, but would normally be rendered as nothing. Having them visible will make it easier for people to actually deal with them.
```
error: unicode codepoint changing visible direction of text present in literal
--> $DIR/unicode-control-codepoints.rs:26:22
|
LL | println!("{:?}", '�');
| ^-^
| ||
| |'\u{202e}'
| this literal contains an invisible unicode text flow control codepoint
|
= note: these kind of unicode codepoints change the way text flows on applications that support them, but can cause confusion because they change the order of characters on the screen
= help: if their presence wasn't intentional, you can remove them
help: if you want to keep them but make them visible in your source code, you can escape them
|
LL | println!("{:?}", '\u{202e}');
| ~~~~~~~~
```
vs the previous
```
error: unicode codepoint changing visible direction of text present in literal
--> $DIR/unicode-control-codepoints.rs:26:22
|
LL | println!("{:?}", '');
| ^-
| ||
| |'\u{202e}'
| this literal contains an invisible unicode text flow control codepoint
|
= note: these kind of unicode codepoints change the way text flows on applications that support them, but can cause confusion because they change the order of characters on the screen
= help: if their presence wasn't intentional, you can remove them
help: if you want to keep them but make them visible in your source code, you can escape them
|
LL | println!("{:?}", '\u{202e}');
| ~~~~~~~~
```
No longer track "zero-width" chars in `SourceMap`, read directly from the line when calculating the `display_col` of a `BytePos`. Move `char_width` to `rustc_span` and use it from the emitter.
This change allows the following to properly align in terminals (depending on the font, the replaced control codepoints are rendered as 1 or 2 width, on my terminal they are rendered as 1, on VSCode text they are rendered as 2):
```
error: this file contains an unclosed delimiter
--> $DIR/issue-68629.rs:5:17
|
LL | ␜␟ts␀)
2. Principal trait's lifetime arguments match (no `*const dyn Tr<'a>` -> `*const dyn Tr<'b>` casts, which are a problem for [#120217](https://github.com/rust-lang/rust/issues/120217))
3. No auto traits can be _added_ (this is a problem for arbitrary self types, see [this comment](https://github.com/rust-lang/rust/pull/120248#discussion_r1463835350))
Some notes:
- We only care about the metadata/last field, so you can still cast `*const dyn T` to `*const WithHeader<dyn T>`, etc
- The lifetime of the trait object itself (`dyn A + 'lt`) is not checked, so you can still cast `*mut FnOnce() + '_` to `*mut FnOnce() + 'static`, etc
- This feels fishy, but I couldn't come up with a reason it must be checked
The diagnostics are currently not great, to say the least, but as far as I can tell this correctly fixes the issues.
cc `@oli-obk` `@compiler-errors` `@lcnr`
Automatically taint InferCtxt when errors are emitted
r? `@nnethercote`
Basically `InferCtxt::dcx` now returns a `DiagCtxt` that refers back to the `Cell<Option<ErrorGuaranteed>>` of the `InferCtxt` and thus when invoking `Diag::emit`, and the diagnostic is an error, we taint the `InferCtxt` directly.
That change on its own has no effect at all, because `InferCtxt` already tracks whether errors have been emitted by recording the global error count when it gets opened, and checking at the end whether the count changed. So I removed that error count check, which had a bit of fallout that I immediately fixed by invoking `InferCtxt::dcx` instead of `TyCtxt::dcx` in a bunch of places.
The remaining new errors are because an error was reported in another query, and never bubbled up. I think they are minor enough for this to be ok, and sometimes it actually improves diagnostics, by not silencing useful diagnostics anymore.
fixes#126485 (cc `@olafes)`
There are more improvements we can do (like tainting in hir ty lowering), but I would rather do that in follow up PRs, because it requires some refactorings.
Fix `...` in multline code-skips in suggestions
When we have long code skips, we write `...` in the line number gutter.
For suggestions, we were "centering" the `...` with the line, but that was inconsistent with what we do in every other case *and* off-center.
Fix assertion failure for some `Expect` diagnostics.
In #120699 I moved some code dealing with `has_future_breakage` earlier in `emit_diagnostic`. Issue #126521 identified a case where that reordering was invalid (leading to an assertion failure) for some `Expect` diagnostics.
This commit partially undoes the change, by moving the handling of unstable `Expect` diagnostics earlier again. This makes `emit_diagnostic` a bit uglier, but is necessary to fix the problem.
Fixes#126521.
r? ``@oli-obk``
When we have long code skips, we write `...` in the line number gutter.
For suggestions, we were "centering" the `...` with the line, but that was consistent with what we do in every other case.
In #120699 I moved some code dealing with `has_future_breakage` earlier
in `emit_diagnostic`. Issue #126521 identified a case where that
reordering was invalid (leading to an assertion failure) for some `Expect`
diagnostics.
This commit partially undoes the change, by moving the handling of
unstable `Expect` diagnostics earlier again. This makes
`emit_diagnostic` a bit uglier, but is necessary to fix the problem.
Fixes#126521.
Most modules have such a blank line, but some don't. Inserting the blank
line makes it clearer that the `//!` comments are describing the entire
module, rather than the `use` declaration(s) that immediately follows.
[perf] Delay the construction of early lint diag structs
Attacks some of the perf regressions from https://github.com/rust-lang/rust/pull/124417#issuecomment-2123700666.
See individual commits for details. The first three commits are not strictly necessary.
However, the 2nd one (06bc4fc671, *Remove `LintDiagnostic::msg`*) makes the main change way nicer to implement.
It's also pretty sweet on its own if I may say so myself.
With the removal of `LintDiagnostic::msg` / the `msg` param from
lint diag APIs, primary messages for lint diags are always constructed
lazily inside decorator fns rendering this wrapper type unused / useless.
* instead simply set the primary message inside the lint decorator functions
* it used to be this way before [#]101986 which introduced `msg` to prevent
good path delayed bugs (which no longer exist) from firing under certain
circumstances when lints were suppressed / silenced
* this is no longer necessary for various reasons I presume
* it shaves off complexity and makes further changes easier to implement
