Const assert prevents compilation but trybuild thinks it succeeds

[rjsberry]

Thanks for building trybuild, David!

I was using the crate to test some const assertions were failing. Despite the fact that I can't actually compile the example code, when I include the same code as a trybuild test case to check for compile failure it fails. In other words, trybuild thinks some code compiles when it definitely doesn't.

I've reduced my problem down to:

// lib.rs

pub struct Assert<const L: usize, const R: usize>;

impl<const L: usize, const R: usize> Assert<L, R> {
    pub const GREATER_EQ: usize = L - R;
}

/// Asserts `L >= R` at compile-time.
#[allow(path_statements)]
pub fn const_assert_gteq<const L: usize, const R: usize>() {
    Assert::<L, R>::GREATER_EQ;
}

Then, my test case:

// tests/ui/gteq_1_2.rs

fn main() {
    trybuild_mre::const_assert_gteq::<1, 2>();
}

Running the test case with:

// tests/ui.rs

#[test]
fn ui() {
    let t = trybuild::TestCases::new();
    t.compile_fail("tests/ui/*.rs");
}

Results in:

% cargo t ui
    Finished test [unoptimized + debuginfo] target(s) in 0.07s
     Running unittests src/lib.rs (target/debug/deps/trybuild_mre-bea342f0800074c4)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/ui.rs (target/debug/deps/ui-8ce52de26e4c0f2d)

running 1 test
    Checking trybuild-mre-tests v0.0.0 (~/Code/trybuild-min/target/tests/trybuild/trybuild-mre)
    Finished dev [unoptimized + debuginfo] target(s) in 0.32s


test tests/ui/gteq_1_2.rs ... error
Expected test case to fail to compile, but it succeeded.


test ui ... FAILED

failures:

---- ui stdout ----
thread 'ui' panicked at '1 of 1 tests failed', ~/.cargo/registry/src/github.com-1ecc6299db9ec823/trybuild-1.0.77/src/run.rs:101:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    ui

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.21s

error: test failed, to rerun pass `--test ui`

However, if I try and build the test case gteq_1_2.rs as an example I get a compile fail:

% cargo b --example gteq_1_2
   Compiling trybuild-mre v0.1.0 (~/Code/trybuild-mre)
error[E0080]: evaluation of `trybuild_mre::Assert::<1, 2>::GREATER_EQ` failed
 --> ~/Code/trybuild-mre/src/lib.rs:4:35
  |
4 |     pub const GREATER_EQ: usize = L - R;
  |                                   ^^^^^ attempt to compute `1_usize - 2_usize`, which would overflow

note: the above error was encountered while instantiating `fn trybuild_mre::const_assert_gteq::<1, 2>`
 --> examples/gteq_1_2.rs:2:5
  |
2 |     trybuild_mre::const_assert_gteq::<1, 2>();
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

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

If you'd like to run this I've pushed the minimum reproducible example to https://github.com/rjsberry/trybuild-mre.

I'm going to take a look into this myself, but my best guess right now (without reading the trybuild source) is that the compile failure is actually occuring in the crate which defines the const assert, rather than the example itself.

[rjsberry]

Ok, so it looks like internally trybuild is checking for failure using cargo check if the project doesn't have any tests which expect to pass. If it does, cargo build is used instead.

If I update my test suite with a basic test that compiles:

// tests/ui/pass.rs

fn main() {}

Then re-write the trybuild test to be:

 // tests/ui.rs

 #[test]
 fn ui() {
     let t = trybuild::TestCases::new();
-    t.compile_fail("tests/ui/*.rs");
+    t.compile_fail("tests/ui/gteq_1_2.rs");
+    t.pass("tests/ui/pass.rs");
 }

Everything works as expected.

Before I make the change and submit a PR, is there a reason not to use cargo build at all times? Is this a problem with cargo check itself -- why doesn't it catch the compile failure?

I think this is a cargo check issue. rust-lang/cargo#11780, rust-lang/rust#70923

In the shorter term I think we can solve this in trybuild by either:

1. Always running cargo build. Drawbacks are it would be slower for complex test cases, but errors that cargo check doesn't currently catch won't slip through.
2. Running checks with cargo build as an opt-in somewhere in the TestCases API. While we can sort of force this with an empty pass test, it would be nice to have a method to explicitly force tests to use cargo build under the hood.

I'd be happy to submit a PR if I could get a steer on how you think this should be tackled.

[dtolnay]

I think this is behaving correctly. What you've written is not a reliable way to elicit a compile time panic because a const is not necessarily evaluated at compile time, as long as it type-checks.

[rjsberry]

Interesting -- in case anyone stumbles across this in the future: https://doc.rust-lang.org/reference/const_eval.html

Certain forms of expressions, called constant expressions, can be evaluated at compile time. In const contexts, these are the only allowed expressions, and are always evaluated at compile time. In other places, such as let statements, constant expressions may be, but are not guaranteed to be, evaluated at compile time.

I suppose what's written in my example above would be a let statement or const expression:

/// Asserts `L >= R` at compile-time.
#[allow(path_statements)]
pub const fn const_assert_gteq<const L: usize, const R: usize>() {
    Assert::<L, R>::GREATER_EQ;
}

In this case it is actually being evaluated at compile-time (cargo build fails), but is not guaranteed to be.

---

@dtolnay This seems like a good regression to catch with trybuild even though it's not guaranteed. Crates such as heapless are currently relying on this pattern to check const generics at compile time: https://github.com/japaric/heapless/blob/644653b/src/sealed.rs and https://github.com/japaric/heapless/blob/644653b/src/vec.rs#L65

What are your thoughts on possibly adding an env var override to trybuild to force it to use cargo build behind the scenes? This would stop us introducing clutter into the API, but also provide the right workaround for anyone else that might stumble into issues caused by rust-lang/rust#70923.

Unfortunately I didn't see any documentation for when trybuild uses cargo check vs cargo build. It wasn't until I read https://github.com/dtolnay/trybuild/blob/e30b128/src/cargo.rs#L112 that I realised adding an empty "pass" test would be a suitable workaround.

I suppose usage of build/check could be considered an implementation detail, but if they can produce different results I think it would be good to allow users to opt in to which they would prefer.

[YuhanLiin]

Now that we have have const blocks, we have a guaranteed way of performing assertions during compile time. For example:

const {
  assert_eq!(1, 0);
}

This currently results in a compile failure with Trybuild::pass, but passes compilation with Trybuild::compile_fail. I think this issue should be reopened.