make run-pass tests with empty main just compile-pass tests

[RalfJung]

Many run-pass tests have an empty main, so there is not actually any point in running them. This makes them compile-pass tests instead, saving some time (generating the binary and then running it).

For now I did this only for run-pass/issues; if there is interest I can also do it for the other directories. I used ^\s*fn\s+main\(\s*\)\s*\{\s*\} as regexp to identify these files.

[rust-highfive]

r? @aturon

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

[RalfJung]

r? @pnkfelix

[pnkfelix]

It seems like this PR is moving a number of the src/test/run-pass/ tests (back?) into the src/test/ui/ directory, which partially reintroduces the problem described at #54047

Long term I wouldn't care, because I am hoping to revise the directory structure to get rid of compile-fail/ and run-pass/, making them all ui tests (and using metadata in the files to let developers control which subset of the tests they wish to run).

Does it not work to tag these tests as // compile-pass while leaving them in src/test/run-pass for now?

[Mark-Simulacrum]

I seem to recall this historically not being an automatable change because some of the tests are verifying link pass (possibly dynamic link pass?), and maybe llvm-pass, which I'm not sure is the case with compile-pass tests.

[RalfJung]

Does it not work to tag these tests as // compile-pass while leaving them in src/test/run-pass for now?

I thought the directories were supposed to be sorted by the kind of their test.

I seem to recall this historically not being an automatable change because some of the tests are verifying link pass (possibly dynamic link pass?), and maybe llvm-pass,

Ah, dang. I can move everything pack that mentions "link" in any way and everything that has auxiliary files. Not sure if that'd be enough.

[RalfJung]

It seems like this PR is moving a number of the src/test/run-pass/ tests (back?) into the src/test/ui/ directory, which partially reintroduces the problem described at #54047

Well, not entirely. compile-pass tests are faster than run-pass tests. And we already have tons of compile-pass tests in ui, how is that different from the ones being moved there? compile-pass just wasn't a think when they got added.

[pnkfelix]

I thought the directories were supposed to be sorted by the kind of their test.

We are trying to work our way towards a world where the tests are sorted by language feature (and maybe by compiler module). Eventually, the compile-fail and run-pass distinctions are, I think, going to be solely encoded in per-test metadata embedded in the source text. (Ie everything will be a ui test.)

But since we aren’t there yet (in terms of not being able to select eg all the run-pass tests in every suite), i would prefer to leave the tests that generate no diagnostic output in their current positions in the file system, if the // compile-pass flag indeed works in that context.

If the flag doesn’t work in that context for some reason, then I guess we can just move the files

(And of course, the best thing might be to actually turn each of these tests into meaningful run-pass tests that actually exercise the dynamic semantics...?)

[RalfJung]

the best thing might be to actually turn each of these tests into meaningful run-pass tests that actually exercise the dynamic semantics

I don't agree. Many of these just make sure something compiles. They test the type checker to accept stuff, and are not interested in actually running any code.

i would prefer to leave the tests that generate no diagnostic output in their current positions in the file system, if the // compile-pass flag indeed works in that context.

I can make that happen for these tests. However, there are already close to 200 compile-pass tests in the ui folder, I will not move those around.

[RalfJung]

But anyway that doesn't even seem to work. This panics even when I have // compile-pass:

fn main() { panic!() }

Not sure why it would do that...

EDIT: Actually, looking at this line that's not surprising.

[pnkfelix]

I don't agree. Many of these just make sure something compiles. They test the type checker to accept stuff, and are not interested in actually running any code.

Okay. I agree that in those cases there's no reason to put in an actual main.

EDIT: Actually, looking at this line that's not surprising.

Hmm. I would suggest we collapse the first two arms in:

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

Lines 265 to 268 in ddb1a2a

	match self.config.mode {
	RunPass => true,
	Ui => self.props.run_pass,
	_ => unimplemented!(),

to just say: RunPass | Ui => self.props.run_pass, but if my memory is correct, I think a problem there is that the run_pass property isn't set, at least not if you don't have the // run-pass comment in the header of the test...

Okay fine I'll just r+ this so we can stop trying to slap band-aids on this in the short term.

[pnkfelix]

@bors r+

[bors]

📌 Commit 1b22bef has been approved by pnkfelix

[pnkfelix]

@bors r-

Just realized that we haven't resolved this question:

I seem to recall this historically not being an automatable change because some of the tests are verifying link pass (possibly dynamic link pass?), and maybe llvm-pass,

Ah, dang. I can move everything pack that mentions "link" in any way and everything that has auxiliary files. Not sure if that'd be enough.

(I assume "move everything pack" there was meant to be "move everything back" ...)

[RalfJung]

Yes, I did grep for "link" and moved everything back that had that term in it.

I also just realized some tests used pub fn main, and added those here.

[pnkfelix]

@bors r+

[bors]

📌 Commit 454b14a has been approved by pnkfelix

[pnkfelix]

Hmm. How can we best prevent cases like #44056 from being swept up in changes like this in the future...