Add compile-pass mode to compiletest

[RalfJung]

rustc itself does not use this, but this is really useful for other clients of compiletest, e.g., miri.

Currently, miri uses some pretty epic hacks to work around not having this -- see for yourself in rust-lang/miri#334.

[rust-highfive]

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

[pnkfelix]

Hmm I thought our usual way to handle this was to force (via an attribute) fn main() to emit an error in a compile-fail/ test (and then have the absence of other errors messages be the way we "prove" that the compile otherwise passed).

• See test/compile-fail/rustc-error.rs for a minimal example of this.

Update: I'm not saying that I prefer the above hack to having a dedicated compile-pass mode.

[RalfJung]

miri needs this because there is no binary being produced, so running the test makes no sense (and indeed, for "cross-interpretation", is not possible). rustc is in a different situation, so why would rustc need this? That test, to me, looks more like it tests the rustc_fail attribute.

But then there is e.g. https://github.com/rust-lang/rust/blob/b1363a73ede57ae595f3a1be2bb75d308ba4f7f6/src/test/compile-fail/privacy/restricted/lookup-ignores-private.rs...

[oli-obk]

The difference between compile-fail + many uses of #[rustc_error] and compile-pass is that compile-pass does not check any warnings, notes, ...

There are many compile-fail tests that fit in the "just compile" category. I assume they aren't run-pass tests because there's no point in running many tests just containing fn main() {} as the only runtime code.

This also means that many run-pass tests could be moved to compile-pass.

[RalfJung]

compile-pass does not check any warnings, notes, ...

What do you man by "check warnings etc."? Should things be deny(warnings)?

I am happy to move some existing #[rustc_error] tests to compile-pass.

[oli-obk]

What do you man by "check warnings etc."?

I mean it ignores //~ WARNING annotations

Should things be deny(warnings)?

No, that can be enabled when explicitly checking whether some test does not emit certain warnings, but should not be turned on in general imo.

[RalfJung]

I mean it ignores //~ WARNING annotations

Oh I see. But for tests that don't have anything like that (e.g. this one), making the compile-pass would make perfect sense?

This also means that many run-pass tests could be moved to compile-pass.

Tests like this? However, it has some "pretty-expanded" flag, so something still seems to be going on somewhere...

[oli-obk]

making the compile-pass would make perfect sense?

yes

Tests like this? However, it has some "pretty-expanded" flag, so something still seems to be going on somewhere...

I'm not sure about that one either.

[RalfJung]

It seems the test suites to run are defined in bootstrap/check.rs. However, I do not understand these definitions there

 Test { path: "src/test/run-pass/pretty", mode: "pretty", suite: "run-pass" },
Test { path: "src/test/run-fail/pretty", mode: "pretty", suite: "run-fail" },
Test { path: "src/test/run-pass-valgrind/pretty", mode: "pretty", suite: "run-pass-valgrind" },
Test { path: "src/test/run-pass-fulldeps/pretty", mode: "pretty", suite: "run-pass-fulldeps" },
Test { path: "src/test/run-fail-fulldeps/pretty", mode: "pretty", suite: "run-fail-fulldeps" },

None of these folders even exist...

[RalfJung]

I moved all run-pass tests that have a line matching fn main\(\) \{ *\} to compile-pass. I hope that makes sense...

I can do similar things for compile-fail, though the grepping will have to be a little more careful there I suppose.

[RalfJung]

Ah dang this is a little overeager and moves too many tests... I'll try sth. more conservative

[RalfJung]

All right, I moved everything that matches fn main\(\) \{[^}]*$ back.

[kennytm]

Does compile-pass perform trans? Linking?

[RalfJung]

It invokes rustc just the way run-pass does. The only thing it does not do is run the generated executable.

I am fine not moving any tests around here; all I really care about is having this test mode available for miri.^^

[oli-obk]

Does compile-pass perform trans? Linking?

That's up to rustc/miri. If the compilation stops early, noone will notice, because noone tries to use the final binary. Miri simply stops compilation before it reaches trans (on non-host targets)

[kennytm]

@oli-obk

If the compilation stops early, noone will notice, because noone tries to use the final binary.

Not really, e.g. if you referred to an undefined external symbol, it will not fail until reaching the linker.

[RalfJung]

I verified that the compile-pass tests all only have one ::main function that is empty.

Let me know if you'd prefer me to

• Keep things the way they are now; or
• revert the 2nd commit, and only add a new compile-pass mode to be used by other consumers for compiletest; or
• also try to move #[rustc_error] compile-fail tests to compile-pass.

[nikomatsakis]

OK so --

This is not the direction that I want to move in (personally) for our test suite. I would prefer to be grouping tests by the functionality that they test and not the mode that they run in. More specifically, my plan was to unify compile-fail, run-pass, and ui into a single unified mechanism. Roughly speaking, I would like the ability to do all of the following:

• Filter tests e.g. based on how many dependencies they require or other keyword like filters (I'd like to be able to have richer categories).
• Place "I expect an error on this line" annotations within the test itself. If such annotations are present, then I would want them to be enforced (they are enforced in compile-fail, they are not in ui). I don't think all tests, even all tests that error out, need such annotations, because we will also be checking the full output from the compiler (next bullet), which ensures that things don't silently start to error in new ways. But sometimes I think they are useful (particularly when intermingling a number of examples of the kind "this should work" and "this should not").
• Always test and see the full output from compiling and running the program (as ui tests do today) -- this applies to run-pass tests too.
  • I consider it a must to have some tool to make updating this output less tedious, as well (i.e., "capture current output"). I then study the diffs. I find it excellent to be able to review PRs and just see the diffs in the compiler output as well.

We currently have the ability to define "run-pass" ui tests, so we're part of the way there. =)

I wrote about this here in this internals thread (with this addendum) and I still personally think it's the way to go.

That said, I have wanted the ability to write compile-pass tests forever and certainly I don't want to block miri.

[oli-obk]

Hmm... our compile-fail tests can't be ui tests, but maybe our run pass tests could be.

[nikomatsakis]

@oli-obk why can't your compile-fail tests be ui tests? Because you don't generate output?

[oli-obk]

I tried it once. Since we generate a stacktrace on failures, the output differs between the emulated platforms, since libstd's condiuonal compilation produces different functions. We might be able to reduce some cases, but many are impossible

[RalfJung]

We'd also have to re-create all ui files if the libstd stack trace "above main" ever changes, which sounds like a pain.

This is not the direction that I want to move in (personally) for our test suite. I would prefer to be grouping tests by the functionality that they test and not the mode that they run in.

Fair enough. As I already said above, I am no no way attached to moving existing rustc tests around, and I can take the 2nd commit out of this PR. Would that work for you?

I noticed that ui tests have a conditional in compiletest to control whether the test is run, but I could not figure out so far how that conditional is controlled:

rust/src/tools/compiletest/src/runtest.rs

Line 2209 in 3a7ab6c

ifself.props.run_pass{

EDIT: Ah, it seems one says // run-pass in the file to enable running a ui test, and the default is to not run them. Indeed we should at least try to use ui tests for miri.

[RalfJung]

It actually seems like ui tests work very well for miri's run-pass suite, see rust-lang/miri#343.