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

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

Closed
AngelicosPhosphoros opened this issue Jul 23, 2022 · 9 comments · Fixed by #3392
Labels
A-cargo Area: affects the cargo wrapper (cargo miri) C-bug Category: This is a bug.

Comments

@AngelicosPhosphoros
Copy link

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
Copy link
Member

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.

@RalfJung RalfJung added C-bug Category: This is a bug. A-cargo Area: affects the cargo wrapper (cargo miri) labels Jul 23, 2022
@RalfJung RalfJung changed the title compile_fail tests that rely on associated constant evaluation errors don't pass miri test compile_fail doctests that rely on post-monomorphization errors don't pass miri test Jul 24, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Jul 24, 2022

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

@RalfJung
Copy link
Member

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
Copy link
Author

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: rust-lang/rust#99471 (comment)

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
Copy link
Member

Okay, reported as rust-lang/rust#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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

bjorn3 commented Mar 19, 2024

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: rust-lang/rustc_codegen_cranelift@e64a7eb#diff-a5f3d43651e52a0742eefa978b45e122ae035eb884742ee9d7178f3f7ecdc4d4R152-R153

@RalfJung
Copy link
Member

collect_and_partition_mono_items seems to work just fine in check mode these days: #3392.

bors added a commit that referenced this issue Mar 20, 2024
fix compile_fail doctests with post-mono errors

Fixes #2423
@bors bors closed this as completed in addad82 Mar 20, 2024
RalfJung pushed a commit to RalfJung/rust that referenced this issue Mar 23, 2024
fix compile_fail doctests with post-mono errors

Fixes rust-lang/miri#2423
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cargo Area: affects the cargo wrapper (cargo miri) C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants