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

don't ICE when normalizing closure input tys #99818

Merged
merged 1 commit into from
Oct 9, 2022

Conversation

aliemjay
Copy link
Member

@aliemjay aliemjay commented Jul 27, 2022

We were ICEing while rendering diagnostics because universe_causes is expected to track every universe created in the typeck's infcx.

normalize_and_add_constraints doesn't update universe_causes
when creating new universes, causing an ICE. Remove it!

Add spans to better track normalization constraints.

Fix couple places where universe_causes is not updated correctly to
track newly added universes.

Fixes #102800

Fixess #99665 (UPDATE: no longer true; the issue has a different failure path than when this PR was created and should be fixed by #101708, but the changes in this PR are still correct and should prevent potential future ICEs)

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

r? @estebank

(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 Jul 27, 2022
@rust-log-analyzer

This comment has been minimized.

@aliemjay aliemjay force-pushed the fix-closure-normalize branch 2 times, most recently from 26cccca to 4eae7a1 Compare July 27, 2022 20:22
@estebank
Copy link
Contributor

estebank commented Sep 1, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Sep 1, 2022

📌 Commit 4eae7a1 has been approved by estebank

It is now in the queue for this repository.

@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 Sep 1, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 2, 2022
…stebank

don't ICE when normalizing closure input tys

We were ICEing while rendering diagnostics because `universe_causes` is expected to track every universe created in the typeck's infcx.

`normalize_and_add_constraints` doesn't update `universe_causes`
when creating new universes, causing an ICE. Remove it!

Add spans to better track normalization constraints.

Fix couple places where `universe_causes` is not updated correctly to
track newly added universes.

Fixes rust-lang#99665
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 2, 2022
…stebank

don't ICE when normalizing closure input tys

We were ICEing while rendering diagnostics because `universe_causes` is expected to track every universe created in the typeck's infcx.

`normalize_and_add_constraints` doesn't update `universe_causes`
when creating new universes, causing an ICE. Remove it!

Add spans to better track normalization constraints.

Fix couple places where `universe_causes` is not updated correctly to
track newly added universes.

Fixes rust-lang#99665
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 2, 2022
…stebank

don't ICE when normalizing closure input tys

We were ICEing while rendering diagnostics because `universe_causes` is expected to track every universe created in the typeck's infcx.

`normalize_and_add_constraints` doesn't update `universe_causes`
when creating new universes, causing an ICE. Remove it!

Add spans to better track normalization constraints.

Fix couple places where `universe_causes` is not updated correctly to
track newly added universes.

Fixes rust-lang#99665
@GuillaumeGomez
Copy link
Member

It failed in #101315.

@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 Sep 2, 2022
@JohnTitor
Copy link
Member

Triage: #101708 fixed the issue, maybe we could close this PR?

@estebank
Copy link
Contributor

Closing based on the above comment.

@aliemjay apologies for this not landing this faster, and thank you for taking the time to try and fix this in the first place! In the future, feel free to ping me if I'm assigned as reviewer and take a long time to get to it ^_^

@aliemjay
Copy link
Member Author

aliemjay commented Sep 27, 2022

@bors r? @estebank

The changes made here are still valid even after #101708 has landed. See updated OP.

@estebank
Copy link
Contributor

Why are there some tests removed?

@aliemjay
Copy link
Member Author

Why are there some tests removed?

The tests were covered in #101708, so I thought they're unnecessary.

@estebank
Copy link
Contributor

@aliemjay I'm confused because the test introduced in that PR is check-pass while the ones added (and removed) here do not successfully compile, so it seemed to me that they were testing slightly different things

@aliemjay
Copy link
Member Author

aliemjay commented Oct 8, 2022

@estebank I've added another regression test #102800.

@rust-log-analyzer

This comment has been minimized.

`normalize_and_add_constraints` doesn't add entries in `universe_causes`
when creating new universes, causing an ICE. Remove it!

Add spans to track normalization constraints.

Fix couple places where `universe_causes` is not updated correctly to
track newly added universes.
@jackh726
Copy link
Member

jackh726 commented Oct 8, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Oct 8, 2022

📌 Commit fc3d7eb has been approved by jackh726

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 8, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 8, 2022
…ackh726

don't ICE when normalizing closure input tys

We were ICEing while rendering diagnostics because `universe_causes` is expected to track every universe created in the typeck's infcx.

`normalize_and_add_constraints` doesn't update `universe_causes`
when creating new universes, causing an ICE. Remove it!

Add spans to better track normalization constraints.

Fix couple places where `universe_causes` is not updated correctly to
track newly added universes.

Fixes rust-lang#102800

~Fixess rust-lang#99665~ (UPDATE: no longer true; the issue has a different failure path than when this PR was created and should be fixed by rust-lang#101708, but the changes in this PR are still correct and should prevent potential future ICEs)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 8, 2022
…ackh726

don't ICE when normalizing closure input tys

We were ICEing while rendering diagnostics because `universe_causes` is expected to track every universe created in the typeck's infcx.

`normalize_and_add_constraints` doesn't update `universe_causes`
when creating new universes, causing an ICE. Remove it!

Add spans to better track normalization constraints.

Fix couple places where `universe_causes` is not updated correctly to
track newly added universes.

Fixes rust-lang#102800

~Fixess rust-lang#99665~ (UPDATE: no longer true; the issue has a different failure path than when this PR was created and should be fixed by rust-lang#101708, but the changes in this PR are still correct and should prevent potential future ICEs)
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 9, 2022
Rollup of 8 pull requests

Successful merges:

 - rust-lang#99818 (don't ICE when normalizing closure input tys)
 - rust-lang#102514 (Don't repeat lifetime names from outer binder in print)
 - rust-lang#102661 (rustdoc: Document effect of fundamental types)
 - rust-lang#102782 (Add regression test for rust-lang#102124)
 - rust-lang#102790 (Fix llvm-tblgen for cross compiling)
 - rust-lang#102807 (Document `rust-docs-json` component)
 - rust-lang#102812 (Remove empty core::lazy and std::lazy)
 - rust-lang#102818 (Clean up rustdoc highlight.rs imports a bit)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit bf37054 into rust-lang:master Oct 9, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 9, 2022
@aliemjay aliemjay deleted the fix-closure-normalize branch October 9, 2022 12:42
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.

borrowck: no entry for key revived
9 participants