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

Const assertions are not guaranteed to cause compile failures #351

Open
rjsberry opened this issue Mar 1, 2023 · 3 comments
Open

Const assertions are not guaranteed to cause compile failures #351

rjsberry opened this issue Mar 1, 2023 · 3 comments
Labels

Comments

@rjsberry
Copy link
Contributor

rjsberry commented Mar 1, 2023

In sealed.rs, there are a number of associated constants which are used in const fns to try and trigger compile failures when const generics don't match particular requirements.

These const fns contain constant expressions, which may (but are not guaranteed to) evaluate at compile time. AFAICT from https://doc.rust-lang.org/reference/const_eval.html while this works now it might stop working in the future and instead cause a runtime panic.

Running cargo check on an example which we know won't build:

% cat examples/bad_indexmap.rs  
fn main() {
    // needs N to be a power of 2
    let _map = heapless::FnvIndexMap::<u8, u8, 3>::new();
}

% cargo check --example bad_indexmap
    Checking heapless v0.8.0 (~/Code/heapless)
    Finished dev [unoptimized + debuginfo] target(s) in 0.17s

% cargo build --example bad_indexmap
   Compiling heapless v0.8.0 (~/Code/heapless)
error[E0080]: evaluation of `heapless::sealed::Assert::<3, 0>::POWER_OF_TWO` failed
  --> ~/Code/heapless/src/sealed.rs:57:37
   |
57 |     pub const POWER_OF_TWO: usize = 0 - (L & (L - 1));
   |                                     ^^^^^^^^^^^^^^^^^ attempt to compute `0_usize - 2_usize`, which would overflow

note: the above error was encountered while instantiating `fn heapless::sealed::power_of_two::<3>`
   --> ~/Code/heapless/src/indexmap.rs:541:9
    |
541 |         crate::sealed::power_of_two::<N>();
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0080`.
error: could not compile `heapless` due to previous error

We know this is a bit of a hack evidenced by the comments: // Const assert hack. It would be good to try and add coverage for these compile fails in the cfail test suite so that we can detect if/when this stops being a compile-time error. Unfortunately the trybuild crate is using cargo check under the hood and would not be able to hit this.

@burrbull
Copy link
Member

burrbull commented Mar 1, 2023

This is not actually related to the issue.
This line can be rewritten like:

pub const POWER_OF_TWO: () = assert!(L.is_power_of_two());

@newAM
Copy link
Member

newAM commented Nov 1, 2023

This is not actually related to the issue. This line can be rewritten like:

pub const POWER_OF_TWO: () = assert!(L.is_power_of_two());

I tried this out for MpMcQueue, but it did not assert at compile time.

    /// ```compile_fail
    /// use heapless::mpmc::MpMcQueue;
    /// let q: MpMcQueue<i32, 1> = MpMcQueue::new();
    /// ```
    const _ASSERT1: () = assert!(N > 1);
    /// ```compile_fail
    /// use heapless::mpmc::MpMcQueue;
    /// let q: MpMcQueue<i32, 3> = MpMcQueue::new();
    /// ```
    const _ASSERT2: () = assert!(N.is_power_of_two());
---- src/mpmc.rs - mpmc::MpMcQueue<T,N>::_ASSERT1 (line 141) stdout ----
Test compiled successfully, but it's marked `compile_fail`.
---- src/mpmc.rs - mpmc::MpMcQueue<T,N>::_ASSERT2 (line 146) stdout ----
Test compiled successfully, but it's marked `compile_fail`.

failures:
    src/mpmc.rs - mpmc::MpMcQueue<T,N>::_ASSERT1 (line 141)
    src/mpmc.rs - mpmc::MpMcQueue<T,N>::_ASSERT2 (line 146)

I'm surprised this compiles successfully 🤔

Edit: They needed to be used, see #403

@YuhanLiin
Copy link
Contributor

From https://doc.rust-lang.org/reference/const_eval.html:

In const contexts, these are the only allowed expressions, and are always evaluated at compile time.

As long as we always evaluate the const expressions in const context (so in const variables, for example), it will always evaluate at compile time. We can also use the static_assertions crate so we don't have to roll this ourselves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants