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

Expand list of trait implementers in E0277 when calling rustc with --verbose #126055

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Expand list of trait implementers in E0277 when calling rustc with --verbose #126055

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2082,12 +2082,16 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
})
.collect();

let end = if candidates.len() <= 9 { candidates.len() } else { 8 };
let end = if candidates.len() <= 9 || self.tcx.sess.opts.verbose {
Copy link
Member

@pnkfelix pnkfelix Jun 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't help but point out that this original logic was/is a little goofy, in that it says: "if the length is 9 or less, keep it. If its greater than 9, then cut it to length 8."

Why wouldn't you cut it to length 9? (Or use "8 or less" as the relevant threshold.)

(My current guess is that it was an off-by-one error in some past person's end in terms of whether candidates[..end] would include or exclude the end itself; otherwise I cannot explain why someone would have written the code like this.)

this has the effect, I think, that will won't actually cut to length 8 until you have 10 elements; a 9-element list is printed in full.

This is not really relevant to this PR though, apart from the fact that reviewing this code tempted me to go in and rewrite it all. But I'm not going to do that.

Copy link

@correabuscar correabuscar Jun 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they might've wanted to avoid a message like and 1 others which would've needed some rewording such as and 1 other(lacking an s ending), so then avoid the more complicated logic with that extra s.

Thus, show 8 + the 2 others msg if above 9, or show all 9 or less with no msg. Makes some sense :D and is kinda clever.

error[E0277]: the trait bound `String: Reconcile` is not satisfied
  --> src/main.rs:72:13
   |
72 |     process(value); // This line will cause a compilation error
   |     ------- ^^^^^ the trait `Reconcile` is not implemented for `String`
   |     |
   |     required by a bound introduced by this call
   |
   = help: the following other types implement trait `Reconcile`:
             bool
             i128
             i16
             i32
             i64
             i8
             u128
             u16
           and 2 others
note: required by a bound in `process`
rror[E0277]: the trait bound `String: Reconcile` is not satisfied
  --> /home/user/sandbox/rust/05_sandbox/error/and_17_others/src/main.rs:72:13
   |
72 |     process(value); // This line will cause a compilation error
   |     ------- ^^^^^ the trait `Reconcile` is not implemented for `String`
   |     |
   |     required by a bound introduced by this call
   |
   = help: the following other types implement trait `Reconcile`:
             bool
             i8
             i16
             i32
             i64
             i128
             u8
             u16
             u32
             u128
note: required by a bound in `process`

oh and btw, I had the exact same thought as you initially, but I wasn't satisfied with the off-by one so I had to search for another reason but still without actually looking at the relevant PR or blame because I was kinda lazy, admittedly (I still haven't looked, I wonder tho, if they did mention it), ok wth, I'm gonna look now...will edit later

EDIT: tracking this down isn't easy lol, but here's the result so far:
It might appear that the source of this is this commit: 3aac307
but it seems it was only improving on something that was originally from this commit: ca54fc76ae30 and thus this PR #39804

So, given the above then, we can conclude that it was even more than just worrying about the extra s, but rather, showing that line, replaced one possible type being listed, so it was useless, thus showing 2 or more was needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't to avoid the "1 others" message, but rather if we're already "wasting" a line to say there's another one, then just print it.

candidates.len()
} else {
8
};
err.help(format!(
"the following {other}types implement trait `{}`:{}{}",
trait_ref.print_trait_sugared(),
candidates[..end].join(""),
if candidates.len() > 9 {
if candidates.len() > 9 && !self.tcx.sess.opts.verbose {
format!("\nand {} others", candidates.len() - 8)
} else {
String::new()
Expand Down
Loading