Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document is_val_statically_known only supports primitives (ICE otherwise) #121115

Closed
nyurik opened this issue Feb 14, 2024 · 8 comments · Fixed by #124502
Closed

Document is_val_statically_known only supports primitives (ICE otherwise) #121115

nyurik opened this issue Feb 14, 2024 · 8 comments · Fixed by #124502
Assignees
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools F-core_intrinsics Issue in the "core intrinsics" for internal usage only. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@nyurik
Copy link
Contributor

nyurik commented Feb 14, 2024

Location

/// Returns whether the argument's value is statically known at
/// compile-time.
///
/// This is useful when there is a way of writing the code that will
/// be *faster* when some variables have known values, but *slower*
/// in the general case: an `if is_val_statically_known(var)` can be used
/// to select between these two variants. The `if` will be optimized away
/// and only the desired branch remains.
///
/// Formally speaking, this function non-deterministically returns `true`
/// or `false`, and the caller has to ensure sound behavior for both cases.
/// In other words, the following code has *Undefined Behavior*:
///
/// ```no_run
/// #![feature(is_val_statically_known)]
/// #![feature(core_intrinsics)]
/// # #![allow(internal_features)]
/// use std::hint::unreachable_unchecked;
/// use std::intrinsics::is_val_statically_known;
///
/// unsafe {
/// if !is_val_statically_known(0) { unreachable_unchecked(); }
/// }
/// ```
///
/// This also means that the following code's behavior is unspecified; it
/// may panic, or it may not:
///
/// ```no_run
/// #![feature(is_val_statically_known)]
/// #![feature(core_intrinsics)]
/// # #![allow(internal_features)]
/// use std::intrinsics::is_val_statically_known;
///
/// unsafe {
/// assert_eq!(is_val_statically_known(0), is_val_statically_known(0));
/// }
/// ```
///
/// Unsafe code may not rely on `is_val_statically_known` returning any
/// particular value, ever. However, the compiler will generally make it
/// return `true` only if the value of the argument is actually known.
///
/// When calling this in a `const fn`, both paths must be semantically
/// equivalent, that is, the result of the `true` branch and the `false`
/// branch must return the same value and have the same side-effects *no
/// matter what*.
#[rustc_const_unstable(feature = "is_val_statically_known", issue = "none")]
#[rustc_nounwind]
pub fn is_val_statically_known<T: Copy>(arg: T) -> bool;

Summary

Per #121064 (comment) comment by @Nilstrieb, is_val_statically_known only supports primitive types - which is not listed anywhere in the documentation.

When trying to use inside the core library on core::fmt::Arguments, compiler crashes #121066.

CC: @NCGThompson

@nyurik nyurik added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Feb 14, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 14, 2024
@jieyouxu
Copy link
Contributor

@rustbot label +A-contributor-roadblock +F-core_intrinsics +T-libs +T-compiler -needs-triage

@rustbot rustbot added A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself F-core_intrinsics Issue in the "core intrinsics" for internal usage only. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Feb 14, 2024
@Teapot4195
Copy link
Contributor

is_val_statically_known only supports primitive types

Fixed in #120484, building on nightly will fix this crash. is_val_statically_known also returns false for non primitive types since that PR.

@NCGThompson
Copy link
Contributor

is_val_statically_known only supports primitive types

Fixed in #120484, building on nightly will fix this crash. is_val_statically_known also returns false for non primitive types since that PR.

In my opinion, I think that is more of a reason to update documentation.

@NCGThompson
Copy link
Contributor

@rustbot claim

@NCGThompson
Copy link
Contributor

@RalfJung In terms of provenance and Miri, what does is_val_statically_known do with ptrs? I want to explain it by stating that two code snippets are equivalent, but I don't know if it would be correct to use ptr.addr() or ptr.expose_addr().

I think from a theoretical perspective, ptr.addr() would be the way to go because the address is never being turned back into a ptr, but I have no idea if Miri knows that.

@RalfJung
Copy link
Member

RalfJung commented Apr 27, 2024

In terms of the spec, is_val_statically_known just returns an arbitrary boolean, so pointer vs non-pointer doesn't really make any difference.

So it's really up to the optimizer to decide whether knowing the address but not the provenance of a pointer is sufficient to consider the value "known". I personally would say "no", but it's always okay to return true from this function so nothing can break if the answer becomes "yes". I don't know what LLVM does.

@NCGThompson
Copy link
Contributor

NCGThompson commented Apr 27, 2024

I don't know what LLVM does.

LLVM either:

  • only considers the raw byte value of the pointer, or
  • only considers the address of the pointer.

I'm not sure, but I'd bet on the first. The potential difference between those two possibilities is explained in the docs for addr and expose_addr. (I suspect the docs for addr is currently incorrect.)

@RalfJung
Copy link
Member

RalfJung commented Apr 27, 2024

There's no difference between those (on targets we currently support). If you think the addr docs are incorrect, please file an issue.

Anyway, we can just call the ptr version of the LLVM intrinsic, and worry about what it does once we actually need this on pointers somewhere.

NCGThompson added a commit to NCGThompson/rust that referenced this issue Apr 29, 2024
* Add `Type Requirements` section, listing the allowed inputs, as requested by rust-lang#121115
* Add `Pointers` subsection, explaining is_val_statically_known handles pointers.
* Add `Stability concerns` section, referring to other documentation relating to consistency in `const` functions.
@bors bors closed this as completed in 5b1d58c Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools F-core_intrinsics Issue in the "core intrinsics" for internal usage only. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants