[RFC] Using --match-full-lines in Clang-Tidy tests

Clang Frontend clang-tidy
1

Today, Clang-Tidy CHECK-FIXES clauses are checked with FileCheck like this:

def check_fixes(self):
    if self.has_check_fixes:
        try_run(
            [
                "FileCheck",
                "-input-file=" + self.temp_file_name,
                self.input_file_name,
                "-check-prefixes=" + ",".join(self.fixes.prefixes),
                "-strict-whitespace",
            ]
        )

This matches partial lines, which causes two issues:

  1. Hacks to force full line matches: adding {{^}} and {{$}} to each directive, or wrapping them in {{^...$}} blocks. Example 1, Example 2, Example 3, Example 4.
  2. Some typos (mostly due to copy-pasting) fall through the cracks, and can give a false sense that the code works in cases it might not. Example 1, which I wrote, Example 2.

FileCheck has a fix for this! --match-full-lines forces entries to contain the entire line.

I think the following changes would be nice:

  1. Remove --strict-whitespace altogether. It clashes with --match-full-lines because of indentation. Only one current test fails with it off, and in that test --match-full-lines would provide the desired functionality. It can be made a script option in the future if a test requires it.
  2. Enable --match-full-lines by default. Make a --match-partial-fixes script option, add it to the invocation of all tests that currently fail with --match-full-lines.
  3. Gradually, edit existing tests to work with --match-full-lines. This represents 94/837 tests. I’ve looked at a good number of them, changes are minor for most and consist of removing {{^}} clauses, or adding a few characters so the entire line is matched (;, return, }, …). There are a few tests with long lines where the partial match may be desired.

Any thoughts or support for these changes?

2 Likes
2

LGTM, anything that can help reduce the possibility of bugs/mistakes/typos is great!

1 Like
3

Published a PR here: [clang-tidy] Use --match-full-lines instead of --strict-whitespace in check_clang_tidy by nicovank · Pull Request #133756 · llvm/llvm-project · GitHub