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

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
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;
----------------
jhenderson wrote:
> 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?
Yup - fixed.


================
Comment at: llvm/test/FileCheck/allow-unused-prefixes.txt:1
+// 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:
> You probably need to test the error reported?
Added for both single and multiple missing prefixes. Note that, because the way prefixes are reported, it required a separation of the test input and this verification. (otherwise <comment> .* Prefix:  would match to a prefix)


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90281