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

Reword E0277 default error message #121766

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

estebank
Copy link
Contributor

error[E0277]: the trait `Copy` is not implemented for `X`
  --> $DIR/trait-impl-bound-suggestions.rs:14:52
   |
LL |     fn return_the_constrained_type(&self, x: X) -> ConstrainedStruct<X> {
   |                                                    ^^^^^^^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `X`

@rustbot
Copy link
Collaborator

rustbot commented Feb 28, 2024

r? @fmease

rustbot has assigned @fmease.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Feb 28, 2024
@estebank
Copy link
Contributor Author

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@compiler-errors
Copy link
Member

compiler-errors commented Feb 29, 2024

Do you mind adding a fleshed out PR description that explains the motivation for this change? And also what it actually implements alongside the main message reordering, like erasing lifetimes from the predicate?

For example, I assume that you're erasing lifetimes because you were getting split-up binders in error messages like "the trait for<'a> Foo was not implemented for &'a Bar"? It would be nice to actually know what motivated that change -- because if it was just for aesthetic purposes, then maybe it's best to separate that out into separable PR for future git-blames and tweaks on error reporting w/ binders.

In general, over-documenting rather than under-documenting is probably best for those who are exploring past PRs, and right now the PR is a bit light on the wording, especially given its size.

@estebank
Copy link
Contributor Author

@compiler-errors FWIW, I opened the PR for the benefit of the zulip chat conversation (which sadly ended up going to discuss an separate topic that I made the mistake of raising on the same thread) with the anticipation that the feedback might require many further changes to this code. I assigned to @oli-obk because we had been talking about making this change and needing input from t-types, so I knew he wouldn't r+ it until that conversation happened. I should probably have opened it as a draft PR.

```
error[E0277]: the trait `Copy` is not implemented for `X`
  --> $DIR/trait-impl-bound-suggestions.rs:14:52
   |
LL |     fn return_the_constrained_type(&self, x: X) -> ConstrainedStruct<X> {
   |                                                    ^^^^^^^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `X`
```
@estebank
Copy link
Contributor Author

@compiler-errors I need to edit the commit descriptions, but I split the changes into independent commits so that you can see what each change affects in the output.

For example, I assume that you're erasing lifetimes because you were getting split-up binders in error messages like "the trait for<'a> Foo was not implemented for &'a Bar"? It would be nice to actually know what motivated that change -- because if it was just for aesthetic purposes,

That's what originally caught my attention, but as I was looking at it I noticed that the flat out removal of lifetimes made the message read clearer without too much lost information. This might be different if the only reason a bound failed was due to outlives restrictions, which we could handle independently. Another thing that would be nice not to lose would be 'static bounds on types that with the current blatant erasure we do.

@bors
Copy link
Contributor

bors commented Mar 2, 2024

☔ The latest upstream changes (presumably #121890) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants