rust-lang/rust

Tracking Issue for more const int functions

TimDiekmann opened this issue Β· 35 comments

(none exhaustive) list of libcore::num, which should be a const fn:

  • feature = const_int_rotate
    • rotate_left #53697 -- stabilized
    • rotate_right #53697 -- stabilized
  • feature = const_checked_int_methods
  • feature = const_saturating_int_methods
  • feature = const_int_wrapping
    • wrapping_add #53697 -- stabilized
    • wrapping_sub #53697 -- stabilized
    • wrapping_mul #53697 -- stabilized
    • wrapping_div [1] #68809
    • wrapping_rem [1] #68809
    • wrapping_neg #58044 -- stabilized
    • wrapping_shl #53697 -- stabilized
    • wrapping_shr #53697 -- stabilized
    • wrapping_abs #63786 -- stabilized in 1.39
  • feature = const_int_overflowing
  • feature = const_euclidean_int_methods
  • feature = const_int_pow
  • feature = const_int_sign
  • feature = const_int_conversion
  • feature = const_nonzero_int_methods
  1. Requires conditionals. Tracking Issue: #49146 (implemented on nightly)
  2. Requires loops. Tracking Issue: #52000 (implemented on nightly)

This issue has been assigned to @9999years via this comment.

transmute() just got merged. You can start on your last block now :)

@TheDarkula Thank you, I'll start implementing them as soon as #53699 is merged.

Added rest of const_int_conversion to #53697

I'm currently rebasing #53697 and testing. Then the first bunch should be ready to merge.

Adding a comment here so I don't have to hunt for the following related issue: #51267 is the tracking issue for feature = const_int_ops, which tracks:

  • count_zeros -- stabilized
  • count_ones -- stabilized
  • leading_zeros -- stabilized
  • trailing_zeros -- stabilized

wrapping_neg isn't on the list here it seems (though overflowing_neg is)

It's possible to make a great number of these const without conditional statements being legal in const fn by using indexing (e.g. [x, -x][(x < 0) as usize]). However, it's a bad idea because the technique results in much less efficient generated code. You can check this by viewing ASM in the previous link. I'm posting this to avoid duplication of effort in case someone else has the same idea.

In fact, we are already doing that to make Vec::new() a const fn πŸ˜›

pub const fn new_in(a: A) -> Self {
// !0 is usize::MAX. This branch should be stripped at compile time.
// FIXME(mark-i-m): use this line when `if`s are allowed in `const`
//let cap = if mem::size_of::<T>() == 0 { !0 } else { 0 };
// Unique::empty() doubles as "unallocated" and "zero-sized allocation"
RawVec {
ptr: Unique::empty(),
// FIXME(mark-i-m): use `cap` when ifs are allowed in const
cap: [0, !0][(mem::size_of::<T>() == 0) as usize],
a,
}
}

@mark-i-m That's probably where I first saw this technique!

My point was that making a similar modification would likely slow down these arithmetic operations considerably. The use in Vec::new can be eliminated via const-propagation post-monomorphization, but that optimization doesn't apply here.

@TimDiekmann. Now that #61635 has been merged, signum is an unstable const fn. Could you edit the initial post to reflect this?

Some of these could be made const today without conditionals. For unsigned:

  • overflowing_div
  • wrapping_div
  • wrapping_div_euclid
  • wrapping_rem
  • wrapping_rem_euclid
  • overflowing_div
  • overflowing_div_euclid
  • overflowing_rem
  • overflowing_rem_euclid
  • div_euclid
  • rem_euclid
  • is_power_of_two

For u8:

  • is_ascii

Also, wrapping_neg and overflowing_neg should be updated in the OP. Both were made const in #58044

is_power_of_two and next_power_of_two shouldn't require conditionals. See https://graphics.stanford.edu/~seander/bithacks.html#DetermineIfPowerOf2 (replacing && with &) and https://graphics.stanford.edu/~seander/bithacks.html#RoundUpPowerOf2, respectively.

