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

Incorrect detection of compilation failures when there's a panic in const evaluation. #2923

Closed
LouisGariepy opened this issue Jun 11, 2023 · 3 comments

Comments

@LouisGariepy
Copy link

LouisGariepy commented Jun 11, 2023

Doctests that yield a compile-time error due to a panic in a const evaluation are buggy under MIRI. As an example, consider the following code:

pub struct Foo<const N: usize>;

impl<const N: usize> Foo<N> {
    const N_GT_ZERO: () = assert!(N > 0);

    /// ```rust,compile_fail
    /// use blackboard::Foo;
    ///
    /// let foo = Foo::<0>::new();
    /// ```
    pub fn new() -> Self {
        // This will cause a panic at const-time if `N == 0`
        let _ = Self::N_GT_ZERO;
        Self
    }
}

Running cargo test informs us that the one doc test passed.

Now, running cargo +nightly miri test instead marks the test as failed (i.e. MIRI thinks it somehow compiled successfully)

Test compiled successfully, but it's marked `compile_fail`.

Removing the compile_fail annotation and rerunning miri also fails (because of the intended compilation error).

It seems like there might be a wart in how compile_fail doctests are handled by MIRI.

@saethlin
Copy link
Member

The problem isn't with doctests, it's a fundamental problem with how Miri works (currently?). Your example can be rewritten as:

pub struct Foo<const N: usize>;

impl<const N: usize> Foo<N> {
    const N_GT_ZERO: () = assert!(N > 0);

    pub fn new() -> Self {
        // This will cause a panic at const-time if `N == 0`
        let _ = Self::N_GT_ZERO;
        Self
    }
}

fn main() {
    let foo = Foo::<0>::new();
}

Then if you try to run it with cargo miri run, you'll see something like this:

Preparing a sysroot for Miri (target: x86_64-unknown-linux-gnu)... done
   Compiling scratch v0.1.0 (/tmp/scratch)
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s
     Running `/home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner target/miri/x86_64-unknown-linux-gnu/debug/scratch`

The program "builds" successfully, and fails at runtime. All const eval and post-mono errors are runtime errors to Miri.

@LouisGariepy LouisGariepy changed the title Incorrect handling of compile_fail doctests when there's a panic in const evaluation. Incorrect detection of compilation failures when there's a panic in const evaluation. Jun 11, 2023
@LouisGariepy
Copy link
Author

@saethlin Thanks for the insight!

I amended the title to make the situation a bit clearer. Feel free to modify it further if it's not appropriate.

I imagine the next step is to determine if this is a "wontfix" or if there's an actionable path forward.

@saethlin
Copy link
Member

I think this is a duplicate of #2423 (I looked for that issue earlier and came up empty).

That other Miri issue links to a rust-lang/rust issue which has some PRs linked to it. We'd like to fix this, but don't hold your breath. You might be able to use #[cfg(not(miri))] to work around this limitation for now.

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

No branches or pull requests

2 participants