[llvm] fd94103 - Fix PR46880: Fail CHECK-NOT with undefined variable

Thomas Preud'homme via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 20 06:42:31 PDT 2021


Author: Thomas Preud'homme
Date: 2021-04-20T14:42:46+01:00
New Revision: fd941036bf2494cfbd2749f2f179733cbe9e2eb4

URL: https://github.com/llvm/llvm-project/commit/fd941036bf2494cfbd2749f2f179733cbe9e2eb4
DIFF: https://github.com/llvm/llvm-project/commit/fd941036bf2494cfbd2749f2f179733cbe9e2eb4.diff

LOG: Fix PR46880: Fail CHECK-NOT with undefined variable

Currently a CHECK-NOT directive succeeds whenever the corresponding
match fails. However match can fail due to an error rather than a lack
of match, for instance if a variable is undefined. This commit makes match
error a failure for CHECK-NOT.

Reviewed By: jdenny

Differential Revision: https://reviews.llvm.org/D86222

Added: 
    

Modified: 
    llvm/lib/FileCheck/FileCheck.cpp
    llvm/lib/FileCheck/FileCheckImpl.h
    llvm/test/FileCheck/dump-input/annotations.txt
    llvm/test/FileCheck/match-time-error-propagation/invalid-excluded-pattern.txt
    llvm/test/FileCheck/match-time-error-propagation/invalid-expected-pattern.txt
    llvm/test/FileCheck/numeric-expression.txt
    llvm/test/FileCheck/var-scope.txt
    llvm/unittests/FileCheck/FileCheckTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/FileCheck/FileCheck.cpp b/llvm/lib/FileCheck/FileCheck.cpp
index 12a0ae63419f5..950d236717db5 100644
--- a/llvm/lib/FileCheck/FileCheck.cpp
+++ b/llvm/lib/FileCheck/FileCheck.cpp
@@ -531,7 +531,7 @@ Expected<std::unique_ptr<NumericVariableUse>> Pattern::parseNumericVariableUse(
   // we get below is null, it means no such variable was defined before. When
   // that happens, we create a dummy variable so that parsing can continue. All
   // uses of undefined variables, whether string or numeric, are then diagnosed
-  // in printSubstitutions() after failing to match.
+  // in printNoMatch() after failing to match.
   auto VarTableIter = Context->GlobalNumericVariableTable.find(Name);
   NumericVariable *NumericVariable;
   if (VarTableIter != Context->GlobalNumericVariableTable.end())
@@ -1250,6 +1250,7 @@ Pattern::MatchResult Pattern::match(StringRef Buffer,
     // Substitute all string variables and expressions whose values are only
     // now known. Use of string variables defined on the same line are handled
     // by back-references.
+    Error Errs = Error::success();
     for (const auto &Substitution : Substitutions) {
       // Substitute and check for failure (e.g. use of undefined variable).
       Expected<std::string> Value = Substitution->getResult();
@@ -1257,13 +1258,20 @@ Pattern::MatchResult Pattern::match(StringRef Buffer,
         // Convert to an ErrorDiagnostic to get location information. This is
         // done here rather than printMatch/printNoMatch since now we know which
         // substitution block caused the overflow.
-        Error Err =
-            handleErrors(Value.takeError(), [&](const OverflowError &E) {
-              return ErrorDiagnostic::get(SM, Substitution->getFromString(),
-                                          "unable to substitute variable or "
-                                          "numeric expression: overflow error");
-            });
-        return std::move(Err);
+        Errs = joinErrors(std::move(Errs),
+                          handleErrors(
+                              Value.takeError(),
+                              [&](const OverflowError &E) {
+                                return ErrorDiagnostic::get(
+                                    SM, Substitution->getFromString(),
+                                    "unable to substitute variable or "
+                                    "numeric expression: overflow error");
+                              },
+                              [&SM](const UndefVarError &E) {
+                                return ErrorDiagnostic::get(SM, E.getVarName(),
+                                                            E.message());
+                              }));
+        continue;
       }
 
       // Plop it into the regex at the adjusted offset.
@@ -1271,6 +1279,8 @@ Pattern::MatchResult Pattern::match(StringRef Buffer,
                     Value->begin(), Value->end());
       InsertOffset += Value->size();
     }
+    if (Errs)
+      return std::move(Errs);
 
     // Match the newly constructed regex.
     RegExToMatch = TmpStr;
@@ -1349,35 +1359,18 @@ void Pattern::printSubstitutions(const SourceMgr &SM, StringRef Buffer,
     for (const auto &Substitution : Substitutions) {
       SmallString<256> Msg;
       raw_svector_ostream OS(Msg);
-      Expected<std::string> MatchedValue = Substitution->getResult();
 
-      // Substitution failed or is not known at match time, print the undefined
-      // variables it uses.
+      Expected<std::string> MatchedValue = Substitution->getResult();
+      // Substitution failures are handled in printNoMatch().
       if (!MatchedValue) {
-        bool UndefSeen = false;
-        handleAllErrors(
-            MatchedValue.takeError(), [](const NotFoundError &E) {},
-            // Handled in printMatch and printNoMatch().
-            [](const ErrorDiagnostic &E) {},
-            // Handled in match().
-            [](const OverflowError &E) {},
-            [&](const UndefVarError &E) {
-              if (!UndefSeen) {
-                OS << "uses undefined variable(s):";
-                UndefSeen = true;
-              }
-              OS << " ";
-              E.log(OS);
-            });
-        if (!OS.tell())
-          continue;
-      } else {
-        // Substitution succeeded. Print substituted value.
-        OS << "with \"";
-        OS.write_escaped(Substitution->getFromString()) << "\" equal to \"";
-        OS.write_escaped(*MatchedValue) << "\"";
+        consumeError(MatchedValue.takeError());
+        continue;
       }
 
+      OS << "with \"";
+      OS.write_escaped(Substitution->getFromString()) << "\" equal to \"";
+      OS.write_escaped(*MatchedValue) << "\"";
+
       // We report only the start of the match/search range to suggest we are
       // reporting the substitutions as set at the start of the match/search.
       // Indicating a non-zero-length range might instead seem to imply that the
@@ -2140,12 +2133,6 @@ static Error printNoMatch(bool ExpectedMatch, const SourceMgr &SM,
         if (Diags)
           ErrorMsgs.push_back(E.getMessage().str());
       },
-      // UndefVarError is reported in printSubstitutions below.
-      // FIXME: It probably should be handled as a pattern error and actually
-      // change the exit status to 1, even if !ExpectedMatch.  To do so, we
-      // could stop calling printSubstitutions and actually report the error
-      // here as we do ErrorDiagnostic above.
-      [](const UndefVarError &E) {},
       // NotFoundError is why printNoMatch was invoked.
       [](const NotFoundError &E) {});
 

diff  --git a/llvm/lib/FileCheck/FileCheckImpl.h b/llvm/lib/FileCheck/FileCheckImpl.h
index d4d891f7c1f0d..29e721e088d2c 100644
--- a/llvm/lib/FileCheck/FileCheckImpl.h
+++ b/llvm/lib/FileCheck/FileCheckImpl.h
@@ -229,8 +229,7 @@ class UndefVarError : public ErrorInfo<UndefVarError> {
 
   /// Print name of variable associated with this error.
   void log(raw_ostream &OS) const override {
-    OS << "\"";
-    OS.write_escaped(VarName) << "\"";
+    OS << "undefined variable: " << VarName;
   }
 };
 
@@ -756,8 +755,7 @@ class Pattern {
   /// current values of FileCheck numeric variables and is updated if this
   /// match defines new numeric values.
   MatchResult match(StringRef Buffer, const SourceMgr &SM) const;
-  /// Prints the value of successful substitutions or the name of the undefined
-  /// string or numeric variables preventing a successful substitution.
+  /// Prints the value of successful substitutions.
   void printSubstitutions(const SourceMgr &SM, StringRef Buffer,
                           SMRange MatchRange, FileCheckDiag::MatchType MatchTy,
                           std::vector<FileCheckDiag> *Diags) const;

diff  --git a/llvm/test/FileCheck/dump-input/annotations.txt b/llvm/test/FileCheck/dump-input/annotations.txt
index 3118ef67e4d60..45bf54698bca0 100644
--- a/llvm/test/FileCheck/dump-input/annotations.txt
+++ b/llvm/test/FileCheck/dump-input/annotations.txt
@@ -692,22 +692,17 @@
 ; SUBST-POS-NEXT:check:1'0     ^~~~~~~~~~~~~~~~~~~~~
 ; SUBST-POS-NEXT:check:1'1                            with "DEF_MATCH1" equal to "def-match1"
 ; SUBST-POS-NEXT:check:1'2                            with "DEF_MATCH2" equal to "def-match2"
-; SUBST-POS-NEXT:check:2'0                          X error: no match found
-; SUBST-POS-NEXT:check:2'1                            with "DEF_MATCH1" equal to "def-match1"
-; SUBST-POS-NEXT:check:2'2                            uses undefined variable(s): "UNDEF"
+; SUBST-POS-NEXT:check:2'0     X error: match failed for invalid pattern
+; SUBST-POS-NEXT:check:2'1                            undefined variable: UNDEF
+; SUBST-POS-NEXT:check:2'2                            with "DEF_MATCH1" equal to "def-match1"
 ; SUBST-POS-NEXT:check:2'3                            with "DEF_NOMATCH" equal to "foobar"
-; SUBST-POS-NEXT:           2: def-match1 def-nomatch 
+; SUBST-POS-NEXT:           2: def-match1 def-nomatch
 ; SUBST-POS-NEXT:check:2'0     ~~~~~~~~~~~~~~~~~~~~~~~
-; SUBST-POS-NEXT:check:2'4              ?              possible intended match
+; SUBST-POS-NEXT:check:2'4 ? possible intended match
 ; SUBST-POS-NEXT:>>>>>>
 
 ;--------------------------------------------------
 ; Substitutions: successful and failed negative directives.
-;
-; FIXME: The first CHECK-NOT directive below uses an undefined variable.
-; Because it therefore cannot match regardless of the input, the directive
-; succeeds.  However, it should fail in this case.  Update the example below
-; once that's fixed.
 ;--------------------------------------------------
 
 ; RUN: echo 'def-match1 def-nomatch' >  %t.in
@@ -725,17 +720,17 @@
 ; RUN: | FileCheck -match-full-lines %s -check-prefix=SUBST-NEG
 
 ;      SUBST-NEG:<<<<<<
-; SUBST-NEG-NEXT:         1: def-match1 def-nomatch 
-; SUBST-NEG-NEXT:not:1'0     X~~~~~~~~~~~~~~~~~~~~~~
-; SUBST-NEG-NEXT:not:1'1                             with "DEF_MATCH1" equal to "def-match1"
-; SUBST-NEG-NEXT:not:1'2                             uses undefined variable(s): "UNDEF"
+; SUBST-NEG-NEXT:         1: def-match1 def-nomatch
+; SUBST-NEG-NEXT:not:1'0     X~~~~~~~~~~~~~~~~~~~~~~ error: match failed for invalid pattern
+; SUBST-NEG-NEXT:not:1'1                             undefined variable: UNDEF
+; SUBST-NEG-NEXT:not:1'2                             with "DEF_MATCH1" equal to "def-match1"
 ; SUBST-NEG-NEXT:not:1'3                             with "DEF_NOMATCH" equal to "foobar"
-; SUBST-NEG-NEXT:         2: def-match1 def-match2 
+; SUBST-NEG-NEXT:         2: def-match1 def-match2
 ; SUBST-NEG-NEXT:not:1'0     ~~~~~~~~~~~~~~~~~~~~~~
 ; SUBST-NEG-NEXT:not:2'0     !~~~~~~~~~~~~~~~~~~~~  error: no match expected
 ; SUBST-NEG-NEXT:not:2'1                            with "DEF_MATCH1" equal to "def-match1"
 ; SUBST-NEG-NEXT:not:2'2                            with "DEF_MATCH2" equal to "def-match2"
-; SUBST-NEG-NEXT:         3: END 
+; SUBST-NEG-NEXT:         3: END
 ; SUBST-NEG-NEXT:check:3     ^~~
 ; SUBST-NEG-NEXT:>>>>>>
 

diff  --git a/llvm/test/FileCheck/match-time-error-propagation/invalid-excluded-pattern.txt b/llvm/test/FileCheck/match-time-error-propagation/invalid-excluded-pattern.txt
index 4513fdc4f8fc6..a40597d90d053 100644
--- a/llvm/test/FileCheck/match-time-error-propagation/invalid-excluded-pattern.txt
+++ b/llvm/test/FileCheck/match-time-error-propagation/invalid-excluded-pattern.txt
@@ -20,8 +20,8 @@ ERR-VV-EMPTY:
          ERR:{{.*}}: error: unable to substitute variable or numeric expression: overflow error
     ERR-NEXT:CHECK-NOT: {{.*}}
     ERR-NEXT:{{ *}}^
-    ERR-NEXT:<stdin>:1:1: note: uses undefined variable(s): "UNDEFVAR"
-    ERR-NEXT:10000000000000000
+    ERR-NEXT:{{.*}}: error: undefined variable: UNDEFVAR
+    ERR-NEXT:{{CHECK-NOT: \[\[#0x8000000000000000\+0x8000000000000000\]\] \[\[UNDEFVAR\]\]}}
     ERR-NEXT:^
      ERR-NOT:{{error|note|remark}}:
 
@@ -29,7 +29,7 @@ ERR-VV-EMPTY:
    DUMP-NEXT:         1: 10000000000000000 
    DUMP-NEXT:not:1'0     X~~~~~~~~~~~~~~~~~ error: match failed for invalid pattern
    DUMP-NEXT:not:1'1                        unable to substitute variable or numeric expression: overflow error
-   DUMP-NEXT:not:1'2                        uses undefined variable(s): "UNDEFVAR"
+   DUMP-NEXT:not:1'2                        undefined variable: UNDEFVAR
 DUMP-VV-NEXT:         2: 
 DUMP-VV-NEXT:eof:1       ^
    DUMP-NEXT:>>>>>>

diff  --git a/llvm/test/FileCheck/match-time-error-propagation/invalid-expected-pattern.txt b/llvm/test/FileCheck/match-time-error-propagation/invalid-expected-pattern.txt
index 6c55b545c031e..577f018e4782f 100644
--- a/llvm/test/FileCheck/match-time-error-propagation/invalid-expected-pattern.txt
+++ b/llvm/test/FileCheck/match-time-error-propagation/invalid-expected-pattern.txt
@@ -9,8 +9,8 @@ RUN: echo > %t.in '10000000000000000'
       ERR:{{.*}}: error: unable to substitute variable or numeric expression: overflow error
  ERR-NEXT:CHECK: {{.*}}
  ERR-NEXT:{{ *}}^
- ERR-NEXT:<stdin>:1:1: note: uses undefined variable(s): "UNDEFVAR"
- ERR-NEXT:10000000000000000
+ ERR-NEXT:{{.*}}: error: undefined variable: UNDEFVAR
+ ERR-NEXT:{{CHECK: \[\[#0x8000000000000000\+0x8000000000000000\]\] \[\[UNDEFVAR\]\]}}
  ERR-NEXT:^
   ERR-NOT:{{error|note|remark}}:
 
@@ -18,7 +18,7 @@ RUN: echo > %t.in '10000000000000000'
 DUMP-NEXT:           1: 10000000000000000 
 DUMP-NEXT:check:1'0     X~~~~~~~~~~~~~~~~~ error: match failed for invalid pattern
 DUMP-NEXT:check:1'1                        unable to substitute variable or numeric expression: overflow error
-DUMP-NEXT:check:1'2                        uses undefined variable(s): "UNDEFVAR"
+DUMP-NEXT:check:1'2                        undefined variable: UNDEFVAR
 DUMP-NEXT:>>>>>>
 
 ;--------------------------------------------------

diff  --git a/llvm/test/FileCheck/numeric-expression.txt b/llvm/test/FileCheck/numeric-expression.txt
index 0a7f2f3d2255c..d76552651ef8a 100644
--- a/llvm/test/FileCheck/numeric-expression.txt
+++ b/llvm/test/FileCheck/numeric-expression.txt
@@ -400,15 +400,29 @@ UNDEF VAR USE
 UNDEFVAR: 11
 UNDEF-USE-LABEL: UNDEF VAR USE
 UNDEF-USE-NEXT: UNDEFVAR: [[#UNDEFVAR1+UNDEFVAR2]]
-UNDEF-USE-MSG: numeric-expression.txt:[[#@LINE-1]]:17: error: {{U}}NDEF-USE-NEXT: expected string not found in input
-UNDEF-USE-MSG-NEXT:      {{U}}NDEF-USE-NEXT: UNDEFVAR: {{\[\[#UNDEFVAR1\+UNDEFVAR2\]\]}}
-UNDEF-USE-MSG-NEXT:     {{^}}                ^{{$}}
-UNDEF-USE-MSG-NEXT:      numeric-expression.txt:[[#@LINE-7]]:14: note: scanning from here
-UNDEF-USE-MSG-NEXT:      UNDEF VAR USE
-UNDEF-USE-MSG-NEXT: {{^}}             ^{{$}}
-UNDEF-USE-MSG-NEXT:      numeric-expression.txt:[[#@LINE-10]]:14: note: uses undefined variable(s): "UNDEFVAR1" "UNDEFVAR2"
-UNDEF-USE-MSG-NEXT:      UNDEF VAR USE
-UNDEF-USE-MSG-NEXT: {{^}}             ^{{$}}
+UNDEF-USE-MSG:      numeric-expression.txt:[[#@LINE-1]]:30: error: undefined variable: UNDEFVAR1
+UNDEF-USE-MSG-NEXT:      {{U}}NDEF-USE-NEXT: {{U}}NDEFVAR: {{\[\[#UNDEFVAR1\+UNDEFVAR2\]\]}}
+UNDEF-USE-MSG-NEXT:             {{^}}                             ^{{$}}
+UNDEF-USE-MSG-NEXT:      numeric-expression.txt:[[#@LINE-4]]:40: error: undefined variable: UNDEFVAR2
+UNDEF-USE-MSG-NEXT:      {{U}}NDEF-USE-NEXT: {{U}}NDEFVAR: {{\[\[#UNDEFVAR1\+UNDEFVAR2\]\]}}
+UNDEF-USE-MSG-NEXT:              {{^}}                                       ^{{$}}
+
+; Numeric expression in negative directive using undefined variables.
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck --check-prefix UNDEF-USE2 --input-file %s %s 2>&1 \
+RUN:   | FileCheck --strict-whitespace --check-prefix UNDEF-USE-MSG2 %s
+
+CHECK NOT UNDEF VAR USE
+END MARKER
+UNDEF-USE2-LABEL: CHECK NOT UNDEF VAR USE
+UNDEF-USE2-NOT: UNDEFVAR: [[#UNDEFVAR1+UNDEFVAR2]]
+UNDEF-USE2: END MARKER
+UNDEF-USE-MSG2:      numeric-expression.txt:[[#@LINE-2]]:30: error: undefined variable: UNDEFVAR1
+UNDEF-USE-MSG2-NEXT:      {{U}}NDEF-USE2-NOT: {{U}}NDEFVAR: {{\[\[#UNDEFVAR1\+UNDEFVAR2\]\]}}
+UNDEF-USE-MSG2-NEXT:             {{^}}                             ^{{$}}
+UNDEF-USE-MSG2-NEXT:      numeric-expression.txt:[[#@LINE-5]]:40: error: undefined variable: UNDEFVAR2
+UNDEF-USE-MSG2-NEXT:      {{U}}NDEF-USE2-NOT: {{U}}NDEFVAR: {{\[\[#UNDEFVAR1\+UNDEFVAR2\]\]}}
+UNDEF-USE-MSG2-NEXT:              {{^}}                                       ^{{$}}
 
 ; Numeric expression with unsupported operator.
 RUN: %ProtectFileCheckOutput \

diff  --git a/llvm/test/FileCheck/var-scope.txt b/llvm/test/FileCheck/var-scope.txt
index e45610b138ef8..9b3ea0e95d143 100644
--- a/llvm/test/FileCheck/var-scope.txt
+++ b/llvm/test/FileCheck/var-scope.txt
@@ -7,11 +7,11 @@ RUN: FileCheck --check-prefixes CHECK,LOCAL3,GLOBAL --input-file %s %s
 
 RUN: FileCheck --check-prefixes CHECK,GLOBAL --enable-var-scope --input-file %s %s
 RUN: %ProtectFileCheckOutput not FileCheck --check-prefixes CHECK,LOCAL1 --enable-var-scope --input-file %s %s 2>&1 \
-RUN:   | FileCheck --check-prefixes ERRUNDEF,ERRUNDEFLOCAL %s
+RUN:   | FileCheck --check-prefix ERRUNDEFLOCAL %s
 RUN: %ProtectFileCheckOutput not FileCheck --check-prefixes CHECK,LOCAL2 --enable-var-scope --input-file %s %s 2>&1 \
-RUN:   | FileCheck --check-prefixes ERRUNDEF,ERRUNDEFLOCNUM %s
+RUN:   | FileCheck --check-prefix ERRUNDEFLOCNUM %s
 RUN: %ProtectFileCheckOutput not FileCheck --check-prefixes CHECK,LOCAL3 --enable-var-scope --input-file %s %s 2>&1 \
-RUN:   | FileCheck --check-prefixes ERRUNDEF,ERRUNDEFLOCAL,ERRUNDEFLOCNUM %s
+RUN:   | FileCheck --check-prefixes ERRUNDEFLOCAL,ERRUNDEFLOCNUM %s
 
 local1
 global1
@@ -33,6 +33,5 @@ LOCAL2: local[[#LOCNUM+2]]
 LOCAL3: [[LOCAL]][[#LOCNUM+2]]
 GLOBAL: [[$GLOBAL]][[#$GLOBNUM+2]]
 
-ERRUNDEF: expected string not found in input
-ERRUNDEFLOCAL: uses undefined variable(s): "LOCAL"
-ERRUNDEFLOCNUM: uses undefined variable(s): "LOCNUM"
+ERRUNDEFLOCAL: undefined variable: LOCAL
+ERRUNDEFLOCNUM: undefined variable: LOCNUM

diff  --git a/llvm/unittests/FileCheck/FileCheckTest.cpp b/llvm/unittests/FileCheck/FileCheckTest.cpp
index 59befc0efb8a5..60ada3836a6a1 100644
--- a/llvm/unittests/FileCheck/FileCheckTest.cpp
+++ b/llvm/unittests/FileCheck/FileCheckTest.cpp
@@ -1445,8 +1445,10 @@ TEST_F(FileCheckTest, Match) {
                        Succeeded());
   Tester.initNextPattern();
   // Match with substitution failure.
-  ASSERT_FALSE(Tester.parsePattern("[[#UNKNOWN]]"));
-  expectUndefErrors({"UNKNOWN"}, Tester.match("FOO").takeError());
+  ASSERT_FALSE(Tester.parsePattern("[[#UNKNOWN1+UNKNOWN2]]"));
+  expectSameErrors<ErrorDiagnostic>(
+      {"undefined variable: UNKNOWN1", "undefined variable: UNKNOWN2"},
+      Tester.match("FOO").takeError());
   Tester.initNextPattern();
   // Check that @LINE matches the later (given the calls to initNextPattern())
   // line number.


        


More information about the llvm-commits mailing list