Skip to content

Fix trait upcasting to dyn type with no principal when there are projections #139421

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged

Conversation

compiler-errors
Copy link
Member

#126660 (which I had originally authored, lol) had a subtle bug that is the moral equivalent of #114036, which is that when upcasting from dyn Principal<Projection = Ty> + AutoTrait to dyn AutoTrait, we were dropping the trait ref for Principal but not its projections (if there were any).

With debug assertions enabled, this triggers the assertion I luckily added in a2a0cfe, but even without debug assertions this is a logical bug since we had a dyn type with just a projection bound but no principal, so it caused a type mismatch.

This does not need an FCP because this should've been covered by the FCP in #126660, but we just weren't testing a case when casting from a dyn type with projections 😸

Fixes #139418

r? @oli-obk (or anyone)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 5, 2025
@oli-obk
Copy link
Contributor

oli-obk commented Apr 7, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 7, 2025

📌 Commit 45afefa has been approved by oli-obk

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 Apr 7, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 7, 2025
…l-with-proj, r=oli-obk

Fix trait upcasting to dyn type with no principal when there are projections

rust-lang#126660 (which I had originally authored, lol) had a subtle bug that is the moral equivalent of rust-lang#114036, which is that when upcasting from `dyn Principal<Projection = Ty> + AutoTrait` to `dyn AutoTrait`, we were dropping the trait ref for `Principal` but not its projections (if there were any).

With debug assertions enabled, this triggers the assertion I luckily added in a2a0cfe, but even without debug assertions this is a logical bug since we had a dyn type with just a projection bound but no principal, so it caused a type mismatch.

This does not need an FCP because this should've been covered by the FCP in rust-lang#126660, but we just weren't testing a case when casting from a `dyn` type with projections 😸

Fixes rust-lang#139418

r? `@oli-obk` (or anyone)
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 7, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#139124 (compiler: report error when trait object type param reference self)
 - rust-lang#139346 (Don't construct preds w escaping bound vars in `diagnostic_hir_wf_check`)
 - rust-lang#139379 (Use delayed bug for normalization errors in drop elaboration)
 - rust-lang#139421 (Fix trait upcasting to dyn type with no principal when there are projections)
 - rust-lang#139468 (Don't call `Span::with_parent` on the good path in `has_stashed_diagnostic`)
 - rust-lang#139476 (rm `RegionInferenceContext::var_infos`)
 - rust-lang#139490 (Update some comment/docs related to "extern intrinsic" removal)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 7, 2025
…l-with-proj, r=oli-obk

Fix trait upcasting to dyn type with no principal when there are projections

rust-lang#126660 (which I had originally authored, lol) had a subtle bug that is the moral equivalent of rust-lang#114036, which is that when upcasting from `dyn Principal<Projection = Ty> + AutoTrait` to `dyn AutoTrait`, we were dropping the trait ref for `Principal` but not its projections (if there were any).

With debug assertions enabled, this triggers the assertion I luckily added in a2a0cfe, but even without debug assertions this is a logical bug since we had a dyn type with just a projection bound but no principal, so it caused a type mismatch.

This does not need an FCP because this should've been covered by the FCP in rust-lang#126660, but we just weren't testing a case when casting from a `dyn` type with projections 😸

Fixes rust-lang#139418

r? ``@oli-obk`` (or anyone)
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 8, 2025
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#139124 (compiler: report error when trait object type param reference self)
 - rust-lang#139346 (Don't construct preds w escaping bound vars in `diagnostic_hir_wf_check`)
 - rust-lang#139421 (Fix trait upcasting to dyn type with no principal when there are projections)
 - rust-lang#139468 (Don't call `Span::with_parent` on the good path in `has_stashed_diagnostic`)
 - rust-lang#139476 (rm `RegionInferenceContext::var_infos`)
 - rust-lang#139490 (Update some comment/docs related to "extern intrinsic" removal)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 8, 2025
…l-with-proj, r=oli-obk

Fix trait upcasting to dyn type with no principal when there are projections

rust-lang#126660 (which I had originally authored, lol) had a subtle bug that is the moral equivalent of rust-lang#114036, which is that when upcasting from `dyn Principal<Projection = Ty> + AutoTrait` to `dyn AutoTrait`, we were dropping the trait ref for `Principal` but not its projections (if there were any).

