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

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

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

lengrongfu
Copy link
Contributor

Fixes: #125984

  • Build rustc use ./x build.
  • Test result
image
  • vim test.rs
trait Reconcile {
    fn reconcile(&self);
}

// Implementing the trait for some types
impl Reconcile for bool {
    fn reconcile(&self) {
        println!("Reconciling bool");
    }
}

impl Reconcile for i8 {
    fn reconcile(&self) {
        println!("Reconciling i8");
    }
}

impl Reconcile for i16 {
    fn reconcile(&self) {
        println!("Reconciling i16");
    }
}

impl Reconcile for i32 {
    fn reconcile(&self) {
        println!("Reconciling i32");
    }
}

impl Reconcile for i64 {
    fn reconcile(&self) {
        println!("Reconciling i64");
    }
}

impl Reconcile for u8 {
    fn reconcile(&self) {
        println!("Reconciling u8");
    }
}

impl Reconcile for u16 {
    fn reconcile(&self) {
        println!("Reconciling u16");
    }
}

impl Reconcile for u32 {
    fn reconcile(&self) {
        println!("Reconciling u32");
    }
}

impl Reconcile for i128 {
    fn reconcile(&self) {
        println!("Reconciling u32");
    }
}

impl Reconcile for u128 {
    fn reconcile(&self) {
        println!("Reconciling u32");
    }
}

fn process<T: Reconcile>(item: T) {
    item.reconcile();
}

fn main() {
    let value = String::from("This will cause an error");
    process(value); // This line will cause a compilation error
}

…verbose

Signed-off-by: rongfu.leng <lenronfu@gmail.com>
@rustbot
Copy link
Collaborator

rustbot commented Jun 6, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pnkfelix (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 6, 2024
@lengrongfu
Copy link
Contributor Author

lengrongfu commented Jun 6, 2024

r? @estebank

@rustbot rustbot assigned estebank and unassigned pnkfelix Jun 6, 2024
@lengrongfu
Copy link
Contributor Author

r? @pnkfelix

@rustbot rustbot assigned pnkfelix and unassigned estebank Jun 10, 2024
@pnkfelix pnkfelix changed the title Expand list of trait implementers in E0277 when calling rustc with --… Expand list of trait implementers in E0277 when calling rustc with --verbose Jun 11, 2024
@@ -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

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

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.

@pnkfelix
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 11, 2024

📌 Commit 69769fc has been approved by pnkfelix

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 11, 2024
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Jun 11, 2024
Expand list of trait implementers in E0277 when calling rustc with --verbose

Fixes: rust-lang#125984

- Build `rustc` use `./x build`.
- Test result
<img width="634" alt="image" src="https://github.com/rust-lang/rust/assets/15009201/89377059-2316-492b-a38a-fa33adfc9793">

- vim test.rs
```rust
trait Reconcile {
    fn reconcile(&self);
}

// Implementing the trait for some types
impl Reconcile for bool {
    fn reconcile(&self) {
        println!("Reconciling bool");
    }
}

impl Reconcile for i8 {
    fn reconcile(&self) {
        println!("Reconciling i8");
    }
}

impl Reconcile for i16 {
    fn reconcile(&self) {
        println!("Reconciling i16");
    }
}

impl Reconcile for i32 {
    fn reconcile(&self) {
        println!("Reconciling i32");
    }
}

impl Reconcile for i64 {
    fn reconcile(&self) {
        println!("Reconciling i64");
    }
}

impl Reconcile for u8 {
    fn reconcile(&self) {
        println!("Reconciling u8");
    }
}

impl Reconcile for u16 {
    fn reconcile(&self) {
        println!("Reconciling u16");
    }
}

impl Reconcile for u32 {
    fn reconcile(&self) {
        println!("Reconciling u32");
    }
}

impl Reconcile for i128 {
    fn reconcile(&self) {
        println!("Reconciling u32");
    }
}

impl Reconcile for u128 {
    fn reconcile(&self) {
        println!("Reconciling u32");
    }
}

fn process<T: Reconcile>(item: T) {
    item.reconcile();
}

fn main() {
    let value = String::from("This will cause an error");
    process(value); // This line will cause a compilation error
}
```
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 11, 2024
Rollup of 7 pull requests

Successful merges:

 - rust-lang#115974 (Split core's PanicInfo and std's PanicInfo)
 - rust-lang#125659 (Remove usage of `isize` in example)
 - rust-lang#125669 (CI: Update riscv64gc-linux job to Ubuntu 22.04, rename to riscv64gc-gnu)
 - rust-lang#125684 (Account for existing bindings when suggesting `pin!()`)
 - rust-lang#126055 (Expand list of trait implementers in E0277 when calling rustc with --verbose)
 - rust-lang#126174 (Migrate `tests/run-make/prefer-dylib` to `rmake.rs`)
 - rust-lang#126256 (Add {{target}} substitution to compiletest)

r? `@ghost`
`@rustbot` modify labels: rollup
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Jun 11, 2024
Expand list of trait implementers in E0277 when calling rustc with --verbose

Fixes: rust-lang#125984

- Build `rustc` use `./x build`.
- Test result
<img width="634" alt="image" src="https://github.com/rust-lang/rust/assets/15009201/89377059-2316-492b-a38a-fa33adfc9793">

- vim test.rs
```rust
trait Reconcile {
    fn reconcile(&self);
}

// Implementing the trait for some types
impl Reconcile for bool {
    fn reconcile(&self) {
        println!("Reconciling bool");
    }
}

impl Reconcile for i8 {
    fn reconcile(&self) {
        println!("Reconciling i8");
    }
}

impl Reconcile for i16 {
    fn reconcile(&self) {
        println!("Reconciling i16");
    }
}

impl Reconcile for i32 {
    fn reconcile(&self) {
        println!("Reconciling i32");
    }
}

impl Reconcile for i64 {
    fn reconcile(&self) {
        println!("Reconciling i64");
    }
}

impl Reconcile for u8 {
    fn reconcile(&self) {
        println!("Reconciling u8");
    }
}

impl Reconcile for u16 {
    fn reconcile(&self) {
        println!("Reconciling u16");
    }
}

impl Reconcile for u32 {
    fn reconcile(&self) {
        println!("Reconciling u32");
    }
}

impl Reconcile for i128 {
    fn reconcile(&self) {
        println!("Reconciling u32");
    }
}

impl Reconcile for u128 {
    fn reconcile(&self) {
        println!("Reconciling u32");
    }
}

fn process<T: Reconcile>(item: T) {
    item.reconcile();
}

fn main() {
    let value = String::from("This will cause an error");
    process(value); // This line will cause a compilation error
}
```
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 11, 2024
Rollup of 6 pull requests

Successful merges:

 - rust-lang#115974 (Split core's PanicInfo and std's PanicInfo)
 - rust-lang#125659 (Remove usage of `isize` in example)
 - rust-lang#125669 (CI: Update riscv64gc-linux job to Ubuntu 22.04, rename to riscv64gc-gnu)
 - rust-lang#125684 (Account for existing bindings when suggesting `pin!()`)
 - rust-lang#126055 (Expand list of trait implementers in E0277 when calling rustc with --verbose)
 - rust-lang#126174 (Migrate `tests/run-make/prefer-dylib` to `rmake.rs`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 12358a7 into rust-lang:master Jun 12, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 12, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 12, 2024
Rollup merge of rust-lang#126055 - lengrongfu:master, r=pnkfelix

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

Fixes: rust-lang#125984

- Build `rustc` use `./x build`.
- Test result
<img width="634" alt="image" src="https://github.com/rust-lang/rust/assets/15009201/89377059-2316-492b-a38a-fa33adfc9793">

- vim test.rs
```rust
trait Reconcile {
    fn reconcile(&self);
}

// Implementing the trait for some types
impl Reconcile for bool {
    fn reconcile(&self) {
        println!("Reconciling bool");
    }
}

impl Reconcile for i8 {
    fn reconcile(&self) {
        println!("Reconciling i8");
    }
}

impl Reconcile for i16 {
    fn reconcile(&self) {
        println!("Reconciling i16");
    }
}

impl Reconcile for i32 {
    fn reconcile(&self) {
        println!("Reconciling i32");
    }
}

impl Reconcile for i64 {
    fn reconcile(&self) {
        println!("Reconciling i64");
    }
}

impl Reconcile for u8 {
    fn reconcile(&self) {
        println!("Reconciling u8");
    }
}

impl Reconcile for u16 {
    fn reconcile(&self) {
        println!("Reconciling u16");
    }
}

impl Reconcile for u32 {
    fn reconcile(&self) {
        println!("Reconciling u32");
    }
}

impl Reconcile for i128 {
    fn reconcile(&self) {
        println!("Reconciling u32");
    }
}

impl Reconcile for u128 {
    fn reconcile(&self) {
        println!("Reconciling u32");
    }
}

fn process<T: Reconcile>(item: T) {
    item.reconcile();
}

fn main() {
    let value = String::from("This will cause an error");
    process(value); // This line will cause a compilation error
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expand list of trait implementers in E0277 when calling rustc with --verbose
6 participants