rust-lang / miri

An interpreter for Rust's mid-level intermediate representation
Apache License 2.0
4.31k stars 327 forks source link

`compile_fail` doctests that rely on post-monomorphization errors don't pass `miri test` #2423

Closed AngelicosPhosphoros closed 5 months ago

AngelicosPhosphoros commented 2 years ago

Example code:

/// You cannot send arrays with different sizes:
/// ```compile_fail
/// // You need to use your package name.
/// use reproduce_miri::G;
/// G::i_am_break_on_different_array_sizes([0u8;5], [0u32;3]);
/// ```
pub struct G;

pub trait IAmArray{
    const SIZE: usize;
}

impl<T, const N: usize> IAmArray for [T; N]{
    const SIZE: usize = N;
}

impl G{
    pub fn i_am_break_on_different_array_sizes<A, B>(_a: A, _b: B)
        where A: IAmArray,
            B: IAmArray,
    {
        trait CompileAssert{
            const TRIGGER: ();
        }
        impl<A, B> CompileAssert for (A, B)
            where A: IAmArray,
                B: IAmArray,
        {
            const TRIGGER: () = if A::SIZE == B::SIZE {} else {panic!("You must provide arrays of same length")};
        }

        let _ = <(A, B) as CompileAssert>::TRIGGER;
    }
}

Using cargo +nightly test:

Output ``` > cargo +nightly test Compiling reproduce_miri v0.1.0 (E:\Programs\Rust\reproduce_miri) Finished test [unoptimized + debuginfo] target(s) in 0.72s Running unittests src\lib.rs (target\debug\deps\reproduce_miri-632b99990da55433.exe) running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s Doc-tests reproduce_miri running 1 test test src\lib.rs - G (line 2) - compile fail ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.10s ```

Using cargo +nightly miri test

Output ``` > cargo +nightly miri test Compiling reproduce_miri v0.1.0 (E:\Programs\Rust\reproduce_miri) Finished test [unoptimized + debuginfo] target(s) in 0.17s Running unittests src\lib.rs (target\miri\x86_64-pc-windows-msvc\debug\deps\reproduce_miri-40824facde3e8e37.exe) running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out Doc-tests reproduce_miri running 1 test test src\lib.rs - G (line 2) - compile fail ... FAILED failures: ---- src\lib.rs - G (line 2) stdout ---- Test compiled successfully, but it's marked `compile_fail`. failures: src\lib.rs - G (line 2) test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.12s error: test failed, to rerun pass '--doc' ```

If I remove compile_fail and run miri, I get post-monomorphization error:

Output ``` > cargo +nightly miri test Compiling reproduce_miri v0.1.0 (E:\Programs\Rust\reproduce_miri) Finished test [unoptimized + debuginfo] target(s) in 0.17s Running unittests src\lib.rs (target\miri\x86_64-pc-windows-msvc\debug\deps\reproduce_miri-40824facde3e8e37.exe) running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out Doc-tests reproduce_miri running 1 test test src\lib.rs - G (line 2) ... FAILED failures: ---- src\lib.rs - G (line 2) stdout ---- Test executable failed (exit code: 1). stderr: error[E0080]: evaluation of `<([u8; 5], [u32; 3]) as reproduce_miri::G::i_am_break_on_different_array_sizes::CompileAssert>::TRIGGER` failed --> E:\Programs\Rust\reproduce_miri\src\lib.rs:29:64 | 29 | const TRIGGER: () = if A::SIZE == B::SIZE {} else {panic!("You must provide arrays of same length")}; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'You must provide arrays of same length', E:\Programs\Rust\reproduce_miri\src\lib.rs:29:64 | = note: this error originates in the macro `$crate::panic::panic_2021` (in Nightly builds, run with -Z macro-backtrace for more info) error: post-monomorphization error: referenced constant has errors --> E:\Programs\Rust\reproduce_miri\src\lib.rs:32:17 | 32 | let _ = <(A, B) as CompileAssert>::TRIGGER; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ referenced constant has errors | = note: inside `reproduce_miri::G::i_am_break_on_different_array_sizes::<[u8; 5], [u32; 3]>` at E:\Programs\Rust\reproduce_miri\src\lib.rs:32:17 note: inside `main::_doctest_main_src_lib_rs_2_0` at src\lib.rs:6:1 --> src\lib.rs:5:1 | 6 | G::i_am_break_on_different_array_sizes([0u8;5], [0u32;3]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: inside `main` at src\lib.rs:7:3 --> src\lib.rs:6:3 | 7 | } _doctest_main_src_lib_rs_2_0() } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0080`. failures: src\lib.rs - G (line 2) test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.23 ```

I expected to get same behaviour as when running cargo test

Configuration:

> rustc +nightly --version
rustc 1.64.0-nightly (87588a2af 2022-07-13)
> cargo +nightly miri --version
miri 0.1.0 (cde87d1 2022-07-08)
RalfJung commented 2 years ago

Oh, so compile_fail for regular rustc actually does monomorphization, but in Miri it doesn't.

Uh... I am not sure if this is fixable. When rustdoc asks us to "build" the test, we do a check-build (--emit=metadata), which still triggers most compilation errors. But it doesn't trigger post-monomorphization errors. Those usually occur during codegen, but we cannot run codegen, we don't have a sysroot that would even have the bitcode for that.

oli-obk commented 2 years ago

We could try to tell rustdoc to skips them entirely (may need a patch to rustdoc).

RalfJung commented 2 years ago

Well, we support other ompile_fail tests just fine. Only post-monomorphization errors are a problem.

We'd basically want to do a build with a "null" codegen backend that actually does all the monomorphization but then does not generate any actual code.

Isn't there a rustc issue somewhere tracking the problem that cargo check can succeed even when there are post-monomorphization errors? This is essentially another instance of that problem.

AngelicosPhosphoros commented 2 years ago

Isn't there a rustc issue somewhere tracking the problem that cargo check can succeed even when there are post-monomorphization errors? This is essentially another instance of that problem.

AFAIK, no. I was pointed to it recently: https://github.com/rust-lang/rust/pull/99471#issuecomment-1189169511

We could try to tell rustdoc to skips them entirely (may need a patch to rustdoc).

Well, my case it was exactly one test and I added #[cfg(miri)] let _: () = 5; to it. But this is a workaround, proper solution would be better. Maybe at least some way to write comment in a way that this particular test is ignored for miri. Something like:

` ` `compile_fail (not(miri))
RalfJung commented 2 years ago

Okay, reported as https://github.com/rust-lang/rust/issues/99682.

Maybe at least some way to write comment in a way that this particular test is ignored for miri. Something like:

Yeah, that sounds like a reasonable work-around. But I think we would need support from rustdoc here -- AFAIK rustdoc does not support any kind of conditionals on doctests.

RalfJung commented 5 months ago

If rustdoc could tell us via some hacky way that it expects this test to fail, we could build it with MIRI_BE_RUSTC and leave the full --emit=link around so that we still get post-monomorphization errors.

Or alternatively we could do this for all rustdoc builds, not sure how important it is that these be fast.

RalfJung commented 5 months ago

Ah, no -- we can't link since we have a check-only sysroot.

We'd need some way to tell rustc to run the collection query but not actual codegen.

bjorn3 commented 5 months ago

That would be calling tcx.collect_and_partition_mono_items(). At least 4 years ago calling that in check mode could result in an ICE however: https://github.com/rust-lang/rustc_codegen_cranelift/commit/e64a7ebcb0c8633d30bc553023a6e7ed830b2709#diff-a5f3d43651e52a0742eefa978b45e122ae035eb884742ee9d7178f3f7ecdc4d4R152-R153

RalfJung commented 5 months ago

collect_and_partition_mono_items seems to work just fine in check mode these days: https://github.com/rust-lang/miri/pull/3392.