[PATCH] Allow multiple check prefixes in FileCheck

Daniel Sanders daniel.sanders at imgtec.com
Wed Sep 25 07:15:45 PDT 2013


  Hi,

  Thanks for the updated patch. It mostly looks good to me. My only concern is that check-dag-substring-prefix.txt passes and I don't think it should. This test only has almost-matches in it (because genuine matches need to start on a word boundary), yet it doesn't trigger the 'no check strings found' error. I believe the problem is that the CHECK-EOF is inserted before CheckStrings is checked for emptiness.


================
Comment at: test/FileCheck/check-dag-substring-prefix.txt:1-7
@@ +1,7 @@
+; RUN: FileCheck -check-prefix=A -check-prefix=AA -input-file %s %s
+
+this is the string to be matched
+this should also be matched
+
+; BAA-DAG: this is the string to be {{matched}}
+; BAA-DAG: this should also be {{matched}}
----------------
I don't think test/FileCheck/check-dag-substring-prefix.txt should pass as it currently is. I think it should fail with the 'no check strings found' error. The second RUN line of test/FileCheck/check-prefixes.txt checks that this happens for single prefixes.

================
Comment at: utils/FileCheck/FileCheck.cpp:38
@@ -36,2 +37,3 @@
 
+
 static cl::opt<std::string>
----------------
Extra blank line?

================
Comment at: utils/FileCheck/FileCheck.cpp:751
@@ +750,3 @@
+// of another, the maximal match should be found. e.g. if "A" and "AA" are
+// prefixes are AA-CHECK: should match the second one.
+static StringRef FindFirstCandidateMatch(StringRef &Buffer,
----------------
I think that 'are' was meant to be 'a' or 'then'.

================
Comment at: utils/FileCheck/FileCheck.cpp:1182
@@ +1181,3 @@
+static bool ValidateCheckPrefixes() {
+  StringSet<> PrefixSet;
+
----------------
What is this variable for? I can only see strings being added to it. There doesn't seem to be anything reading from this variable.

================
Comment at: utils/FileCheck/FileCheck.cpp:930-936
@@ -816,7 +929,9 @@
+  // prefix as a filler for the error message.
   if (!DagNotMatches.empty()) {
     CheckStrings.push_back(CheckString(Pattern(Check::CheckEOF),
+                                       CheckPrefixes[0],
                                        SMLoc::getFromPointer(Buffer.data()),
                                        Check::CheckEOF));
     std::swap(DagNotMatches, CheckStrings.back().DagNotStrings);
   }
 
----------------
This if-statement is making CheckStrings non-empty which inhibits the 'no check strings found' even when no check-strings are found.


http://llvm-reviews.chandlerc.com/D1374



More information about the llvm-commits mailing list