Skip to content

Allow tests to specify a //@ filecheck-flags: header #120656

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 6 commits into from
Feb 26, 2024

Conversation

Zalathar
Copy link
Contributor

@Zalathar Zalathar commented Feb 5, 2024

This allows individual codegen/assembly/mir-opt tests to pass extra flags to the LLVM filecheck tool as needed.


The original motivation was noticing that tests/run-make/instrument-coverage was very close to being an ordinary codegen test, except that it needs some extra logic to set up platform-specific variables to be passed into filecheck.

I then saw the comment in verify_with_filecheck indicating that a filecheck-flags header might be useful for other purposes as well.

@rustbot
Copy link
Collaborator

rustbot commented Feb 5, 2024

r? @wesleywiser

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

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Feb 5, 2024
@bors
Copy link
Collaborator

bors commented Feb 16, 2024

☔ The latest upstream changes (presumably #120881) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Collaborator

bors commented Feb 22, 2024

☔ The latest upstream changes (presumably #121370) made this pull request unmergeable. Please resolve the merge conflicts.

This removes a version check for LLVM >=13, and specifies prefixes as a series
of independent `--check-prefix` flags instead of a single `--check-prefixes`.
Any flags specified here will be passed to LLVM's `filecheck` tool, in tests
that use that tool.
This makes room for migrating over `tests/run-make/instrument-coverage`,
without increasing the number of top-level items in the codegen test directory.
This test was already very close to being an ordinary codegen test, except that
it needed some extra logic to set a few variables based on (target) platform
characteristics.

Now that we have support for `//@ filecheck-flags:`, we can instead set those
variables using the normal test revisions mechanism.
This define was copied over from the run-make version of the test, but doesn't
seem to serve any useful purpose.
@Zalathar
Copy link
Contributor Author

Updated for #121370.

@Zalathar Zalathar changed the title Allow tests to specify a // filecheck-flags: header Allow tests to specify a //@ filecheck-flags: header Feb 23, 2024
@wesleywiser
Copy link
Member

Awesome, always nice to see run-make tests getting migrated into regular compiler tests!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Feb 23, 2024

📌 Commit e56cc84 has been approved by wesleywiser

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 23, 2024
@bors
Copy link
Collaborator

bors commented Feb 23, 2024

⌛ Testing commit e56cc84 with merge b9ad718...

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 23, 2024
Allow tests to specify a `//@ filecheck-flags:` header

This allows individual codegen/assembly/mir-opt tests to pass extra flags to the LLVM `filecheck` tool as needed.

---

The original motivation was noticing that `tests/run-make/instrument-coverage` was very close to being an ordinary codegen test, except that it needs some extra logic to set up platform-specific variables to be passed into filecheck.

I then saw the comment in `verify_with_filecheck` indicating that a `filecheck-flags` header might be useful for other purposes as well.
@bors
Copy link
Collaborator

bors commented Feb 23, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 23, 2024
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
2
Image input checksum 12016981bb9ecd7e1cb6029ce5b25a12860623d0aa9252b426f2e1a39932f276e74bd6f7eff8601d92ae022fd82e7cf12608bc40f9a6c9b8d243079bcbc2c1c0
##[group]Building docker image for dist-various-2
Docker version 24.0.8, build e0dfb46
Error response from daemon: Get "https://ghcr.io/v2/": net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)
##[error]Process completed with exit code 1.

@Zalathar
Copy link
Contributor Author

Looks like this got caught up in whatever CI failure has the tree closed at the moment.

@ehuss
Copy link
Contributor

ehuss commented Feb 26, 2024

@bors retry

ghcr.io network error

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 26, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 26, 2024
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#120656 (Allow tests to specify a `//@ filecheck-flags:` header)
 - rust-lang#120840 (Split Diagnostics for Uncommon Codepoints: Add Individual Identifier Types)
 - rust-lang#121554 (Don't unnecessarily change `SIGPIPE` disposition in `unix_sigpipe` tests)
 - rust-lang#121590 (Correctly handle if rustdoc JS script hash changed)
 - rust-lang#121620 (Fix more rust-lang#121208 fallout (round 3))

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0e08be5 into rust-lang:master Feb 26, 2024
@rustbot rustbot added this to the 1.78.0 milestone Feb 26, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 26, 2024
Rollup merge of rust-lang#120656 - Zalathar:filecheck-flags, r=wesleywiser

Allow tests to specify a `//@ filecheck-flags:` header

This allows individual codegen/assembly/mir-opt tests to pass extra flags to the LLVM `filecheck` tool as needed.

---

The original motivation was noticing that `tests/run-make/instrument-coverage` was very close to being an ordinary codegen test, except that it needs some extra logic to set up platform-specific variables to be passed into filecheck.

I then saw the comment in `verify_with_filecheck` indicating that a `filecheck-flags` header might be useful for other purposes as well.
@Zalathar Zalathar deleted the filecheck-flags branch February 26, 2024 13:00
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 18, 2024
compiletest: don't register predefined `MSVC`/`NONMSVC` FileCheck prefixes

This was fragile as it was based on host target passed to compiletest,
but the user could cross-compile and run test for a different target
(e.g. cross from linux to msvc, but msvc won't be set on the target).
Furthermore, it was also very surprising as normally revision names
(other than `CHECK`) was accepted as FileCheck prefixes.

This partially reverts the `MSVC`/`NONMSVC` predefined FileCheck
prefix registration introduced in rust-lang#120656 for some codegen tests.

This makes some codegen tests more verbose since they now need to
explicitly introduce `MSVC`/`NONMSVC` revisions, but I think that's
less surprising, e.g.:

```rs
//@ revisions: MSVC NONMSVC
//`@[MSVC]` only-msvc
//`@[NONMSVC]` ignore-msvc
```

Note that revisions are not *only* FileCheck prefixes in
FileCheck-based test suites, as they also can be used
to conditionally apply certain compiletest directives.

r? `@Zalathar` (or reroll a `r/? compiletest` reviewer)

try-job: x86_64-msvc
try-job: i686-msvc
try-job: x86_64-mingw
try-job: i686-mingw
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 18, 2024
compiletest: don't register predefined `MSVC`/`NONMSVC` FileCheck prefixes

This was fragile as it was based on host target passed to compiletest,
but the user could cross-compile and run test for a different target
(e.g. cross from linux to msvc, but msvc won't be set on the target).
Furthermore, it was also very surprising as normally revision names
(other than `CHECK`) was accepted as FileCheck prefixes.

This partially reverts the `MSVC`/`NONMSVC` predefined FileCheck
prefix registration introduced in rust-lang#120656 for some codegen tests.

This makes some codegen tests more verbose since they now need to
explicitly introduce `MSVC`/`NONMSVC` revisions, but I think that's
less surprising, e.g.:

```rs
//@ revisions: MSVC NONMSVC
//`@[MSVC]` only-msvc
//`@[NONMSVC]` ignore-msvc
```

Note that revisions are not *only* FileCheck prefixes in
FileCheck-based test suites, as they also can be used
to conditionally apply certain compiletest directives.

r? `@Zalathar` (or reroll a `r/? compiletest` reviewer)

try-job: x86_64-msvc
try-job: i686-msvc
try-job: x86_64-mingw-1
try-job: i686-mingw
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants