Skip to content

remove an ineffective check in const_prop #100239

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 2 commits into from
Aug 29, 2022

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Aug 7, 2022

Based on #100043, only the last two commits are new.

ConstProp has a special check when reading from a local that prevents reading uninit locals. However, if that local flows into force_allocation, then no check fires and evaluation proceeds. So this check is not really effective at preventing accesses to uninit locals.

With #100043, read_immediate and friends always fail when reading uninit locals, so I don't see why ConstProp would need a separate check. Thus I propose we remove it. This is needed to be able to do #100085.

@rustbot
Copy link
Collaborator

rustbot commented Aug 7, 2022

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

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

r? @jackh726

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

This comment has been minimized.

@RalfJung RalfJung force-pushed the const-prop-uninit branch 2 times, most recently from b3c5a09 to 48e0184 Compare August 7, 2022 17:25
@jackh726
Copy link
Member

jackh726 commented Aug 7, 2022

Hmm, maybe r? @oli-obk would be a good choice here? Feel free to reassign.

@rust-highfive rust-highfive assigned oli-obk and unassigned jackh726 Aug 7, 2022
@bors
Copy link
Collaborator

bors commented Aug 12, 2022

☔ The latest upstream changes (presumably #100456) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Collaborator

bors commented Aug 27, 2022

☔ The latest upstream changes (presumably #100043) made this pull request unmergeable. Please resolve the merge conflicts.

@RalfJung
Copy link
Member Author

@oli-obk this should be ready for review now.

@oli-obk
Copy link
Contributor

oli-obk commented Aug 29, 2022

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 29, 2022

📌 Commit aff9841 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 Aug 29, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 29, 2022
Rollup of 8 pull requests

Successful merges:

 - rust-lang#98304 (Add MaybeUninit memset test)
 - rust-lang#98801 (Add a `File::create_new` constructor)
 - rust-lang#99821 (Remove separate indexing of early-bound regions)
 - rust-lang#100239 (remove an ineffective check in const_prop)
 - rust-lang#100337 (Stabilize `std::io::read_to_string`)
 - rust-lang#100819 (Make use of `[wrapping_]byte_{add,sub}`)
 - rust-lang#100934 (Remove a panicking branch from `fmt::builders::PadAdapter`)
 - rust-lang#101000 (Separate CountIsStar from CountIsParam in rustc_parse_format.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3ea5456 into rust-lang:master Aug 29, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 29, 2022
@RalfJung RalfJung deleted the const-prop-uninit branch August 31, 2022 10:45
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 21, 2022
…, r=oli-obk

fix ConstProp handling of written_only_inside_own_block_locals

Fixes a regression introduced by rust-lang#100239, which adds an early return and thus skips some code in `visit_terminator` that must be run for soundness.

Fixes rust-lang#101973
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 21, 2022
…, r=oli-obk

fix ConstProp handling of written_only_inside_own_block_locals

Fixes a regression introduced by rust-lang#100239, which adds an early return and thus skips some code in `visit_terminator` that must be run for soundness.

Fixes rust-lang#101973
# 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.

7 participants