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

Gracefully fail to resolve associated items instead of delay_span_bug. #96806

Merged
merged 2 commits into from
May 12, 2022
Merged

Gracefully fail to resolve associated items instead of delay_span_bug. #96806

merged 2 commits into from
May 12, 2022

Conversation

cjgillot
Copy link
Contributor

@cjgillot cjgillot commented May 7, 2022 •
edited
Loading

codegen_fulfill_obligation is used during instance resolution for trait items.

In case of insufficient normalization issues during MIR inlining, it caused ICEs.
It's better to gracefully refuse to resolve the associated item, and let the caller decide what to do with this.

Split from #91743
Closes #69121
Closes #73021
Closes #88599
Closes #93008
Closes #93248
Closes #94680
Closes #96170
r? @oli-obk

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 7, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 7, 2022
@oli-obk
Copy link
Contributor

oli-obk commented May 9, 2022

This should affect the ICEs in https://github.com/rust-lang/rust/issues?q=is%3Aissue+is%3Aopen+Encountered+errors+resolving+bounds+outside+of+type+inference

Please add tests for them.

This also likely affects all tests in https://github.com/rust-lang/rust/issues?q=is%3Aissue+is%3Aopen+Encountered+error+%60Unimplemented%60+selecting+during+codegen

That's a lot of ICE issues (21 right now). While in theory we could just merge this PR and let glacier handle the follow up, it seems like some of them may actually end up passing compilation with this PR, and we should be careful and check if that is intended.

@oli-obk
Copy link
Contributor

oli-obk commented May 10, 2022

@bors r+

@bors
Copy link
Contributor

bors commented May 10, 2022

📌 Commit 931e7d61395b40f63ff5b617af97f0362ae36a1c has been approved by oli-obk

@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 May 10, 2022
@oli-obk oli-obk mentioned this pull request May 10, 2022
Closed
@bors
Copy link
Contributor

bors commented May 11, 2022

⌛ Testing commit 931e7d61395b40f63ff5b617af97f0362ae36a1c with merge b202fd73bd2be73dc84df779c173596bd365d936...

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented May 11, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 11, 2022
@oli-obk
Copy link
Contributor

oli-obk commented May 11, 2022

r=me with rebase+bless

@cjgillot
Copy link
Contributor Author

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented May 11, 2022

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

@bors
Copy link
Contributor

bors commented May 11, 2022

📌 Commit 931e7d61395b40f63ff5b617af97f0362ae36a1c has been approved by oli-obk

@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 May 11, 2022
@cjgillot
Copy link
Contributor Author

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 11, 2022
@cjgillot
Copy link
Contributor Author

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented May 11, 2022

📌 Commit dacf118 has been approved by oli-obk

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 11, 2022
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 11, 2022
@bors
Copy link
Contributor

bors commented May 11, 2022

⌛ Testing commit dacf118 with merge cb9cb4d...

@bors
Copy link
Contributor

bors commented May 12, 2022

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing cb9cb4d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 12, 2022
@bors bors merged commit cb9cb4d into rust-lang:master May 12, 2022
@rustbot rustbot added this to the 1.62.0 milestone May 12, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (cb9cb4d): comparison url.

Summary: This benchmark run did not return any relevant results.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

@cjgillot cjgillot deleted the codegen-fulfill-nice branch May 12, 2022 09:35
@cjgillot cjgillot mentioned this pull request May 12, 2022
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 12, 2022
@matthiaskrgr
Rollup merge of rust-lang#96968 - cjgillot:codegen-fulfill-nice, r=ol…
5656ea2 
…i-obk

Add tests for rust-lang#96806

I messed up the rebase in rust-lang#96806.

I took the opportunity to add an extra mir-opt test from rust-lang#91743.

r? `@oli-obk`
@matthiaskrgr matthiaskrgr mentioned this pull request May 12, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 12, 2022
@bors
Auto merge of rust-lang#96974 - matthiaskrgr:rollup-jd4otnc, r=matthi…
c6499fd 
…askrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#95896 (Note the contacts for the nvptx64 target(s))
 - rust-lang#96860 (openbsd: convert futex timeout managment to Timespec usage)
 - rust-lang#96939 (Fix settings page CSS)
 - rust-lang#96941 (update graphviz links)
 - rust-lang#96968 (Add tests for rust-lang#96806)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@Alexendoo
Copy link
Member

From glacier: they all now compile without errors, in addition to the ones already closed by the PR the following no longer ICE

This was referenced Jun 11, 2022
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@oli-obk oli-obk

Labels
merged-by-bors This PR was explicitly merged by bors. 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
Milestone
1.62.0
8 participants
@cjgillot @oli-obk @bors @rust-log-analyzer @rust-timer @Alexendoo @rust-highfive @rustbot