[PATCH] D90281: [FileCheck] Report missing prefixes when more than one is provided.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Please also update the FileCheck documentation with the new option.



================
Comment at: llvm/lib/FileCheck/FileCheck.cpp:1941-1942
       (ImplicitNegativeChecks.empty() || !Req.IsDefaultCheckPrefix)) {
     errs() << "error: no check strings found with prefix"
            << (Req.CheckPrefixes.size() > 1 ? "es " : " ");
+    bool First = true;
----------------
If I follow this correctly, let's say I have specified `--check-prefixes=FOO,BAR`, and then only use `FOO`, I'll get the message `error: no check strings found with prefixes 'BAR'`, but really it should be `with prefix 'BAR'` (i.e. no "es"). The logic here probably wants to be using `PrefixesNotFound.size()`, right?


================
Comment at: llvm/test/FileCheck/allow-unused-prefixes.txt:2
+// RUN: not FileCheck  --allow-unused-prefixes=false --check-prefixes=P1,P2 --input-file %s %s
+// RUN: FileCheck --allow-unused-prefixes=true  --check-prefixes=P1,P2 --input-file %s %s 
+
----------------
grimar wrote:
> I think it also worth to add a test for the default value, i.e. when no `--allow-unused-prefixes` is present.



================
Comment at: llvm/test/FileCheck/allow-unused-prefixes.txt:4
+
+hello
+; P1: hello
----------------
I don't think you need this line, because the P1 line will match itself (by default FileCheck doesn't require full line matches).


================
Comment at: llvm/test/FileCheck/allow-unused-prefixes.txt:5
+hello
+; P1: hello
----------------
Be consistent with your "comment" markers - you've used both ';' and '//' in this file. They're actually not needed here at all, but are probably useful to help the RUN/CHECK lines stand out from the rest of the data.


================
Comment at: llvm/test/Transforms/NewGVN/todo-pr37121-seens-this-value-a-lot.ll:3
 
-; RUN: opt -newgvn -S %s | FileCheck %s
+; RUN: not opt -newgvn -S %s
 
----------------
Seems unrelated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90281