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

better ObligationCause for normalization errors in can_type_implement_copy #93714

Merged
merged 2 commits into from
Feb 25, 2022

Conversation

compiler-errors
Copy link
Member

Some logic is needed so we can point to the field when given totally nonsense types like struct Foo(<u32 as Iterator>::Item);

Fixes #93687

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 6, 2022
@rust-highfive
Copy link
Collaborator

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 6, 2022
@rust-log-analyzer

This comment has been minimized.

Comment on lines 53 to 65
// If the ADT has substs, point to the cause we are given.
// If it does not, then this field probably doesn't normalize
// to begin with, and point to the bad field.
let cause = if field
.ty(tcx, traits::InternalSubsts::identity_for_item(tcx, adt.did))
.has_param_types_or_consts()
{
cause.clone()
} else {
ObligationCause::dummy_with_span(span)
};
Copy link
Member

Choose a reason for hiding this comment

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

This is...weird. If you don't do this, what changes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Some test starts pointing at a derive(Copy) saying that the type doesn't implement a trait, instead of the struct field.

Not at my computer, but it's some test that contains <u32 as Iterator>::Item in a struct that derives Copy.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've split out this hack into its own commit -- see aa4d68a for the regression in spans in issue-50480 (i.e. without this code).

There's probably something I don't understand about the ordering of the impl Copy correctness check and whatever check (wf?) that raises an error that the struct field <i32 as Iterator>::Item is invalid. Perhaps I can fix this in a better way than I am doing right now.

Copy link
Member

Choose a reason for hiding this comment

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

Ugh, I see that this does fix a Span, but it still feels wrong to me. Kind of signals that this isn't the best diff. Anyways, can you just add a FIXME.

@compiler-errors compiler-errors force-pushed the can-type-impl-copy-error-span branch from 81b2700 to 6de05ba Compare February 6, 2022 23:57
@jackh726
Copy link
Member

r=me with FIXME

@bors delegate+

@bors
Copy link
Contributor

bors commented Feb 24, 2022

✌️ @compiler-errors can now approve this pull request

@compiler-errors compiler-errors force-pushed the can-type-impl-copy-error-span branch from 6de05ba to ee98dc8 Compare February 24, 2022 16:41
@compiler-errors
Copy link
Member Author

CI is broken because crates.io API is having troubles. Will r= later.

@compiler-errors
Copy link
Member Author

@bors r=jackh726

@bors
Copy link
Contributor

bors commented Feb 24, 2022

📌 Commit ee98dc8 has been approved by jackh726

@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 Feb 24, 2022
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@compiler-errors
Copy link
Member Author

@bors retry crates.io is still sad

@compiler-errors
Copy link
Member Author

er, maybe just need to r+ again actually

@bors r=jackh726

@bors
Copy link
Contributor

bors commented Feb 24, 2022

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Feb 24, 2022

📌 Commit ee98dc8 has been approved by jackh726

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Feb 24, 2022
…error-span, r=jackh726

better ObligationCause for normalization errors in `can_type_implement_copy`

Some logic is needed so we can point to the field when given totally nonsense types like `struct Foo(<u32 as Iterator>::Item);`

Fixes rust-lang#93687
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Feb 24, 2022
…error-span, r=jackh726

better ObligationCause for normalization errors in `can_type_implement_copy`

Some logic is needed so we can point to the field when given totally nonsense types like `struct Foo(<u32 as Iterator>::Item);`

Fixes rust-lang#93687
This was referenced Feb 24, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 24, 2022
Rollup of 9 pull requests

Successful merges:

 - rust-lang#91795 (resolve/metadata: Stop encoding macros as reexports)
 - rust-lang#93714 (better ObligationCause for normalization errors in `can_type_implement_copy`)
 - rust-lang#94175 (Improve `--check-cfg` implementation)
 - rust-lang#94212 (Stop manually SIMDing in `swap_nonoverlapping`)
 - rust-lang#94242 (properly handle fat pointers to uninhabitable types)
 - rust-lang#94308 (Normalize main return type during mono item collection & codegen)
 - rust-lang#94315 (update auto trait lint for `PhantomData`)
 - rust-lang#94316 (Improve string literal unescaping)
 - rust-lang#94327 (Avoid emitting full macro body into JSON errors)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7f99536 into rust-lang:master Feb 25, 2022
@rustbot rustbot added this to the 1.61.0 milestone Feb 25, 2022
flip1995 pushed a commit to flip1995/rust that referenced this pull request Mar 14, 2022
…error-span, r=jackh726

better ObligationCause for normalization errors in `can_type_implement_copy`

Some logic is needed so we can point to the field when given totally nonsense types like `struct Foo(<u32 as Iterator>::Item);`

Fixes rust-lang#93687
@compiler-errors compiler-errors deleted the can-type-impl-copy-error-span branch April 7, 2022 04:32
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.

Error message pointing to struct definition instead of pointing to trait impl with missing trait bound
6 participants