Repository navigation
fix unsoundness when compiling [diverging_const; 0] - #165416
rust-bors[bot] merged 2 commits into
Conversation
Previusly we had a disagreement between hir_typeck (assuming that anything following the diverging constant is unreachable) and mir_build (assuming we don't need to evaluate constants in `[...; 0]`). This (temporarily) fixes this by making hir_typeck aware that such constants should not affect divergence / reachibility analysis
|
r? @wesleywiser rustbot has assigned @wesleywiser. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
Does this need a crater run, and does this need to be beta-nominated? |
|
Nominating for a beta backport. This fixes an unsoundness with the never type, that is currently stable in beta (and which will be stable in ~9 days). |
|
@theemathas I don't think this needs a crater run; this can only move code from being unsound to not compiling (and you can only write the unsound code by mentioning |
| let count_is_zero = { | ||
| if let Ok(count) = tcx.try_normalize_erasing_regions( | ||
| self.typing_env(self.param_env), | ||
| Unnormalized::new_wip(count), | ||
| ) { |
There was a problem hiding this comment.
It's unclear to me if this does anything / is needed, given the self.normalize call above. cc @khyperia
There was a problem hiding this comment.
yea I think just running the try_to_value method chan below would suffice, no need to try to reevaluate
|
isn't this the wrong way around? Shouldn't we instead eval the const (or at least eval it if it's uninhabited)? We put a lot of effort into making sure all runtime-reachable const blocks or uses of const items are evaluated, even if optimized out. Edit: ah this was discussed on the issue, could have still just done that for constants of type |
There was a problem hiding this comment.
While I would prefer to ensure the consts aren't dropped on the floor by mir building, this change is ready and can be merged now, and is strictly rejecting more code than fixing the issue properly.
@bors r+ p=1
|
@bors r=oli-obk force |
…ted_zero_times, r=oli-obk fix unsoundness when compiling `[diverging_const; 0]` As I described in rust-lang#165323 (comment), currently `[diverging_const; 0]` leads to usoundness because hir type check and mir building disagree on whatever `diverging_const` is evaluated or not. hir type check assumes that it does (and marks anything after it as unreachable) while mir building sees that the constant is repeated 0 times and decides to skip evaluating the constant, resulting in unsoundness. This PR fixes this by doing the same kind of special case in hir type check. Fixes rust-lang#165323
|
Should #165323 be kept open after this PR merges, since it seems like the behavior isn't finalized yet? |
…uwer Rollup of 21 pull requests Successful merges: - #165416 (fix unsoundness when compiling `[diverging_const; 0]`) - #149245 (Make inline attributes apply to the generated `poll` in async fn) - #163495 (Parallelize AST -> HIR lowering) - #164304 ([perf] Reuse CanonicalVarKinds if no new variables were added) - #165109 (Avoid building a region map when erasing bound regions) - #163393 (std: Use getexecpath() for current_exe() on OpenBSD) - #163625 (Add a lint helper attribute namespace ) - #164414 (Add `let_underscore_drop_guard` lint.) - #165342 (Avoid unnecessary MIR traversals in CheckPackedRef lint) - #165351 (delegation: insert probing adjustments before args check adjustments) - #165358 (Reenable some cg_gcc tests that were disabled) - #165377 (Make `no_implicit_prelude` not suppress tool attributes) - #165397 (Add codegen test for unused match arms removed after inlining) - #165398 (Rework parallel reproducibility tests) - #162608 (Avoid oscillating line wrapping due to symbol indices) - #164241 (make the safety documentation consistent in Ptr module) - #165029 (Use `MalformedLintAttributeInput` in more places) - #165203 (AIX: Implement TLS destructor guard in std and update dynamic-loading-cdylib test) - #165230 (Fix rigidity assertion ICE) - #165293 (unstable-book: update default-visibility.md to link to tracking issue) - #165374 (Refactorings in preparation for the move trait)
|
⌛ Testing commit 23d29d9 with merge 69e1d01... Workflow: https://github.com/rust-lang/rust/actions/runs/41696987455 |
…mes, r=oli-obk fix unsoundness when compiling `[diverging_const; 0]` As I described in #165323 (comment), currently `[diverging_const; 0]` leads to usoundness because hir type check and mir building disagree on whatever `diverging_const` is evaluated or not. hir type check assumes that it does (and marks anything after it as unreachable) while mir building sees that the constant is repeated 0 times and decides to skip evaluating the constant, resulting in unsoundness. This PR fixes this by doing the same kind of special case in hir type check. Fixes (but does not close as this is a temporary workaround) #165323
|
💔 I suspect this PR failed tests as part of a rollup After fixing the problem, consider running a try job for the failed job before re-approving. Link to failure: #165418 (comment) |
|
This pull request was unapproved. This PR was contained in a rollup (#165418), which was unapproved. Auto build was cancelled due to unapproval. Cancelled workflows: |
This comment has been minimized.
This comment has been minimized.
…mes, r=<try> fix unsoundness when compiling `[diverging_const; 0]` try-job: test-x86_64-gnu-llvm-22-3
|
@bors r=oli-obk force |
…uwer Rollup of 7 pull requests Successful merges: - #165416 (fix unsoundness when compiling `[diverging_const; 0]`) - #163217 ([perf] rustdoc: Avoid re-HIR-ty-lowering tys & const args where possible) - #165371 (Don't use `llvm_type` as the type of `static`s.) - #165354 (Remove `AbiAlign`) - #165357 (rustdoc: Fix invalid clean of paths when they end with `self`) - #165401 (Add codegen test for step_by(1) loop vectorization) - #165408 (LLVMRustWriteOutputFile: Check the result of runCodeGenPipeline)
Rollup merge of #165416 - WaffleLapkin:diverging_const_repeated_zero_times, r=oli-obk fix unsoundness when compiling `[diverging_const; 0]` As I described in #165323 (comment), currently `[diverging_const; 0]` leads to usoundness because hir type check and mir building disagree on whatever `diverging_const` is evaluated or not. hir type check assumes that it does (and marks anything after it as unreachable) while mir building sees that the constant is repeated 0 times and decides to skip evaluating the constant, resulting in unsoundness. This PR fixes this by doing the same kind of special case in hir type check. Fixes (but does not close as this is a temporary workaround) #165323
|
beta backport approved as per compiler team on Zulip. A backport PR will be authored by the release team at the end of the current development cycle. Backport labels are handled by them. |
…uwer Rollup of 7 pull requests Successful merges: - rust-lang/rust#165416 (fix unsoundness when compiling `[diverging_const; 0]`) - rust-lang/rust#163217 ([perf] rustdoc: Avoid re-HIR-ty-lowering tys & const args where possible) - rust-lang/rust#165371 (Don't use `llvm_type` as the type of `static`s.) - rust-lang/rust#165354 (Remove `AbiAlign`) - rust-lang/rust#165357 (rustdoc: Fix invalid clean of paths when they end with `self`) - rust-lang/rust#165401 (Add codegen test for step_by(1) loop vectorization) - rust-lang/rust#165408 (LLVMRustWriteOutputFile: Check the result of runCodeGenPipeline)
View all comments
As I described in #165323 (comment), currently
[diverging_const; 0]leads to usoundness because hir type check and mir building disagree on whateverdiverging_constis evaluated or not. hir type check assumes that it does (and marks anything after it as unreachable) while mir building sees that the constant is repeated 0 times and decides to skip evaluating the constant, resulting in unsoundness.This PR fixes this by doing the same kind of special case in hir type check.
Fixes (but does not close as this is a temporary workaround) #165323