compiletest: Deduplicate --check-prefix flags

[tgross35]

Currently having a revision named like MSVC causes errors because it gets passed via --check-prefix twice; once from the revision name and once from the default msvc_or_not value that compiletest sets. Fix this by deduplicating revision names before passing the arguments.

[rustbot]

r? @onur-ozkan

rustbot has assigned @onur-ozkan.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

Some changes occurred in src/tools/compiletest

cc @jieyouxu

[tgross35]

I noticed this when working on #134290, having a revision named MSVC causes a duplicate flag warning. Also weird: this warning showed up only when running in CI on an actual MSVC machine, not locally for me even though it is testing with a -msvc target (via no_core). Is it possible the MSVC/NOMSVC prefix is getting set based on the host rather than the target? Doesn't look like it from the code, but I don't know why else that would happen.

r? @jieyouxu

[jieyouxu]

... That seems slightly cursed, I'll have to do some digging and look at this tmrw.

[tgross35]

For reference the failure was here #134290 (comment). I never ran the whole suite locally but ./x t tests/codegen/i128-x86-callconv.rs passed successfully running on my mac.

That PR includes this one's commit for now.

[jieyouxu]

cc @Zalathar since this was added in #120656. However, that logic AFAICT only accepts MSVC and NONMSVC as "predefined FileCheck prefixes" but I can't find any logic that actually distinguishes between MSVC and NONMSVC. I feel like I'm missing some context. @Zalathar was MSVC meant to imply some kind of implicit only-msvc-esque logic...?

EDIT: yes, I'm just blind, the MSVC prefixes is only set when:

if self.config.target.contains("msvc") { "MSVC" } else { "NONMSVC" };

Which AFAIK is probably based on the host. I would probably say that we might not want to set these FileCheck prefixes automatically, even at the cost of a few more lines in the codegen tests, because this behavior is really not straightforward.

[Zalathar]

FWIW these automatic FileCheck prefixes predate me; I was just the person who most recently tried to clean up the code for setting them.

Getting rid of this implicit behaviour should be doable. I've looked into doing so in the past, but never got around to actually filing a PR.

[jieyouxu]

Yeah I realized that now, as I somehow completely missed the contains("msvc") in the diff, lol, sorry. I've opened #134463 to get rid of the MSVC/NONMSVC predefined prefix and updated codegen tests that use them.

@tgross35 I think we should not paper over the underlying issue that MSVC/NONMSVC are some magical predefined FileCheck prefixes (which are not predefined revisions!), so I'm inclined to close this in favor of #134463. Can you check that if you rebase #134290 over #134463 and apply my suggestion here to introduce explicit revisions instead, that it passes for you when cross-compiling to msvc?

[tgross35]

Sounds reasonable, thanks for putting up a fix! I’ll try it later today.