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

E0277 highlights really long types for no reason, and doesn't highlight useful help #132013

Closed
jyn514 opened this issue Oct 21, 2024 · 4 comments · Fixed by #132147
Closed

E0277 highlights really long types for no reason, and doesn't highlight useful help #132013

jyn514 opened this issue Oct 21, 2024 · 4 comments · Fixed by #132147
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Oct 21, 2024

Code

#!/usr/bin/env cargo

//! ```cargo
//! [dependencies]
//! chumsky = "1.0.0-alpha.7"
//! ```

use chumsky::{primitive::any, Parser};

pub(crate) trait CSTParser<'a, O = ()>: Parser<'a, &'a str, O> {}

impl<'a, O, T> CSTParser<'a, O> for T where T: Parser<'a, &'a str, O> {}

fn leaf<'a, O>(parser: impl CSTParser<'a, O>) -> impl CSTParser<'a, ()> {
    let ws = any()
        .filter(|c: &char| *c != '\n' && c.is_whitespace())
        .ignored();
    ws().then(parser.map(|_| ()))
}

fn main() {}

Current output

Image

error[E0277]: the trait bound `Then<Ignored<chumsky::combinator::Filter<chumsky::primitive::Any<&str, chumsky::extra::Full<EmptyErr, (), ()>>, {closure@repro.rs:16:17: 16:27}>, char>, chumsky::combinator::Map<impl CSTParser<'a, O>, O, {closure@repro.rs:18:32: 18:35}>, (), (), chumsky::extra::Full<EmptyErr, (), ()>>: CSTParser<'a>` is not satisfied
  --> repro.rs:14:50
   |
14 | fn leaf<'a, O>(parser: impl CSTParser<'a, O>) -> impl CSTParser<'a, ()> {
   |                                                  ^^^^^^^^^^^^^^^^^^^^^^ the trait `chumsky::private::ParserSealed<'_, &str, (), chumsky::extra::Full<EmptyErr, (), ()>>` is not implemented for `Then<Ignored<Filter<..., ...>, ...>, ..., ..., ..., ...>`, which is required by `Then<Ignored<chumsky::combinator::Filter<chumsky::primitive::Any<&str, chumsky::extra::Full<EmptyErr, (), ()>>, {closure@repro.rs:16:17: 16:27}>, char>, chumsky::combinator::Map<impl CSTParser<'a, O>, O, {closure@repro.rs:18:32: 18:35}>, (), (), chumsky::extra::Full<EmptyErr, (), ()>>: CSTParser<'a>`
   |
   = help: the trait `chumsky::private::ParserSealed<'_, &'a str, ((), ()), chumsky::extra::Full<EmptyErr, (), ()>>` is implemented for `Then<Ignored<chumsky::combinator::Filter<chumsky::primitive::Any<&str, chumsky::extra::Full<EmptyErr, (), ()>>, {closure@repro.rs:16:17: 16:27}>, char>, chumsky::combinator::Map<impl CSTParser<'a, O>, O, {closure@repro.rs:18:32: 18:35}>, (), (), chumsky::extra::Full<EmptyErr, (), ()>>`
   = help: for that trait implementation, expected `((), ())`, found `()`
   = note: required for `Then<Ignored<Filter<..., ...>, ...>, ..., ..., ..., ...>` to implement `Parser<'_, &str, ()>`
note: required for `Then<Ignored<Filter<..., ...>, ...>, ..., ..., ..., ...>` to implement `CSTParser<'a>`
  --> repro.rs:12:16
   |
12 | impl<'a, O, T> CSTParser<'a, O> for T where T: Parser<'a, &'a str, O> {}
   |                ^^^^^^^^^^^^^^^^     ^          ---------------------- unsatisfied trait bound introduced here
   = note: the full name for the type has been written to '/home/jyn/.local/lib/cargo/target/debug/deps/foo-f0b1a0054d2a8996.long-type-7449552674356047294.txt'
   = note: consider using `--verbose` to print the full type name to the console
   = note: the full name for the type has been written to '/home/jyn/.local/lib/cargo/target/debug/deps/foo-f0b1a0054d2a8996.long-type-7449552674356047294.txt'
   = note: consider using `--verbose` to print the full type name to the console

For more information about this error, try `rustc --explain E0277`.

Desired output

  • expected `((), ())`, found `()` should be highlighted instead of is implemented for `Then<...>`
  • the difference between ParserSealed<((), ())> and ParserSealed<()> should be highlighted.
  • "the full name for the type has been written ..." should not be printed multiple times
  • "trait is not implemented for T" followed by "trait is implemented for T" is really confusing. rustc shouldn't do that; or at least word it differently.
  • especially since T is the same in both cases, it should not highlight T in purple.
  • this should show the return value of the function somewhere; right now it's not even clear that the ws().then() line is related (from the spans, it looks like this is an error at the level of the function signature, not the function implementation)

Rationale and extra context

Other cases

No response

Rust Version

rustc 1.84.0-nightly (662180b 2024-10-20)
binary: rustc
commit-hash: 662180b
commit-date: 2024-10-20
host: x86_64-unknown-linux-gnu
release: 1.84.0-nightly
LLVM version: 19.1.1

Anything else?

No response

@jyn514 jyn514 added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 21, 2024
@jyn514
Copy link
Member Author

jyn514 commented Oct 21, 2024

also this is a bit of a larger change, but it would be nice if rustc never broke up a diagnostic across a type name. it means that it's harder to visually parse, it has bad degenerate behavior when the types are long, and it doesn't really seem more clear than, e.g., "a trait bound was not satisfied: Then<...>: CSTParser"

@jyn514
Copy link
Member Author

jyn514 commented Oct 21, 2024

i think the the whole ", which is required by ..." clause shouldn't be there either tbh - it's already in a note below, and it's right in the middle of the span making it harder to see the rest of the diagnostic.

estebank added a commit to estebank/rust that referenced this issue Oct 24, 2024
…ng same path multiple times

Because `note_obligation_cause_code` is recursive, if multiple types are too
long to print to the terminal, a `long_ty_file` will be created. Before, one was
created *per recursion*. Now, it is passed in so it gets printed only once.

Part of rust-lang#132013.
@estebank
Copy link
Contributor

estebank commented Oct 24, 2024

but it would be nice if rustc never broke up a diagnostic across a type name

We changed this some time back so it doesn't do that: now it always replaces generic parameters as a whole, the trimming is not textual anymore.

this should show the return value of the function somewhere; right now it's not even clear that the ws().then() line is related (from the spans, it looks like this is an error at the level of the function signature, not the function implementation)

I think that we fail to highlight the tail or return expression for all cases involving impl Trait... We should fix that.

I need to clean this up, but I managed to reuse the type comparison machinery for traits and got this:

Image

textual output
error[E0277]: the trait bound `Then<Ignored<chumsky::combinator::Filter<chumsky::primitive::Any<&str, chumsky::extra::Full<EmptyErr, (), ()>>, {closure@src/main.rs:9:17: 9:27}>, char>, chumsky::combinator::Map<impl CSTParser<'a, O>, O, {closure@src/main.rs:11:24: 11:27}>, (), (), chumsky::extra::Full<EmptyErr, (), ()>>: CSTParser<'a>` is not satisfied
 --> src/main.rs:7:50
  |
7 | fn leaf<'a, O>(parser: impl CSTParser<'a, O>) -> impl CSTParser<'a, ()> {
  |                                                  ^^^^^^^^^^^^^^^^^^^^^^ the trait `chumsky::private::ParserSealed<'_, &str, (), chumsky::extra::Full<EmptyErr, (), ()>>` is not implemented for `Then<Ignored<..., ...>, ..., ..., ..., ...>`
  |
  = help: the trait `ParserSealed<'_, &_, (), Full<_, _, _>>` is not implemented for `Then<Ignored<chumsky::combinator::Filter<chumsky::primitive::Any<&str, chumsky::extra::Full<EmptyErr, (), ()>>, {closure@src/main.rs:9:17: 9:27}>, char>, chumsky::combinator::Map<impl CSTParser<'a, O>, O, {closure@src/main.rs:11:24: 11:27}>, (), (), chumsky::extra::Full<EmptyErr, (), ()>>`
  = help: the trait `ParserSealed<'_, &'a _, ((), ()), Full<_, _, _>>` is implemented for `Then<Ignored<chumsky::combinator::Filter<chumsky::primitive::Any<&str, chumsky::extra::Full<EmptyErr, (), ()>>, {closure@src/main.rs:9:17: 9:27}>, char>, chumsky::combinator::Map<impl CSTParser<'a, O>, O, {closure@src/main.rs:11:24: 11:27}>, (), (), chumsky::extra::Full<EmptyErr, (), ()>>`
  = help: for that trait implementation, expected `((), ())`, found `()`
  = note: required for `Then<Ignored<..., ...>, ..., ..., ..., ...>` to implement `Parser<'_, &str, ()>`
note: required for `Then<Ignored<..., ...>, ..., ..., ..., ...>` to implement `CSTParser<'a>`
 --> src/main.rs:5:16
  |
5 | impl<'a, O, T> CSTParser<'a, O> for T where T: Parser<'a, &'a str, O> {}
  |                ^^^^^^^^^^^^^^^^     ^          ---------------------- unsatisfied trait bound introduced here
  = note: the full name for the type has been written to '/home/gh-estebank/longlong/target/debug/deps/longlong-df9d52be87eada65.long-type-2878432305708693415.txt'
  = note: consider using `--verbose` to print the full type name to the console

The reason the is/isn't implemented for X type is present is because although in this case the implementer is the same, like in:

error[E0277]: the trait bound `&str: AsExpression<Integer>` is not satisfied
  --> $DIR/as_expression.rs:57:15
   |
LL |     SelectInt.check("bar");
   |               ^^^^^ the trait `AsExpression<Integer>` is not implemented for `&str`
   |
   = help: the trait `AsExpression<Integer>` is not implemented for `&str`
   = help: the trait `AsExpression<Text>` is implemented for `&str`
   = help: for that trait implementation, expected `Text`, found `Integer`

there are cases where they are not, like:

error[E0277]: the type `[{integer}]` cannot be indexed by `i32`
  --> $DIR/index-help.rs:3:7
   |
LL |     x[0i32];
   |       ^^^^ slice indices are of type `usize` or ranges of `usize`
   |
   = help: the trait `SliceIndex<[{integer}]>` is not implemented for `i32`, which is required by `Vec<{integer}>: Index<_>`
   = help: the trait `SliceIndex<_>` is not implemented for `i32`
   = help: the trait `SliceIndex<_>` is implemented for `usize`
   = help: for that trait implementation, expected `usize`, found `i32`
   = note: required for `Vec<{integer}>` to implement `Index<i32>`

We should absolutely account for both cases slightly differently.

@estebank estebank pinned this issue Oct 24, 2024
@estebank estebank unpinned this issue Oct 24, 2024
@estebank estebank self-assigned this Oct 25, 2024
@estebank
Copy link
Contributor

@jyn514 what do you think of this?

Image

textual output
error[E0277]: the trait bound `Then<Ignored<chumsky::combinator::Filter<chumsky::primitive::Any<&str, chumsky::extra::Full<EmptyErr, (), ()>>, {closure@src/main.rs:9:17: 9:27}>, char>, chumsky::combinator::Map<impl CSTParser<'a, O>, O, {closure@src/main.rs:11:24: 11:27}>, (), (), chumsky::extra::Full<EmptyErr, (), ()>>: CSTParser<'a>` is not satisfied
  --> src/main.rs:7:50
   |
7  | fn leaf<'a, O>(parser: impl CSTParser<'a, O>) -> impl CSTParser<'a, ()> {
   |                                                  ^^^^^^^^^^^^^^^^^^^^^^ unsatisfied trait bound
...
11 |     ws.then(parser.map(|_| ()))
   |     --------------------------- return type was inferred to be `Then<Ignored<Filter<Any<&str, ...>, ...>, ...>, ..., ..., ..., ...>` here
   |
   = help: the trait `ParserSealed<'_, &_, (), Full<_, _, _>>` is not implemented for `Then<Ignored<Filter<Any<&str, ...>, ...>, ...>, ..., ..., ..., ...>`
           but trait `ParserSealed<'_, &'a _, ((), ()), Full<_, _, _>>` is implemented for it
   = help: for that trait implementation, expected `((), ())`, found `()`
   = note: required for `Then<Ignored<Filter<Any<&str, ...>, ...>, ...>, ..., ..., ..., ...>` to implement `Parser<'_, &str, ()>`
note: required for `Then<Ignored<Filter<Any<&str, ...>, ...>, ...>, ..., ..., ..., ...>` to implement `CSTParser<'a>`
  --> src/main.rs:5:16
   |
5  | impl<'a, O, T> CSTParser<'a, O> for T where T: Parser<'a, &'a str, O> {}
   |                ^^^^^^^^^^^^^^^^     ^          ---------------------- unsatisfied trait bound introduced here
   = note: the full name for the type has been written to '/home/gh-estebank/longlong/target/debug/deps/longlong-df9d52be87eada65.long-type-3075078524015204798.txt'
   = note: consider using `--verbose` to print the full type name to the console

estebank added a commit to estebank/rust that referenced this issue Oct 25, 2024
estebank added a commit to estebank/rust that referenced this issue Oct 25, 2024
…ng same path multiple times

Because `note_obligation_cause_code` is recursive, if multiple types are too
long to print to the terminal, a `long_ty_file` will be created. Before, one was
created *per recursion*. Now, it is passed in so it gets printed only once.

Part of rust-lang#132013.
estebank added a commit to estebank/rust that referenced this issue Oct 25, 2024
estebank added a commit to estebank/rust that referenced this issue Oct 25, 2024
…ng same path multiple times

Because `note_obligation_cause_code` is recursive, if multiple types are too
long to print to the terminal, a `long_ty_file` will be created. Before, one was
created *per recursion*. Now, it is passed in so it gets printed only once.

Part of rust-lang#132013.
jieyouxu added a commit to jieyouxu/rust that referenced this issue Oct 28, 2024
Tweak E0277 highlighting and "long type" path printing

Partially address rust-lang#132013.

