[FileCheck] forbid filecheck check prefix definitions to end with directive name

[klensy]

Split from #92248

This should resolve ambiguity in cases like: -check-prefixes=FOO,FOO-NEXT

FOO: xxx
FOO-NEXT: yyy

where it unknown how to parse FOO-NEXT: like FOO-NEXT prefix or FOO with -NEXT directive?

Current PR checks only prefix definition, not usages.

[llvmbot]

@llvm/pr-subscribers-objectyaml
@llvm/pr-subscribers-lld-macho
@llvm/pr-subscribers-llvm-binary-utilities
@llvm/pr-subscribers-pgo
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-testing-tools

Author: klensy (klensy)

Changes

Split from #92248

---

Full diff: https://github.com/llvm/llvm-project/pull/92735.diff

4 Files Affected:

• (modified) llvm/lib/FileCheck/FileCheck.cpp (+10)
• (modified) llvm/test/FileCheck/comment/bad-comment-prefix.txt (+5-5)
• (modified) llvm/test/FileCheck/comment/suffixes.txt (+4-4)
• (modified) llvm/test/FileCheck/numeric-expression.txt (+8-8)

diff --git a/llvm/lib/FileCheck/FileCheck.cpp b/llvm/lib/FileCheck/FileCheck.cpp
index 1719f8ef2b436..a584b851a9f02 100644
--- a/llvm/lib/FileCheck/FileCheck.cpp
+++ b/llvm/lib/FileCheck/FileCheck.cpp
@@ -2468,6 +2468,9 @@ FileCheckString::CheckDag(const SourceMgr &SM, StringRef Buffer,
 
 static bool ValidatePrefixes(StringRef Kind, StringSet<> &UniquePrefixes,
                              ArrayRef<StringRef> SuppliedPrefixes) {
+  static const char *Suffixes[] = {"-NEXT",  "-SAME", "-EMPTY", "-NOT",
+                                   "-COUNT", "-DAG",  "-LABEL"};
+
   for (StringRef Prefix : SuppliedPrefixes) {
     if (Prefix.empty()) {
       errs() << "error: supplied " << Kind << " prefix must not be the empty "
@@ -2486,6 +2489,13 @@ static bool ValidatePrefixes(StringRef Kind, StringSet<> &UniquePrefixes,
              << "check and comment prefixes: '" << Prefix << "'\n";
       return false;
     }
+    for (StringRef Suffix : Suffixes) {
+      if (Prefix.ends_with(Suffix)) {
+        errs() << "error: supplied " << Kind << " prefix must not end with "
+               << "directive: '" << Suffix << "', prefix: '" << Prefix << "'\n";
+        return false;
+      }
+    }
   }
   return true;
 }
diff --git a/llvm/test/FileCheck/comment/bad-comment-prefix.txt b/llvm/test/FileCheck/comment/bad-comment-prefix.txt
index 58a8873da3218..19be577618d00 100644
--- a/llvm/test/FileCheck/comment/bad-comment-prefix.txt
+++ b/llvm/test/FileCheck/comment/bad-comment-prefix.txt
@@ -3,17 +3,17 @@
 # Check empty comment prefix.
 RUN: %ProtectFileCheckOutput not FileCheck /dev/null < /dev/null 2>&1 \
 RUN:                                      -comment-prefixes= | \
-RUN:   FileCheck -check-prefix=PREFIX-EMPTY %s
+RUN:   FileCheck -check-prefix=EMPTY-PREFIX %s
 RUN: %ProtectFileCheckOutput not FileCheck /dev/null < /dev/null 2>&1 \
 RUN:                                      -comment-prefixes=,FOO | \
-RUN:   FileCheck -check-prefix=PREFIX-EMPTY %s
+RUN:   FileCheck -check-prefix=EMPTY-PREFIX %s
 RUN: %ProtectFileCheckOutput not FileCheck /dev/null < /dev/null 2>&1 \
 RUN:                                      -comment-prefixes=FOO, | \
-RUN:   FileCheck -check-prefix=PREFIX-EMPTY %s
+RUN:   FileCheck -check-prefix=EMPTY-PREFIX %s
 RUN: %ProtectFileCheckOutput not FileCheck /dev/null < /dev/null 2>&1 \
 RUN:                                      -comment-prefixes=FOO,,BAR | \
-RUN:   FileCheck -check-prefix=PREFIX-EMPTY %s
-PREFIX-EMPTY: error: supplied comment prefix must not be the empty string
+RUN:   FileCheck -check-prefix=EMPTY-PREFIX %s
+EMPTY-PREFIX: error: supplied comment prefix must not be the empty string
 
 # Check invalid characters in comment prefix.
 RUN: %ProtectFileCheckOutput not FileCheck /dev/null < /dev/null 2>&1 \
diff --git a/llvm/test/FileCheck/comment/suffixes.txt b/llvm/test/FileCheck/comment/suffixes.txt
index 85b05fb5778cf..88a5c4cd3bf9e 100644
--- a/llvm/test/FileCheck/comment/suffixes.txt
+++ b/llvm/test/FileCheck/comment/suffixes.txt
@@ -1,5 +1,5 @@
-# Comment prefixes plus check directive suffixes are not comment directives
-# and are treated as plain text.
+# Comment prefixes plus check directive suffixes are forbidden.
+# FIXME: currently not verified bq ValidatePrefixes missing defaulted comment prefixes?
 
 RUN: echo foo                    >  %t.in
 RUN: echo bar                    >> %t.in
@@ -12,11 +12,11 @@ RUN:   FileCheck -check-prefix=CHECK1 %s
 CHECK1: .chk:1:18: remark: CHECK: expected string found in input
 CHECK1: .chk:2:17: remark: CHECK: expected string found in input
 
-# But we can define them as comment prefixes.
+# But we can define them as comment prefixes; still forbidden.
 
 RUN: %ProtectFileCheckOutput \
 RUN: FileCheck -dump-input=never -vv -comment-prefixes=COM,RUN,RUN-NOT %t.chk < %t.in 2>&1 | \
 RUN:   FileCheck -check-prefix=CHECK2 %s
 
-CHECK2: .chk:1:18: remark: CHECK: expected string found in input
+CHECK2: error: supplied comment prefix must not end with directive: '-NOT', prefix: 'RUN-NOT'
 CHECK2-NOT: .chk:2
diff --git a/llvm/test/FileCheck/numeric-expression.txt b/llvm/test/FileCheck/numeric-expression.txt
index 1430484d08ebc..f23628f5fbc9a 100644
--- a/llvm/test/FileCheck/numeric-expression.txt
+++ b/llvm/test/FileCheck/numeric-expression.txt
@@ -593,16 +593,16 @@ CALL-MISSING-ARGUMENT-MSG-NEXT: {{C}}ALL-MISSING-ARGUMENT-NEXT: {{\[\[#add\(NUMV
 CALL-MISSING-ARGUMENT-MSG-NEXT:      {{^}}                                          ^{{$}}
 
 RUN: %ProtectFileCheckOutput \
-RUN: not FileCheck -D#NUMVAR=10 --check-prefix CALL-WRONG-ARGUMENT-COUNT --input-file %s %s 2>&1 \
-RUN:   | FileCheck --strict-whitespace --check-prefix CALL-WRONG-ARGUMENT-COUNT-MSG %s
+RUN: not FileCheck -D#NUMVAR=10 --check-prefix CALL-WRONG-ARGUMENT-NUM --input-file %s %s 2>&1 \
+RUN:   | FileCheck --strict-whitespace --check-prefix CALL-WRONG-ARGUMENT-NUM-MSG %s
 
-CALL WRONG ARGUMENT COUNT
+CALL WRONG ARGUMENT NUM
 30
-CALL-WRONG-ARGUMENT-COUNT-LABEL: CALL WRONG ARGUMENT COUNT
-CALL-WRONG-ARGUMENT-COUNT-NEXT: [[#add(NUMVAR)]]
-CALL-WRONG-ARGUMENT-COUNT-MSG: numeric-expression.txt:[[#@LINE-1]]:36: error: function 'add' takes 2 arguments but 1 given
-CALL-WRONG-ARGUMENT-COUNT-MSG-NEXT: {{C}}ALL-WRONG-ARGUMENT-COUNT-NEXT: {{\[\[#add\(NUMVAR\)\]\]}}
-CALL-WRONG-ARGUMENT-COUNT-MSG-NEXT:    {{^}}                                   ^{{$}}
+CALL-WRONG-ARGUMENT-NUM-LABEL: CALL WRONG ARGUMENT NUM
+CALL-WRONG-ARGUMENT-NUM-NEXT: [[#add(NUMVAR)]]
+CALL-WRONG-ARGUMENT-NUM-MSG: numeric-expression.txt:[[#@LINE-1]]:34: error: function 'add' takes 2 arguments but 1 given
+CALL-WRONG-ARGUMENT-NUM-MSG-NEXT: {{C}}ALL-WRONG-ARGUMENT-NUM-NEXT: {{\[\[#add\(NUMVAR\)\]\]}}
+CALL-WRONG-ARGUMENT-NUM-MSG-NEXT:    {{^}}                                 ^{{$}}
 
 RUN: %ProtectFileCheckOutput \
 RUN: not FileCheck -D#NUMVAR=10 --check-prefix CALL-UNDEFINED-FUNCTION --input-file %s %s 2>&1 \

[github-actions]

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

[klensy]

I'm currently trying to resolve requested changes and fix tests, so no need to review yet.

[pogo59]

My comment in the other patch was about directives read from the file, not command-line arguments, if that's what prompted this. Now, the check added here seems like a reasonable one, so please proceed with it, but be aware it's not what I asked for.

Because there should be no changes to the suffixes.txt test, there needs to be a separate test that the suffixes are rejected.

[pogo59]

I'm currently trying to resolve requested changes and fix tests, so no need to review yet.

Ah sorry, didn't see this comment before I started reviewing. But I hope the comments are helpful anyway.

[klensy]

Added tests and updated doc. Yes, this PR only checks for definition of check prefixes, not using in file, which simpler to start with.

[klensy]

Is current impl are ok and i can start fixing tests?

[klensy]

Currently there error only with that test.