Normalize FileCheck directives
Proposal
Background
LLVM FileCheck is a LLVM bin tool is that used in our
tests/{assembly,codegen} test suites. FileCheck directives are not handled
by compiletest (which uses //@ prefixes).
Currently, assembly and codegen tests ("tests") uses the following syntax:
Default FileCheck directives
Without specifying revisions, or in the case of revisions specified but you want
unconditionally checked FileCheck directives, FileCheck will look for COM
(which are comment directives that are ignored) and CHECK-prefixed directives
(default). Example matching directives:
// COM:
// CHECK:
// CHECK-NEXT:
// CHECK-SAME:
// CHECK-EMPTY:
// CHECK-NOT:
// CHECK-COUNT:
// CHECK-DAG:
// CHECK-LABEL:
Revisioned FileCheck directives
When you specify revisions, i.e. //@ revisions: a b c, compiletest will pass
the revision names to FileCheck as --check-prefixes=a,b,c, which will modify
how FileCheck matches directives. Namely, FileCheck will now look for
directives with matching prefixes in addition to the default CHECK prefix.
Example matching directives:
// COM:
// CHECK: # <- universally matched: all be matched in each revision a, b, c
// a: # <- only checked for test revision a
// b-SAME: # <- only checked for test revision b
Proposed changes
- Normalize anything that uses an invalid prefix (e.g.
FIXME-CHECK:) toCOM:so we can eventually check for invalid prefixes. - Decide on if we want to enforce casing of revisions and FileCheck prefixes,
e.g.
x86_64-NOT:vsX86_64-NOT:orfoo-not:vsFOO-NOT:. If so, enforce that they are used consistency. - Decide on if we want to enforce FileCheck directive prefixes with
CHECK-, i.e. instead offoo-NOT:we will always writeCHECK-foo-NOT;. If so, enforce it.
@tgross35 has a prototype PR at https://github.com/rust-lang/rust/pull/128018 for how some of the changes might look like (subject to changes as the unresolved questions getting resolved).
Unresolved questions
- Should FileCheck prefixes be always capitalized regardless of
revisionname capitalization? - Should we make sure revision names (in test suites that uses FileCheck) always be uppercase? Always lowercase?
- Should FileCheck prefixes always begin with
CHECK? I.e. if we have arevisions: foo, instead offoo:orfoo-NOT:we would haveCHECK-foo:andCHECK-foo-NOT:.
Remarks
- We don't currently warn on FileCheck directives without a colon
:: https://github.com/rust-lang/rust/issues/130981 - We don't currently deny special FileCheck prefixes like
CHECK: https://github.com/rust-lang/rust/issues/130982
Mentors or Reviewers
Anyone who writes a lot of assembly and codegen tests for UX feedback in both reading and writing. cc @scottmcm, @workingjubilee, @saethlin, @RalfJung, @DianQK, @nikic, @rust-lang/wg-llvm for advice and preferences.
Process
The main points of the Major Change Process are as follows:
- [x] File an issue describing the proposal.
- [ ] A compiler team member or contributor who is knowledgeable in the area can
second by writing
@rustbot second.- Finding a "second" suffices for internal changes. If however, you are
proposing a new public-facing feature, such as a
-C flag, then full team check-off is required. - Compiler team members can initiate a check-off via
@rfcbot fcp mergeon either the MCP or the PR.
- Finding a "second" suffices for internal changes. If however, you are
proposing a new public-facing feature, such as a
- [ ] Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.
You can read more about Major Change Proposals on forge.
Comments
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.
Concerns or objections to the proposal should be discussed on Zulip and formally registered here by adding a comment with the following syntax:
@rustbot concern reason-for-concern
<description of the concern>
Concerns can be lifted with:
@rustbot resolve reason-for-concern
See documentation at https://forge.rust-lang.org
cc @rust-lang/compiler @rust-lang/compiler-contributors
I'm going to close this as postpone, going to revisit this once the compiletest directive handling is less of a pain.