With debug assertions enabled, this triggers the assertion I luckily added in a2a0cfe, but even without debug assertions this is a logical bug since we had a dyn type with just a projection bound but no principal, so it caused a type mismatch.

This does not need an FCP because this should've been covered by the FCP in rust-lang#126660, but we just weren't testing a case when casting from a `dyn` type with projections 😸

Fixes rust-lang#139418

r? ```@oli-obk``` (or anyone)
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 8, 2025
…errors

Rollup of 19 pull requests

Successful merges:

 - rust-lang#138676 (Implement overflow for infinite implied lifetime bounds)
 - rust-lang#139024 (Make error message for missing fields with `..` and without `..` more consistent)
 - rust-lang#139098 (Tell LLVM about impossible niche tags)
 - rust-lang#139124 (compiler: report error when trait object type param reference self)
 - rust-lang#139321 (Update to new rinja version (askama))
 - rust-lang#139346 (Don't construct preds w escaping bound vars in `diagnostic_hir_wf_check`)
 - rust-lang#139386 (make it possible to use stage0 libtest on compiletest)
 - rust-lang#139421 (Fix trait upcasting to dyn type with no principal when there are projections)
 - rust-lang#139468 (Don't call `Span::with_parent` on the good path in `has_stashed_diagnostic`)
 - rust-lang#139476 (rm `RegionInferenceContext::var_infos`)
 - rust-lang#139481 (Add job summary links to post-merge report)
 - rust-lang#139485 (compiletest: Stricter parsing for diagnostic kinds)
 - rust-lang#139490 (Update some comment/docs related to "extern intrinsic" removal)
 - rust-lang#139491 (Update books)
 - rust-lang#139496 (Revert r-a changes of rust-lang#139455)
 - rust-lang#139500 (document panic behavior of Vec::resize and Vec::resize_with)
 - rust-lang#139501 (Fix stack overflow in exhaustiveness due to recursive HIR opaque hidden types)
 - rust-lang#139504 (add missing word in doc comment)
 - rust-lang#139507 (compiletest: Trim whitespace from environment variable names)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 8, 2025
Rollup of 10 pull requests

Successful merges:

 - rust-lang#138676 (Implement overflow for infinite implied lifetime bounds)
 - rust-lang#139024 (Make error message for missing fields with `..` and without `..` more consistent)
 - rust-lang#139098 (Tell LLVM about impossible niche tags)
 - rust-lang#139124 (compiler: report error when trait object type param reference self)
 - rust-lang#139321 (Update to new rinja version (askama))
 - rust-lang#139346 (Don't construct preds w escaping bound vars in `diagnostic_hir_wf_check`)
 - rust-lang#139386 (make it possible to use stage0 libtest on compiletest)
 - rust-lang#139421 (Fix trait upcasting to dyn type with no principal when there are projections)
 - rust-lang#139464 (Allow for reparsing failure when reparsing a pasted metavar.)
 - rust-lang#139490 (Update some comment/docs related to "extern intrinsic" removal)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 056756c into rust-lang:master Apr 8, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 8, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 8, 2025
Rollup merge of rust-lang#139421 - compiler-errors:upcast-no-principal-with-proj, r=oli-obk

Fix trait upcasting to dyn type with no principal when there are projections

rust-lang#126660 (which I had originally authored, lol) had a subtle bug that is the moral equivalent of rust-lang#114036, which is that when upcasting from `dyn Principal<Projection = Ty> + AutoTrait` to `dyn AutoTrait`, we were dropping the trait ref for `Principal` but not its projections (if there were any).

With debug assertions enabled, this triggers the assertion I luckily added in a2a0cfe, but even without debug assertions this is a logical bug since we had a dyn type with just a projection bound but no principal, so it caused a type mismatch.

This does not need an FCP because this should've been covered by the FCP in rust-lang#126660, but we just weren't testing a case when casting from a `dyn` type with projections 😸

Fixes rust-lang#139418

r? ````@oli-obk```` (or anyone)
# for free to join this conversation on GitHub. Already have an account? # 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.

ICE: expected Binder to have 0 projections, but it has 1
4 participants