Skip to content

EUV: fix place of deref pattern's interior's scrutinee #138442

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
merged 1 commit into from
Mar 14, 2025

Conversation

dianne
Copy link
Contributor

@dianne dianne commented Mar 13, 2025

The place previously used here was that of the temporary holding the reference returned by Deref::deref or DerefMut::deref_mut. However, since the inner pattern of deref!(inner) expects the deref-target type itself, this would ICE when that type was inspected (e.g. by the EUV case for slice patterns). This adds a deref projection to fix that.

Since current in-tree consumers of EUV (upvar inference and clippy) don't care about Rvalues, the place could be simplified to self.cat_rvalue(pat.hir_id, self.pat_ty_adjusted(subpat)?) to save some cycles. I personally find EUV to be a bit fragile, so I've opted for pedantic correctness. Maybe a HACK comment would suffice though?

Fixes #125059

r? @compiler-errors

@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 Mar 13, 2025
@compiler-errors
Copy link
Member

compiler-errors commented Mar 13, 2025

Can you give this a better test name? I don't think it's describing much about what was going on here.

@compiler-errors
Copy link
Member

Also can you copy the commit description into the PR description? Given this is a one commit PR and all, it's a bit of indirection to tell what this does lol

@@ -1,6 +1,8 @@
//@ known-bug: rust-lang/rust#125059
//@ run-pass
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//@ run-pass
//@ check-pass

Given that this doesn't actually execute anything meaningful

The place previously used here was that of the temporary holding the
reference returned by `Deref::deref` or `DerefMut::deref_mut`. However,
since the inner pattern of `deref!(inner)` expects the deref-target type
itself, this would ICE when that type was inspected (e.g. by the EUV
case for slice patterns). This adds a deref projection to fix that.

Since current in-tree consumers of EUV (upvar inference and clippy)
don't care about Rvalues, the place could be simplified to
`self.cat_rvalue(pat.hir_id, self.pat_ty_adjusted(subpat)?)` to save
some cycles. I personally find EUV to be a bit fragile, so I've opted
for pedantic correctness. Maybe a `HACK` comment would suffice though?
@dianne dianne force-pushed the deref-pat-euv-fix branch from 2c0889c to 36ff87e Compare March 13, 2025 08:04
@rustbot
Copy link
Collaborator

rustbot commented Mar 13, 2025

This PR changes a file inside tests/crashes. If a crash was fixed, please move into the corresponding ui subdir and add 'Fixes #' to the PR description to autoclose the issue upon merge.

@dianne
Copy link
Contributor Author

dianne commented Mar 13, 2025

I've amended the test with your suggestions (plus a little extra description in the test comment) and edited the PR description. ty!

@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Mar 13, 2025

📌 Commit 36ff87e has been approved by compiler-errors

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 Mar 13, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 14, 2025
Rollup of 16 pull requests

Successful merges:

 - rust-lang#136001 (Overhaul examples for PermissionsExt)
 - rust-lang#136230 (Reword incorrect documentation about SocketAddr having varying layout)
 - rust-lang#136892 (Sync Fuchsia target spec with clang Fuchsia driver)
 - rust-lang#136911 (Add documentation URL to selected jobs)
 - rust-lang#137870 ( Improve HashMap docs for const and static initializers)
 - rust-lang#138179 (Add `src/tools/x` to the main workspace)
 - rust-lang#138389 (use `expect` instead of `allow`)
 - rust-lang#138396 (Enable metrics and verbose tests in PR CI)
 - rust-lang#138398 (atomic intrinsics: clarify which types are supported and (if applicable) what happens with provenance)
 - rust-lang#138432 (fix: remove the check of lld not supporting `@response-file)`
 - rust-lang#138434 (Visit `PatField` when collecting lint levels)
 - rust-lang#138441 (update error message)
 - rust-lang#138442 (EUV: fix place of deref pattern's interior's scrutinee)
 - rust-lang#138457 (Remove usage of legacy scheme paths on RedoxOS)
 - rust-lang#138461 (Remove an outdated line from a test comment)
 - rust-lang#138466 (Remove myself from libs review)

Failed merges:

 - rust-lang#138452 (Remove `RUN_CHECK_WITH_PARALLEL_QUERIES`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f505572 into rust-lang:master Mar 14, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 14, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 14, 2025
Rollup merge of rust-lang#138442 - dianne:deref-pat-euv-fix, r=compiler-errors

EUV: fix place of deref pattern's interior's scrutinee

The place previously used here was that of the temporary holding the reference returned by `Deref::deref` or `DerefMut::deref_mut`. However, since the inner pattern of `deref!(inner)` expects the deref-target type itself, this would ICE when that type was inspected (e.g. by the EUV case for slice patterns). This adds a deref projection to fix that.

Since current in-tree consumers of EUV (upvar inference and clippy) don't care about Rvalues, the place could be simplified to `self.cat_rvalue(pat.hir_id, self.pat_ty_adjusted(subpat)?)` to save some cycles. I personally find EUV to be a bit fragile, so I've opted for pedantic correctness. Maybe a `HACK` comment would suffice though?

Fixes rust-lang#125059

r? `@compiler-errors`
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Mar 19, 2025
Rollup of 16 pull requests

Successful merges:

 - rust-lang#136001 (Overhaul examples for PermissionsExt)
 - rust-lang#136230 (Reword incorrect documentation about SocketAddr having varying layout)
 - rust-lang#136892 (Sync Fuchsia target spec with clang Fuchsia driver)
 - rust-lang#136911 (Add documentation URL to selected jobs)
 - rust-lang#137870 ( Improve HashMap docs for const and static initializers)
 - rust-lang#138179 (Add `src/tools/x` to the main workspace)
 - rust-lang#138389 (use `expect` instead of `allow`)
 - rust-lang#138396 (Enable metrics and verbose tests in PR CI)
 - rust-lang#138398 (atomic intrinsics: clarify which types are supported and (if applicable) what happens with provenance)
 - rust-lang#138432 (fix: remove the check of lld not supporting `@response-file)`
 - rust-lang#138434 (Visit `PatField` when collecting lint levels)
 - rust-lang#138441 (update error message)
 - rust-lang#138442 (EUV: fix place of deref pattern's interior's scrutinee)
 - rust-lang#138457 (Remove usage of legacy scheme paths on RedoxOS)
 - rust-lang#138461 (Remove an outdated line from a test comment)
 - rust-lang#138466 (Remove myself from libs review)

Failed merges:

 - rust-lang#138452 (Remove `RUN_CHECK_WITH_PARALLEL_QUERIES`)

r? `@ghost`
`@rustbot` modify labels: rollup
# 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: explicit index of non-indexable type
4 participants