Document consteval behavior of ub_checks, overflow_checks, is_val_statically_known.#154140
Conversation
rustbot
added
S-waiting-on-review
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
|
r? me |
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
theemathas
force-pushed
the
checks_always_on_in_const
branch
from
8c10a41 to
72958b5
Compare
theemathas
changed the title
|
I've addressed the comments. While we're at it, I also documented @rustbot ready |
rustbot
added
S-waiting-on-review
library/core/src/intrinsics/mod.rs
Outdated
| /// is currently done in consteval. Note that this function must always return | ||
| /// the same value for any given call site in consteval, for soundness. | ||
| /// For an example of an unsoundness that nondeterministic consteval could cause, | ||
| /// see [#148328](https://github.com/rust-lang/rust/issues/148328). |
There was a problem hiding this comment.
I don't think we should say anything about what is_val_statically_known does in const-eval. It's an implementation detail, we can change it any time, users should never care.
There was a problem hiding this comment.
Should I undo the changes for is_val_statically_known, and leave the old documentation as-is?
There was a problem hiding this comment.
I think removing the old section is fine. We have meanwhile accepted non-determinism in const fn by stabilizing float operations there, this isn't really any different.
library/core/src/intrinsics/mod.rs
Outdated
| /// In consteval, this function currently returns `true`. This is because the value of the `ub_checks` | ||
| /// configuration can differ across crates, but we need this function to always return the same | ||
| /// value in consteval in order to avoid unsoundness. For an example of an unsoundness that | ||
| /// nondeterministic consteval could cause, see [#148328](https://github.com/rust-lang/rust/issues/148328). |
There was a problem hiding this comment.
IMO it's a waste of space to describe this soundness issues here -- it's relevant to implementors of the intrinsic, but not to users, so if we have such a comment it makes much more sense at the implementation than here (and I think we already have a comment at the implementation).
But I don't have a strong opinion here so if for some reason you really want to keep this here then 🤷 whatever.
We've already stabilized float operations in const, which means we've already accepted that a `const fn` might behave differently in consteval vs at run time.
theemathas
force-pushed
the
checks_always_on_in_const
branch
from
72958b5 to
338deef
Compare
|
Hopefully this PR should be good to go now? |
|
Thanks. :) @bors r+ rollup |
rust-bors
bot
added
S-waiting-on-bors
…st, r=RalfJung Document consteval behavior of ub_checks, overflow_checks, is_val_statically_known.
…st, r=RalfJung Document consteval behavior of ub_checks, overflow_checks, is_val_statically_known.
…uwer Rollup of 13 pull requests Successful merges: - #154241 (`rust-analyzer` subtree update) - #153686 (`std`: include `dlmalloc` for all non-wasi Wasm targets) - #154105 (bootstrap: Pass `--features=rustc` to rustc_transmute) - #153069 ([BPF] add target feature allows-misaligned-mem-access) - #154085 (Parenthesize or-patterns in prefix pattern positions in pretty printer) - #154191 (refactor RangeFromIter overflow-checks impl) - #154207 (Refactor query loading) - #153540 (drop derive helpers during attribute parsing) - #154140 (Document consteval behavior of ub_checks, overflow_checks, is_val_statically_known.) - #154161 (On E0277 tweak help when single type impls traits) - #154218 (interpret/validity: remove unreachable error kind) - #154225 (diagnostics: avoid ICE in confusable_method_name for associated functions) - #154228 (Improve inline assembly error messages)
6 participants
No description provided.