[PATCH] D110798: [NFC] Use CHECK-NEXT instead of CHECK-SAME in target-invalid-cpu-note.c

David Spickett via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 30 01:38:27 PDT 2021


DavidSpickett added a comment.

> I found the old way cannot verify if there are some extra outputs between two different CHECK-SAME. So I changed to CHECK-NEXT.

In principle I agree but did you have this failure mode actually happen?

> But it will introduce bad format issue. Anyway, the old way has broken clang-format already. So I would prefer the CHECK-NEXT. WDYT?

Not sure I like crazy long lines, but I see that -NEXT then -SAME would fall to the same issue.

Arm targets are just checking that we print *some* list of CPUs, others are putting the full list. Which isn't great because if you add a new CPU it's possible you'll not get a failure here. I looked for other tests that might check the exact set of CPUs but this is the only one.

I think a reasonable compromise is to -NEXT the `note: valid target CPU values are: <at least one cpu name>` then -SAME the rest. Check the last line ends in `{{$}}`. That limits where extra stuff can sneak in and means you can read the file and it's failure output more easily. (each -SAME line has multiple CPUs on it so that limits how much can be missed)

If you feel like adding the full CPU list to the Arm targets go ahead.

> Anyway, the old way has broken clang-format already.

What does clang-format complain about? This is a test file so formatting is less of a concern than being readable for maintainers and having useful FileCheck output. Splitting the matches enables that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110798/new/

https://reviews.llvm.org/D110798



More information about the cfe-commits mailing list