![Output from this PR for the repro case in rust-lang#132013](https://github.com/user-attachments/assets/a073ba37-4adc-411e-81f7-6cb9a945ce3d)
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 28, 2024
Rollup merge of rust-lang#132086 - estebank:long-types, r=jieyouxu

Tweak E0277 highlighting and "long type" path printing

Partially address rust-lang#132013.

![Output from this PR for the repro case in rust-lang#132013](https://github.com/user-attachments/assets/a073ba37-4adc-411e-81f7-6cb9a945ce3d)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 1, 2024
Tweak E0277 output when a candidate is available

*Follow up to rust-lang#132086.*

Go from

```
error[E0277]: the trait bound `Then<Ignored<chumsky::combinator::Filter<chumsky::primitive::Any<&str, chumsky::extra::Full<EmptyErr, (), ()>>, {closure@src/main.rs:9:17: 9:27}>, char>, chumsky::combinator::Map<impl CSTParser<'a, O>, O, {closure@src/main.rs:11:24: 11:27}>, (), (), chumsky::extra::Full<EmptyErr, (), ()>>: CSTParser<'a>` is not satisfied
 --> src/main.rs:7:50
  |
7 | fn leaf<'a, O>(parser: impl CSTParser<'a, O>) -> impl CSTParser<'a, ()> {
  |                                                  ^^^^^^^^^^^^^^^^^^^^^^ the trait `chumsky::private::ParserSealed<'_, &str, (), chumsky::extra::Full<EmptyErr, (), ()>>` is not implemented for `Then<Ignored<Filter<Any<&str, ...>, ...>, ...>, ..., ..., ..., ...>`, which is required by `Then<Ignored<chumsky::combinator::Filter<chumsky::primitive::Any<&str, chumsky::extra::Full<EmptyErr, (), ()>>, {closure@src/main.rs:9:17: 9:27}>, char>, chumsky::combinator::Map<impl CSTParser<'a, O>, O, {closure@src/main.rs:11:24: 11:27}>, (), (), chumsky::extra::Full<EmptyErr, (), ()>>: CSTParser<'a>`
  |
  = help: the trait `chumsky::private::ParserSealed<'_, &'a str, ((), ()), chumsky::extra::Full<EmptyErr, (), ()>>` is implemented for `Then<Ignored<chumsky::combinator::Filter<chumsky::primitive::Any<&str, chumsky::extra::Full<EmptyErr, (), ()>>, {closure@src/main.rs:9:17: 9:27}>, char>, chumsky::combinator::Map<impl CSTParser<'a, O>, O, {closure@src/main.rs:11:24: 11:27}>, (), (), chumsky::extra::Full<EmptyErr, (), ()>>`
  = help: for that trait implementation, expected `((), ())`, found `()`
  = note: required for `Then<Ignored<Filter<Any<&str, ...>, ...>, ...>, ..., ..., ..., ...>` to implement `Parser<'_, &str, ()>`
note: required for `Then<Ignored<Filter<Any<&str, ...>, ...>, ...>, ..., ..., ..., ...>` to implement `CSTParser<'a>`
 --> src/main.rs:5:16
  |
5 | impl<'a, O, T> CSTParser<'a, O> for T where T: Parser<'a, &'a str, O> {}
  |                ^^^^^^^^^^^^^^^^     ^          ---------------------- unsatisfied trait bound introduced here
  = note: the full name for the type has been written to '/home/gh-estebank/longlong/target/debug/deps/longlong-0008f9a4f2023b08.long-type-13239977239800463552.txt'
  = note: consider using `--verbose` to print the full type name to the console
  = note: the full name for the type has been written to '/home/gh-estebank/longlong/target/debug/deps/longlong-0008f9a4f2023b08.long-type-13239977239800463552.txt'
  = note: consider using `--verbose` to print the full type name to the console
```

to

```
error[E0277]: the trait bound `Then<Ignored<chumsky::combinator::Filter<chumsky::primitive::Any<&str, chumsky::extra::Full<EmptyErr, (), ()>>, {closure@src/main.rs:9:17: 9:27}>, char>, chumsky::combinator::Map<impl CSTParser<'a, O>, O, {closure@src/main.rs:11:24: 11:27}>, (), (), chumsky::extra::Full<EmptyErr, (), ()>>: CSTParser<'a>` is not satisfied
  --> src/main.rs:7:50
   |
7  | fn leaf<'a, O>(parser: impl CSTParser<'a, O>) -> impl CSTParser<'a, ()> {
   |                                                  ^^^^^^^^^^^^^^^^^^^^^^ unsatisfied trait bound
...
11 |     ws.then(parser.map(|_| ()))
   |     --------------------------- return type was inferred to be `Then<Ignored<..., ...>, ..., ..., ..., ...>` here
   |
   = help: the trait `ParserSealed<'_, &_, (), Full<_, _, _>>` is not implemented for `Then<Ignored<..., ...>, ..., ..., ..., ...>`
           but trait `ParserSealed<'_, &'a _, ((), ()), Full<_, _, _>>` is implemented for it
   = help: for that trait implementation, expected `((), ())`, found `()`
   = note: required for `Then<Ignored<..., ...>, ..., ..., ..., ...>` to implement `Parser<'_, &str, ()>`
note: required for `Then<Ignored<..., ...>, ..., ..., ..., ...>` to implement `CSTParser<'a>`
  --> src/main.rs:5:16
   |
5  | impl<'a, O, T> CSTParser<'a, O> for T where T: Parser<'a, &'a str, O> {}
   |                ^^^^^^^^^^^^^^^^     ^          ---------------------- unsatisfied trait bound introduced here
   = note: the full name for the type has been written to '/home/gh-estebank/longlong/target/debug/deps/longlong-df9d52be87eada65.long-type-1337037744507305372.txt'
   = note: consider using `--verbose` to print the full type name to the console
```

* Remove redundant wording
* Introduce trait diff highlighting logic and use it
* Fix incorrect "long type written to path" logic (can be split off)
* Point at tail expression in more cases in E0277
* Avoid long primary span labels in E0277 by moving them to a `help`

Fix rust-lang#132013.

There are individual commits that can be their own PR. If the review load is too big, happy to split them off.
@bors bors closed this as completed in b3f75cc Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants