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

1 files changed, 26 insertions, 39 deletions

diff --git a/llvm/lib/FileCheck/FileCheck.cpp b/llvm/lib/FileCheck/FileCheck.cpp
index 12a0ae6..950d236 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) {});