Skip to content

put fake reads for guards' fake borrows on the guard's failure path - #161581

Open
dianne wants to merge 1 commit into
rust-lang:mainfrom
dianne:otherwise-fake-read
Open

put fake reads for guards' fake borrows on the guard's failure path#161581
dianne wants to merge 1 commit into
rust-lang:mainfrom
dianne:otherwise-fake-read

Conversation

@dianne

@dianne dianne commented Aug 23, 2026

Copy link
Copy Markdown
Member

Conceptually, I think it makes sense to have fake reads be on guard failure, since it's the failure path where we continue matching (and thus it's soundness-critical to prevent mutation). Practically, this fixes #161578.

This is technically a breaking change, so it'll need a crater run and FCP.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 23, 2026
@dianne
dianne force-pushed the otherwise-fake-read branch from 2053636 to 6579371 Compare August 23, 2026 09:10
@dianne

dianne commented Aug 23, 2026

Copy link
Copy Markdown
Member Author

@bors try

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Aug 23, 2026
put fake reads for guards' fake borrows on the guard's failure path
@dianne dianne added needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. needs-crater This change needs a crater run to check for possible breakage in the ecosystem. labels Aug 23, 2026
@dianne

dianne commented Aug 23, 2026

Copy link
Copy Markdown
Member Author

I'm guessing a types fcp would make the most sense since this is kind of a borrow-checking change?

r? @oli-obk maybe since you're familiar with both MIR building and T-types procedure, but feel free to reassign ^^

@dianne
dianne marked this pull request as ready for review August 23, 2026 09:23
@rustbot

rustbot commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

oli-obk is not on the review rotation at the moment.
They may take a while to respond.

@rustbot

rustbot commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in match lowering

cc @Nadrieril

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 23, 2026
@rust-log-analyzer

This comment has been minimized.

@dianne
dianne force-pushed the otherwise-fake-read branch from 6579371 to 80441e9 Compare August 23, 2026 09:53
@rustbot

rustbot commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in coverage tests.

cc @Zalathar

@rust-bors

rust-bors Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 8511135 (85111357388f7cbb75083e86a3f3a1cc76d512fb)
Base parent: baf1f7c (baf1f7cc4faa79b938148ad52c53e96c0102cbc4)

@dianne

dianne commented Aug 23, 2026

Copy link
Copy Markdown
Member Author

@craterbot check

@craterbot

Copy link
Copy Markdown
Collaborator

👌 Experiment pr-161581 created and queued.
🤖 Automatically detected try build 8511135
⚠️ Try build based on commit 6579371, but latest commit is 80441e9. Did you forget to make a new try build?
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 23, 2026
@dianne

dianne commented Aug 23, 2026

Copy link
Copy Markdown
Member Author

just noticed oli is on vacation; oops! @rustbot reroll

@rustbot rustbot assigned chenyukang and unassigned oli-obk Aug 23, 2026
@Zalathar

Copy link
Copy Markdown
Member

You will also need to bless the coverage-run tests with ./x test coverage --bless --set=build.profiler=true.

(You can also [build] profiler = true in your bootstrap.toml to make running/blessing the coverage-run tests work by default.)

I'm not sure whether the resulting changes to coverage output are a problem or not, but my hope is that #161517 will make them vanish anyway.

@dianne
dianne force-pushed the otherwise-fake-read branch from 80441e9 to e20b342 Compare August 23, 2026 13:13
@dianne

dianne commented Aug 23, 2026

Copy link
Copy Markdown
Member Author

the coverage-run tests should be blessed now. I'm not familiar with how coverage uses spans (or how to read coverage tests), so if there is a problem I'll need some guidance. though I imagine #161517 should help if it means coverage won't scrape spans from fake reads anymore

@dianne
dianne force-pushed the otherwise-fake-read branch from e20b342 to 3990980 Compare August 23, 2026 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-crater This change needs a crater run to check for possible breakage in the ecosystem. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. S-waiting-on-crater Status: Waiting on a crater run to be completed. 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.

match guards with unreachable success blocks ignore fake borrows

7 participants