dtolnay / trybuild

Test harness for ui tests of compiler diagnostics
Apache License 2.0
792 stars 67 forks source link

Use `cargo build` instead of `cargo check` #241

Closed madsmtm closed 1 year ago

madsmtm commented 1 year ago

I know that this has been discussed before in https://github.com/dtolnay/trybuild/issues/225, but after the policy change in RFC-3477 (which will probably be approved shortly), cargo check effectively doesn't guarantee the amount of warnings that it produces.

So maybe it now makes sense to reconsider using cargo build instead?

I recognize that unfortunately this will make trybuild slower, but then again, the project is called trybuild; I would argue it's the desired behaviour to completely try to build the files, including the steps noted in the above RFC (linking, const errors, and so on).

dtolnay commented 1 year ago

I think this is all right as currently implemented in trybuild, especially if it encourages people to write their macros in a way that surfaces misuse under both cargo build and cargo check.

wmmc88 commented 1 year ago

I have a use case where a test case will fail both trybuild::TestCases::new().compile_fail() and trybuild::TestCases::new().pass(). The code being tested requires a build script to link properly, so trybuild::TestCases::new().pass() will fail due to linker issues, and trybuild::TestCases::new().compile_fail() will fail since compile_fail triggers cargo check instead of cargo build.

What should be done in these cases since there's a discrepancy between what commands actually run for pass vs compile_fail?

joshlf commented 10 months ago

As part of https://github.com/google/zerocopy/issues/716, we're implementing a fix which is specifically designed to fail after monomorphization, so putting in another vote for supporting cargo build as an option or as the default.