Better error reporting in Copy derive #50536
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
estebank
reviewed
May 8, 2018
src/test/ui/issue-50480.stderr
Outdated
| | ^^^^ | ||
| ... | ||
| LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); | ||
| | ------- this field does not implement `Copy` |
There was a problem hiding this comment.
This is not introduced by your PR, nor it needs to be addressed here, but the String) span is wrong here and in the previous error's Vec<i32>,.
| tcx.def_span(field.did), | ||
| "this field does not implement `Copy`") | ||
| .emit() | ||
| for field in fields { |
There was a problem hiding this comment.
Instead of having one error per field, I think there should be one error and one span_label per field. This way we reduce the amount of errors being emitted to only one per struct:
+error[E0204]: the trait `Copy` may not be implemented for this type
+ --> $DIR/issue-50480.rs:11:17
+ |
+LL | #[derive(Clone, Copy)]
+ | ^^^^
+...
+LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+ | --------- ------- this field does not implement `Copy`
+ | |
+ | this field does not implement `Copy`
|
Merged the errors as requested, I intended to expand on this PR but gave up, it will be just this for now. |
|
@bors r+ |
|
📌 Commit dfac81c has been approved by |
bors
added
S-waiting-on-bors
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
May 10, 2018
Fix tuple struct field spans Fix rust-lang#50578. Will have a merge conflict with rust-lang#50536.
|
⌛ Testing commit dfac81c2abfb6255f5d24960b2b0acf435d200da with merge 09a553f39d39568bf8ea1d32fa093c5ec9d1082b... |
|
💔 Test failed - status-appveyor |
bors
added
S-waiting-on-review
|
I just need to rebase this. |
In Copy derive, report all fulfillment erros when present and do not report errors for types tainted with `TyErr`. Also report all fields which are not Copy rather than just the first. Also refactored `fn fully_normalize`, removing the not very useful helper function along with a FIXME to the closed issue rust-lang#26721 that's looks out of context now.
leoyvens
force-pushed
the
report-fullfilment-errors-in-copy-derive
branch
from
dfac81c
to
6389f35
Compare
|
@bors r+ |
|
📌 Commit 6389f35 has been approved by |
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
May 12, 2018
…derive, r=estebank Better error reporting in Copy derive In Copy derive, report all fulfillment erros when present and do not report errors for types tainted with `TyErr`. Also report all fields which are not Copy rather than just the first. Also refactored `fn fully_normalize`, removing the not very useful helper function along with a FIXME to the closed issue #26721 that looks out of context now. Fixes #50480 r? @estebank
|
☀️ Test successful - status-appveyor, status-travis |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In Copy derive, report all fulfillment erros when present and do not report errors for types tainted with
TyErr. Also report all fields which are not Copy rather than just the first.Also refactored
fn fully_normalize, removing the not very useful helper function along with a FIXME to the closed issue #26721 that looks out of context now.Fixes #50480
r? @estebank