@jonas-schievink is_power_of_two was recently constified. I've updated the original post.

Currently, next_power_of_two uses a non-zero count trailing zeros intrinsic, so it might be hard to constify without regressing performance. It would be a good first issue for someone to try this out and see though.

Actually, quite a few of these are now implemented. I've edited the post to reflect this. If anyone sees something out-of-date, let me know.

@ecstatic-morse The intrinsic can be const and implemented in miri. That's what's done for trailing_zeros, for example.

You need to handle zero somehow without branching, and you need to do so without regressing performance for non-const code.

(Also, I meant leading zeros above, my bad).

Should NonZero*::new fall in this category?

not until const if

@SimonSapin Yes. I've added it to the OP.

Out of curiosity, what's the bar for the const if / const match functions to get merged? By which I mean– the standard library makes use of a lot of unstable features, even in stable code (iterator specializations being the most obvious example); at what point can the same thing happen with const conditionals?

@Lucretiel I can't speak for the libs team, but I think anything in the standard library that could be constified with this feature gate should be. However, any PRs that do this need to also add #![rustc_const_unstable] to the newly const function, since without the attribute we would basically be back-door stabilizing #![feature(const_if_match)]. There's at least one lang team member who feels this way as well.

There's one additional complication: The current stage0 compiler, which is used to bootstrap rustc does not have #66507. That means any use of #![feature(const_if_match)] in the standard library must be behind #[cfg(not(bootstrap))]. Unless there is an urgent need, I think we should wait until this is no longer necessary to constify standard library functions. I believe this will happen in mid-December when the beta branch occurs.

@rustbot claim

Working on the ones blocked by const_if_match in #66884 (thanks for #66507, @ecstatic-morse!)

It's my understanding that things needing const if will stay nightly gated while const if itself is nightly gated. But you can probably submit it to be available as const on Nightly with just a PR.

@rustbot release-assignment

Are these changes alone sufficient to allow to use arithmetic ops like β€œ+” in a constant context or does is require landing of const traits (for e.g. std::ops::Add)?

I think that the {to,from}_{be,le,ne}_bytes methods should be made const in stable. While it is true that currently the implementation depends on e.g. transmute, that is a hidden implementation detail, and they could already be implemented as const stably with e.g. b[3] << 24 | b[2] << 16 | b[1] << 8 | b[0]. Of course, I'm not suggesting that the implementation itself should be changed, just that it should not stop them from being marked const in stable.

Would a PR making .rem_euclid a const fn on stable be accepted? It could be trivially implemented as ((x % y) + y) % y. This reasoning has been used for other methods in the past.

@jhpratt

It doesn't give the same results for negative y. 3.rem_euclid(-2) == 1, but ((3 % (-2)) + (-2)) % (-2) == -1. This could be solved by using y.abs().

But it would overflow for example when x == i32::MAX - 1 && y == i32::MAX.

Can we stabilize the const_int_conversion gate, or is there a standing issue preventing stabilization? As has been mentioned upthread, these functions have equivalent const-able expressions, but as they are still unstable, these must be copied into each crate that needs them. This copying is bug-prone, so stabilization would reduce duplication and the possibility of error production.

@myrrlyn It's already stabilized in beta.

Hello, as far as I can tell, both [1] and [2] are now stable, right? so there is no extra blocking issue to implement/stabilize those, right? Is there something that can be done to get them stabilized?

@Razican Most of [1] were marked stable in #73858, so they will be stabilized in 1.47. The PR did not include division and remainder because they can panic, so maybe they need another footnote.

To stabilize the rest that can be stabilized (the pow functions I think), someone needs to write a PR, which will then need to be approved, pass through FCP, and get merged.

#80962 is up to stabilize all remaining methods mentioned at the top of this issue. The diagnostic on nightly is functionally the same as already-stable equivalents (5 % 0 vs 5.rem_euclid(0), for example).