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

Check that all hidden types are the same and then deduplicate them. #95731

Merged
merged 1 commit into from
Apr 6, 2022

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Apr 6, 2022

fixes #95538

This used to trigger a sanity check. Now we accept that there may be multiple places where a hidden type is constrained and we merge all of these at the end.

Ideally we'd merge eagerly, but that is a larger refactoring that I don't want to put into a backport

@oli-obk oli-obk added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Apr 6, 2022
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 6, 2022
@rust-highfive
Copy link
Collaborator

r? @compiler-errors

(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 Apr 6, 2022
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

r=me with or without comment addressed

// once we convert the generic parameters to those of the opaque type.
if let Some(prev) = result.get_mut(&opaque_type_key) {
if prev.ty != ty {
let mut err = infcx.tcx.sess.struct_span_err(
Copy link
Member

Choose a reason for hiding this comment

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

Is this an error that we can trigger from code? If so, could you add a unit test (if it's easy to reproduce)?

Otherwise, maybe we can bug (or delay a bug) here instead if it should never happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should with type-alias-impl-trait, but due to a funky bug it never happens in practice. I'll prep a second PR that we won't backport that does this.

Until then, I'll leave in this error just so in case I'm wrong, we get an error instead of an ICE on beta.

@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 6, 2022

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Apr 6, 2022

📌 Commit 27dc503 has been approved by compiler-errors

@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 Apr 6, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 6, 2022
…piler-errors

Check that all hidden types are the same and then deduplicate them.

fixes rust-lang#95538

This used to trigger a sanity check. Now we accept that there may be multiple places where a hidden type is constrained and we merge all of these at the end.

Ideally we'd merge eagerly, but that is a larger refactoring that I don't want to put into a backport
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 6, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#95342 (Ignore "format the world" commit in git blame)
 - rust-lang#95353 ([bootstrap] Give a hard error when filtering tests for a file that does not exist)
 - rust-lang#95649 (New mir-opt deref_separator)
 - rust-lang#95721 (Fix typo in bootstrap/setup.rs)
 - rust-lang#95730 (Rename RWLock to RwLock in std::sys.)
 - rust-lang#95731 (Check that all hidden types are the same and then deduplicate them.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ebba894 into rust-lang:master Apr 6, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 6, 2022
@apiraino
Copy link
Contributor

apiraino commented Apr 7, 2022

Beta backport decision postponed to next week, this patch was merged just a few hours ago, so there are not yet perf results to look at.

(Zulip discussion)

@oli-obk oli-obk deleted the lazy_tait_regression branch April 7, 2022 15:15
@apiraino
Copy link
Contributor

Beta backport approved as per compiler team on Zulip

Adding a note from the meeting: a follow-up patch should check the perf. results which were shadowed by other PRs in the rollup merge

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Apr 14, 2022
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Apr 24, 2022
@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.62.0, 1.61.0 Apr 24, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 25, 2022
…ulacrum

[beta] backports rollup

*  Remove NodeIdHashingMode. rust-lang#95656
*  Check that all hidden types are the same and then deduplicate them. rust-lang#95731

r? `@Mark-Simulacrum`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. 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.

Nightly 2022-03-30 ICE: Collection VecMap(...) should have just one matching element
7 participants