[FileCheck] Fix numeric error propagation

[jdenny]

Summary

A more general name might be match-time error propagation. That is,
it's conceivable we'll one day have non-numeric errors that require
the handling fixed by this patch.

Without this patch, FileCheck behaves as follows:

$ cat check
CHECK-NOT: [[#0x8000000000000000+0x8000000000000000]]

$ FileCheck -vv -dump-input=never check < input
check:1:54: remark: implicit EOF: expected string found in input
CHECK-NOT: [[#0x8000000000000000+0x8000000000000000]]
                                                     ^
<stdin>:2:1: note: found here

^
check:1:15: error: unable to substitute variable or numeric expression: overflow error
CHECK-NOT: [[#0x8000000000000000+0x8000000000000000]]
              ^
$ echo $?
0

Notice that the exit status is 0 even though there's an error.
Moreover, FileCheck doesn't print the error diagnostic unless both
-dump-input=never and -vv are specified.

The same problem occurs when CHECK-NOT does have a match but a
capture fails due to overflow: exit status is 0, and no diagnostic is
printed unless both -dump-input=never and -vv are specified. The
usefulness of capturing from CHECK-NOT is questionable, but this
case should certainly produce an error.

With this patch, FileCheck always includes the error diagnostic and
has non-zero exit status for the above examples. It's conceivable
that this change will cause some existing tests to fail, but my
assumption is that they should fail. Moreover, with nearly every
project enabled, this patch didn't produce additional check-all
failures for me.

This patch also extends input dumps to include such numeric error
diagnostics for both expected and excluded patterns.

As noted in fixmes in some of the tests added by this patch, this
patch worsens an existing issue with redundant diagnostics. I'll fix
that bug in a subsequent patch.

[jdenny]

This patch also extends input dumps to include similar error
diagnostics.

I realize now that the wording in this patch isn't general enough: it handles not only pattern errors but also overflow errors due to large values in the input. I should extend the tests accordingly. So far, I believe the logic in this patch is correct.

[jdenny]

Actually, I do need to rethink the logic for that overflow case: PrintNoMatch is being called after a match, so its diagnostics don't make sense in that case. Sorry for the noise. I'll work on it.

[jdenny]

I've generalized this patch to also handle errors that occur after matching. One issue is that PrintNoMatch was handling those, but I moved the handling to PrintMatch so that we actually report the full match that was found. That seems like useful information and helps with -dump-input.

Also, I started renaming functions I modified significantly to follow the current LLVM style: printNoMatch and printMatch. I knows some LLVM projects update style incrementally like this. I forget if we decided to do that in FileCheck, but I'm happy to revert that part if people don't want to.

[thopre]

I like the idea behind it. I suppose that's what you had in mind when you wrote your comments in https://reviews.llvm.org/D86222. It should make that patch a lot simpler. Thanks and sorry I didn't progress faster on that one.

llvm/include/llvm/FileCheck/FileCheck.h

137–141

May I suggest making it clear that this error is despite the match being successful? It was not obvious to me until I read the rest of the patch.

llvm/lib/FileCheck/FileCheck.cpp

1194–1199

Nit

2091–2120

Why the reordering of MatchError(s)?

2434–2441

Shouldn't we delegate to printMatch to check for Req.VerboseVerbose?

2491–2492

Likewise.

llvm/lib/FileCheck/FileCheckImpl.h

701–713

Move below above MatchResult.

715–716

[jdenny]

In D98086#2611178, @thopre wrote:

I like the idea behind it. I suppose that's what you had in mind when you wrote your comments in https://reviews.llvm.org/D86222. It should make that patch a lot simpler.

Wow. That patch was clearly lurking in my subconscious, but my conscious mind had forgotten it. I even replicated the PrintNoMatch -> printNoMatch change! I did notice a strange feeling of deja vu while I was working on the patch.

Thanks and sorry I didn't progress faster on that one.

No worries, and thanks for the reminder. If there are ideas in that patch that belong here, please let me know.

I've replied to some comments, and I'll work on the others later.

llvm/include/llvm/FileCheck/FileCheck.h

137–141

Does the following wording help?

"Indicates an error while processing a match after it was found for an expected or excluded pattern."

I'm hesitant to use the word success because (1) there's an error and (2) matching is an error anyway if the pattern is excluded.

llvm/lib/FileCheck/FileCheck.cpp

2091–2120

I was going for symmetry with printMatch, for which any error is included in the MatchResult parameter in that position. To say it another way, that position holds the "result" in both cases. If that doesn't look good to you, I'm fine to revert.

2434–2441

There are two printMatch calls in this function. The first triggers on (an error or) Req.VerboseVerbose, which reports CHECK-DAG discarded matches. The second triggers on !Req.VerboseVerbose to be sure the final CHECK-DAG match is reported when discarded matches aren't shown. We'd have to tell printMatch which case this is, but it seems easier just to keep that logic in the caller.

I'm open to changing it, but what I'm imagining doesn't seem as clean.

llvm/lib/FileCheck/FileCheckImpl.h

701–713

Thanks for catching that.

Actually, this comment is left over from a previous attempt and is now wrong in either place. That is, in the case of an error, MatchResult might not have a match.

The comment preceding match below says it better. What if we just drop the above comment?

[thopre]

llvm/include/llvm/FileCheck/FileCheck.h

137–141

Sure, LGTM.

llvm/lib/FileCheck/FileCheck.cpp

2091–2120

Ah no that's fine, I like symmetry. It just felt like a spurious change but I'm happy with the motivation.

2434–2441

Ah sorry, I missed the negation in the second call. Nevermind then

llvm/lib/FileCheck/FileCheckImpl.h

701–713

I'm fine with that. After all the structure is fairly self explanatory (Optional match, an error which we know can be success and it's named MatchResult)

llvm/test/FileCheck/match-time-error-propagation/invalid-excluded-pattern.txt

14

Did you intend to have two undefined variables (UNDEF and VAR)? Why not just UNDEFVAR? I don't think the parser looks anything after the minus sign.

llvm/test/FileCheck/match-time-error-propagation/invalid-expected-pattern.txt

10

Same as for CHECK-NOT testcase: UNDEFVAR

llvm/test/FileCheck/match-time-error-propagation/matched-excluded-pattern.txt

11–19

FYI I'd like to make a patch to use APInt in FileCheck to allow numbers bigger than 64bits. I've found the need while working on __builtin_isnan for long double. No timeline and I don't expect to have time on it soon but that would remove the only post-match error unless a new one is added before that. On the other hand I haven't even started that work so I think this testcase should be kept.

[jdenny]

Applied @thopre's suggestions.

[jdenny]

llvm/test/FileCheck/match-time-error-propagation/invalid-excluded-pattern.txt

14

Good catch. I was trying to hyphenate. I'll go with UNDEFVAR.

(BTW, the parser doesn't complain when UNDEF is defined. It just ignores -VAR. We should fix that.)

llvm/test/FileCheck/match-time-error-propagation/matched-excluded-pattern.txt

11–19

Thanks for mentioning it. Inequality constraints will fit here, right? If so, hopefully they'll come first so it's easy to maintain this handling.

[jdenny]

llvm/test/FileCheck/match-time-error-propagation/matched-excluded-pattern.txt

11–19

Actually, based on the way == works, I suppose not.

[thopre]

LGTM but maybe you want feedback from James, though he appears very busy

llvm/test/FileCheck/match-time-error-propagation/matched-excluded-pattern.txt

11–19

Inequality will need code needed for using a variable defined on the same line: the ability of tentatively match a line, set variables etc. and discard if a constraint is not met. I have an old patch that does it but I wanted something more automatic (i.e. require a transaction object to perform a match with a commit method and the destructor would delete everything unless committed. That's the general idea but everytime I try to compartmentalize FileCheck more (e.g. creating a numeric expression parser object) I get into an encapsulation hell with info I need but not easily available. Sorry for renting

Anyway, perhaps it's time for me to get back to the use of var defined on the same line and get closure on that FileCheck numeric expression patchset.

[thopre]

llvm/test/FileCheck/match-time-error-propagation/invalid-excluded-pattern.txt

14

Ouch. Could you create a bug and subscribe me?

[jdenny]

llvm/test/FileCheck/match-time-error-propagation/matched-excluded-pattern.txt

11–19

I agree that FileCheck internals could benefit from some refactoring.

I now see three potential uses cases for these after-match errors (represented by MatchFoundErrorNote and reported by printMatch in this patch):

1. FileCheck internal limitations: the pattern matches as specified but FileCheck cannot handle it. The only limitation right now is overflow/underflow, but that could be eliminated in the future, as you have pointed out.

2. A user-visible two-stage check of a directive's constraints: instead of searching for a match that satisfies all constraints of the directive, it searches for a match that satisfies only some constraints and then fails if the remaining constraints aren't satisfied by that match. This would be confusing in ways we've discussed before. For example, we once discussed doing this for a variable captured and later used in an expression within the same directive. Given the input 1 0 2, the following directive would then fail (0 isn't 1 + 1), but it seems better for it to succeed because, intuitively, the pattern should match (2 is 1 + 1):
   
   CHECK: [[%u,X:]] {{.*}} [[%u,X+1]]
   
   It's especially confusing how CHECK-NOT should then behave: with the current error handling in this patch, CHECK-NOT would also fail on a match not satisfying the remaining constraints. Is that really the meaning of CHECK-NOT? So, use case 2 seems wrong, and your comments suggest you're not planning to pursue it for this example. From the user's view, it will then seem like one stage.

3. What about division by zero for a variable captured and used in the same directive? For example, given the input 0 / 0 = 0, should the following produce an after-match error, or should FileCheck keep looking for another match?
   
   CHECK: [[#%u,X:]] / [[#%u,Y:]] = [[%u:X/Y]]
   
   What should CHECK-NOT do? It seems to me FileCheck should fail in both cases because it cannot compute what should actually match. What do you think?

[jdenny]

llvm/test/FileCheck/match-time-error-propagation/invalid-excluded-pattern.txt

14

https://bugs.llvm.org/show_bug.cgi?id=49531

[jhenderson]

In D98086#2617788, @thopre wrote:

LGTM but maybe you want feedback from James, though he appears very busy

Yes, sorry, been busy. I was off sick much of last week, and am participating in the ACCU conference most of this week. I'll try to find time to look at this next week, if you want my input, but there's no need to wait on me otherwise.

[jdenny]

In D98086#2618793, @jhenderson wrote:

Yes, sorry, been busy. I was off sick much of last week, and am participating in the ACCU conference most of this week. I'll try to find time to look at this next week, if you want my input, but there's no need to wait on me otherwise.

Glad you're better! I'd be happy to have your input. I'll wait a week or so in case you have time. Of course, post-commit reviews are fine too.

[jhenderson]

I've run out of steam as I started looking at this, partly because I'm getting hung up on the bool return indicating some kind of errors. This is to me a significant design smell, and I'd prefer a different approach, although I haven't got any concrete suggestions.

llvm/lib/FileCheck/FileCheck.cpp

1376

Aside: this is a good example of where auto is hurting readability of the code. I can't tell whether VariableDef is an Optional, or something else with a getValue() method below. If it's the former, you could replace getValue() with -> for example.

2012

I'm really not a fan of bool return types to indicate an error. Does true indicate an error or false, for example? There's also nothing enforcing that the error is checked (see my "Error handling in libraries" lightning talk from LLVM Dev 2018 for more).

2141

StringRef?

2240

A good example of my concern above: printNoMatch returns a bool, but you do nothing with it here. Should you? If not, why not?

[jdenny]

Address @jhenderson's review.

printMatch and printNoMatch now return either ErrorSuccess or ErrorReported, which is a new class so that these functions can inform their callers that some error has occurred but has already been reported. In most cases, I've wrapped the printMatch and printNoMatch calls in reportMatchResult, which cleans up the callers.

llvm/lib/FileCheck/FileCheck.cpp

1376

Perhaps we need a patch that adjusts all such occurrences in this file.

[jhenderson]

I've gone through the code and briefly skimmed the tests. I need to focus on clearing other reviews, so won't spend more time on this, and I didn't see any further problems, so LGTM.

llvm/lib/FileCheck/FileCheck.cpp

1376

+1 to that suggestion.

[jdenny]

Thanks for the review. I'm rebasing and retesting, and then I'll push.

llvm/include/llvm/FileCheck/FileCheck.h

struct FileCheckDiag {
   /// Where is the FileCheck directive for this diagnostic?
   SMLoc CheckLoc;
   /// What type of match result does this diagnostic describe?
   ///
   /// A directive's supplied pattern is said to be either expected or excluded
   /// depending on whether the pattern must have or must not have a match in
   /// order for the directive to succeed.  For example, a CHECK directive's
   /// pattern is expected, and a CHECK-NOT directive's pattern is excluded.
-  /// All match result types whose names end with "Excluded" are for excluded
-  /// patterns, and all others are for expected patterns.
   ///
   /// There might be more than one match result for a single pattern.  For
   /// example, there might be several discarded matches
   /// (MatchFoundButDiscarded) before either a good match
   /// (MatchFoundAndExpected) or a failure to match (MatchNoneButExpected),
   /// and there might be a fuzzy match (MatchFuzzy) after the latter.
   enum MatchType {
     /// Indicates a good match for an expected pattern.
     MatchFoundAndExpected,
     /// Indicates a match for an excluded pattern.
     MatchFoundButExcluded,
     /// Indicates a match for an expected pattern, but the match is on the
     /// wrong line.
     MatchFoundButWrongLine,
     /// Indicates a discarded match for an expected pattern.
     MatchFoundButDiscarded,
+    /// Indicates an error while processing a match after the match was found
+    /// for an expected or excluded pattern.  The error is specified by \c Note,
+    /// to which it should be appropriate to prepend "error: " later.  The full
+    /// match itself should be recorded in a preceding diagnostic of a different
+    /// \c MatchFound match type.
+    MatchFoundErrorNote,
     /// Indicates no match for an excluded pattern.
     MatchNoneAndExcluded,
     /// Indicates no match for an expected pattern, but this might follow good
     /// matches when multiple matches are expected for the pattern, or it might
     /// follow discarded matches for the pattern.
     MatchNoneButExpected,
+    /// Indicates no match due to an expected or excluded pattern that has
+    /// proven to be invalid at match time.  The exact problems are usually
+    /// reported in subsequent diagnostics of the same match type but with
+    /// \c Note set.
+    MatchNoneForInvalidPattern,
     /// Indicates a fuzzy match that serves as a suggestion for the next
     /// intended match for an expected pattern with too few or no good matches.
     MatchFuzzy,
   } MatchTy;
-  /// The search range if MatchTy is MatchNoneAndExcluded or
-  /// MatchNoneButExpected, or the match range otherwise.
+  /// The search range if MatchTy starts with MatchNone, or the match range
+  /// otherwise.
   unsigned InputStartLine;
   unsigned InputStartCol;
   unsigned InputEndLine;
   unsigned InputEndCol;
   /// A note to replace the one normally indicated by MatchTy, or the empty
   /// string if none.
   std::string Note;
   FileCheckDiag(const SourceMgr &SM, const Check::FileCheckType &CheckTy,

llvm/lib/FileCheck/FileCheck.cpp

 }
 
 void Pattern::AddBackrefToRegEx(unsigned BackrefNum) {
   assert(BackrefNum >= 1 && BackrefNum <= 9 && "Invalid backref number");
   std::string Backref = std::string("\\") + std::string(1, '0' + BackrefNum);
   RegExStr += Backref;
 }
 
-Expected<size_t> Pattern::match(StringRef Buffer, size_t &MatchLen,
+Pattern::MatchResult Pattern::match(StringRef Buffer,
                                     const SourceMgr &SM) const {
   // If this is the EOF pattern, match it immediately.
-  if (CheckTy == Check::CheckEOF) {
-    MatchLen = 0;
-    return Buffer.size();
-  }
+  if (CheckTy == Check::CheckEOF)
+    return MatchResult(Buffer.size(), 0, Error::success());
 
   // If this is a fixed string pattern, just match it now.
   if (!FixedStr.empty()) {
-    MatchLen = FixedStr.size();
     size_t Pos =
         IgnoreCase ? Buffer.find_lower(FixedStr) : Buffer.find(FixedStr);
     if (Pos == StringRef::npos)
       return make_error<NotFoundError>();
-    return Pos;
+    return MatchResult(Pos, /*MatchLen=*/FixedStr.size(), Error::success());
   }
 
   // Regex match.
 
   // If there are substitutions, we need to create a temporary string with the
   // actual value.
   StringRef RegExToMatch = RegExStr;
   std::string TmpStr;
   if (!Substitutions.empty()) {
     TmpStr = RegExStr;
     if (LineNumber)
       Context->LineVariable->setValue(ExpressionValue(*LineNumber));
 
     size_t InsertOffset = 0;
     // 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.
     for (const auto &Substitution : Substitutions) {
       // Substitute and check for failure (e.g. use of undefined variable).
       Expected<std::string> Value = Substitution->getResult();
       if (!Value) {
         // Convert to an ErrorDiagnostic to get location information. This is
-        // done here rather than PrintNoMatch since now we know which
+        // 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);
Pattern::MatchResult Pattern::match(StringRef Buffer,
 
   // If this defines any string variables, remember their values.
   for (const auto &VariableDef : VariableDefs) {
     assert(VariableDef.second < MatchInfo.size() && "Internal paren error");
     Context->GlobalVariableTable[VariableDef.first] =
         MatchInfo[VariableDef.second];
   }
 
+  // Like CHECK-NEXT, CHECK-EMPTY's match range is considered to start after
+  // the required preceding newline, which is consumed by the pattern in the
+  // case of CHECK-EMPTY but not CHECK-NEXT.
+  size_t MatchStartSkip = CheckTy == Check::CheckEmpty;
+  Match TheMatch;
+  TheMatch.Pos = FullMatch.data() - Buffer.data() + MatchStartSkip;
+  TheMatch.Len = FullMatch.size() - MatchStartSkip;
+
   // If this defines any numeric variables, remember their values.
   for (const auto &NumericVariableDef : NumericVariableDefs) {
     const NumericVariableMatch &NumericVariableMatch =
         NumericVariableDef.getValue();
     unsigned CaptureParenGroup = NumericVariableMatch.CaptureParenGroup;
     assert(CaptureParenGroup < MatchInfo.size() && "Internal paren error");
     NumericVariable *DefinedNumericVariable =
         NumericVariableMatch.DefinedNumericVariable;
 
     StringRef MatchedValue = MatchInfo[CaptureParenGroup];
     ExpressionFormat Format = DefinedNumericVariable->getImplicitFormat();
     Expected<ExpressionValue> Value =
         Format.valueFromStringRepr(MatchedValue, SM);
     if (!Value)
-      return Value.takeError();
+      return MatchResult(TheMatch, Value.takeError());
     DefinedNumericVariable->setValue(*Value, MatchedValue);
   }
 
-  // Like CHECK-NEXT, CHECK-EMPTY's match range is considered to start after
-  // the required preceding newline, which is consumed by the pattern in the
-  // case of CHECK-EMPTY but not CHECK-NEXT.
-  size_t MatchStartSkip = CheckTy == Check::CheckEmpty;
-  MatchLen = FullMatch.size() - MatchStartSkip;
-  return FullMatch.data() - Buffer.data() + MatchStartSkip;
+  return MatchResult(TheMatch, Error::success());
 }
 
 unsigned Pattern::computeMatchDistance(StringRef Buffer) const {
   // Just compute the number of matching characters. For regular expressions, we
   // just compare against the regex itself and hope for the best.
   //
   // FIXME: One easy improvement here is have the regex lib generate a single
   // example regular expression which matches, and use that as the example
for (const auto &Substitution : Substitutions) {
       Expected<std::string> MatchedValue = Substitution->getResult();
 
       // Substitution failed or is not known at match time, print the undefined
       // variables it uses.
       if (!MatchedValue) {
         bool UndefSeen = false;
         handleAllErrors(
             MatchedValue.takeError(), [](const NotFoundError &E) {},
-            // Handled in PrintNoMatch().
+            // 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;
               }
for (const auto &VariableDef : VariableDefs) {
     VarCapture VC;
     VC.Name = VariableDef.first;
     StringRef Value = Context->GlobalVariableTable[VC.Name];
     SMLoc Start = SMLoc::getFromPointer(Value.data());
     SMLoc End = SMLoc::getFromPointer(Value.data() + Value.size());
     VC.Range = SMRange(Start, End);
     VarCaptures.push_back(VC);
   }
   for (const auto &VariableDef : NumericVariableDefs) {
     VarCapture VC;
     VC.Name = VariableDef.getKey();
-    StringRef StrValue = VariableDef.getValue()
-                             .DefinedNumericVariable->getStringValue()
-                             .getValue();
-    SMLoc Start = SMLoc::getFromPointer(StrValue.data());
-    SMLoc End = SMLoc::getFromPointer(StrValue.data() + StrValue.size());
+    Optional<StringRef> StrValue =
+        VariableDef.getValue().DefinedNumericVariable->getStringValue();
+    if (!StrValue)
+      continue;
+    SMLoc Start = SMLoc::getFromPointer(StrValue->data());
+    SMLoc End = SMLoc::getFromPointer(StrValue->data() + StrValue->size());
     VC.Range = SMRange(Start, End);
     VarCaptures.push_back(VC);
   }
   // Sort variable captures by the order in which they matched the input.
   // Ranges shouldn't be overlapping, so we can just compare the start.
   llvm::sort(VarCaptures, [](const VarCapture &A, const VarCapture &B) {
     assert(A.Range.Start != B.Range.Start &&
            "unexpected overlapping variable captures");
CheckStrings->emplace_back(
         Pattern(Check::CheckEOF, PatternContext.get(), LineNumber + 1),
         *Req.CheckPrefixes.begin(), SMLoc::getFromPointer(Buffer.data()));
     std::swap(DagNotMatches, CheckStrings->back().DagNotStrings);
   }
 
   return false;
 }
 
-static void PrintMatch(bool ExpectedMatch, const SourceMgr &SM,
+static bool printMatch(bool ExpectedMatch, const SourceMgr &SM,
                        StringRef Prefix, SMLoc Loc, const Pattern &Pat,
-                       int MatchedCount, StringRef Buffer, size_t MatchPos,
-                       size_t MatchLen, const FileCheckRequest &Req,
+                       int MatchedCount, StringRef Buffer,
+                       Pattern::MatchResult MatchResult,
+                       const FileCheckRequest &Req,
                        std::vector<FileCheckDiag> *Diags) {
+  // Suppress some verbosity if there's no error.
+  bool HasError = !ExpectedMatch || MatchResult.TheError;
   bool PrintDiag = true;
-  if (ExpectedMatch) {
+  if (!HasError) {
     if (!Req.Verbose)
-      return;
+      return HasError;
     if (!Req.VerboseVerbose && Pat.getCheckTy() == Check::CheckEOF)
-      return;
+      return HasError;
     // Due to their verbosity, we don't print verbose diagnostics here if we're
-    // gathering them for a different rendering, but we always print other
-    // diagnostics.
+    // gathering them for Diags to be rendered elsewhere, but we always print
+    // other diagnostics.
     PrintDiag = !Diags;
   }
+
+  // Add "found" diagnostic, substitutions, and variable definitions to Diags.
   FileCheckDiag::MatchType MatchTy = ExpectedMatch
                                          ? FileCheckDiag::MatchFoundAndExpected
                                          : FileCheckDiag::MatchFoundButExcluded;
   SMRange MatchRange = ProcessMatchResult(MatchTy, SM, Loc, Pat.getCheckTy(),
-                                          Buffer, MatchPos, MatchLen, Diags);
+                                          Buffer, MatchResult.TheMatch->Pos,
+                                          MatchResult.TheMatch->Len, Diags);
   if (Diags) {
     Pat.printSubstitutions(SM, Buffer, MatchRange, MatchTy, Diags);
     Pat.printVariableDefs(SM, MatchTy, Diags);
   }
-  if (!PrintDiag)
-    return;
+  if (!PrintDiag) {
+    assert(!HasError && "expected to report more diagnostics for error");
+    return HasError;
+  }
 
+  // Print the match.
   std::string Message = formatv("{0}: {1} string found in input",
                                 Pat.getCheckTy().getDescription(Prefix),
                                 (ExpectedMatch ? "expected" : "excluded"))
                             .str();
   if (Pat.getCount() > 1)
     Message += formatv(" ({0} out of {1})", MatchedCount, Pat.getCount()).str();
-
   SM.PrintMessage(
       Loc, ExpectedMatch ? SourceMgr::DK_Remark : SourceMgr::DK_Error, Message);
   SM.PrintMessage(MatchRange.Start, SourceMgr::DK_Note, "found here",
                   {MatchRange});
+
+  // Print additional information, which can be useful even if there are errors.
   Pat.printSubstitutions(SM, Buffer, MatchRange, MatchTy, nullptr);
   Pat.printVariableDefs(SM, MatchTy, nullptr);
+
+  // Print errors and add them to Diags.  We report these errors after the match
+  // itself because we found them after the match.  If we had found them before
+  // the match, we'd be in printNoMatch.
+  handleAllErrors(std::move(MatchResult.TheError),
+                  [&](const ErrorDiagnostic &E) {
+                    E.log(errs());
+                    if (Diags) {
+                      Diags->emplace_back(SM, Pat.getCheckTy(), Loc,
+                                          FileCheckDiag::MatchFoundErrorNote,
+                                          E.getRange(), E.getMessage().str());
+                    }
+                  });
+  return HasError;
 }
 
-static void PrintMatch(bool ExpectedMatch, const SourceMgr &SM,
+static bool printMatch(bool ExpectedMatch, const SourceMgr &SM,
                        const FileCheckString &CheckStr, int MatchedCount,
-                       StringRef Buffer, size_t MatchPos, size_t MatchLen,
+                       StringRef Buffer, Pattern::MatchResult MatchResult,
                        FileCheckRequest &Req,
                        std::vector<FileCheckDiag> *Diags) {
-  PrintMatch(ExpectedMatch, SM, CheckStr.Prefix, CheckStr.Loc, CheckStr.Pat,
-             MatchedCount, Buffer, MatchPos, MatchLen, Req, Diags);
+  return printMatch(ExpectedMatch, SM, CheckStr.Prefix, CheckStr.Loc,
+                    CheckStr.Pat, MatchedCount, Buffer, std::move(MatchResult),
+                    Req, Diags);
 }
 
-static void PrintNoMatch(bool ExpectedMatch, const SourceMgr &SM,
+static bool printNoMatch(bool ExpectedMatch, const SourceMgr &SM,
                          StringRef Prefix, SMLoc Loc, const Pattern &Pat,
-                         int MatchedCount, StringRef Buffer,
-                         bool VerboseVerbose, std::vector<FileCheckDiag> *Diags,
-                         Error MatchErrors) {
-  assert(MatchErrors && "Called on successful match");
+                         int MatchedCount, StringRef Buffer, Error MatchError,
+                         bool VerboseVerbose,
+                         std::vector<FileCheckDiag> *Diags) {
+  // Print any pattern errors, and record them to be added to Diags later.
+  bool HasError = ExpectedMatch;
+  bool HasPatternError = false;
+  FileCheckDiag::MatchType MatchTy = ExpectedMatch
+                                         ? FileCheckDiag::MatchNoneButExpected
+                                         : FileCheckDiag::MatchNoneAndExcluded;
+  SmallVector<std::string, 4> ErrorMsgs;
+  handleAllErrors(
+      std::move(MatchError),
+      [&](const ErrorDiagnostic &E) {
+        HasError = HasPatternError = true;
+        MatchTy = FileCheckDiag::MatchNoneForInvalidPattern;
+        E.log(errs());
+        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) {});
+
+  // Suppress some verbosity if there's no error.
   bool PrintDiag = true;
-  if (!ExpectedMatch) {
-    if (!VerboseVerbose) {
-      consumeError(std::move(MatchErrors));
-      return;
-    }
+  if (!HasError) {
+    if (!VerboseVerbose)
+      return HasError;
     // Due to their verbosity, we don't print verbose diagnostics here if we're
-    // gathering them for a different rendering, but we always print other
-    // diagnostics.
+    // gathering them for Diags to be rendered elsewhere, but we always print
+    // other diagnostics.
     PrintDiag = !Diags;
   }
 
-  FileCheckDiag::MatchType MatchTy = ExpectedMatch
-                                         ? FileCheckDiag::MatchNoneButExpected
-                                         : FileCheckDiag::MatchNoneAndExcluded;
+  // Add "not found" diagnostic, substitutions, and pattern errors to Diags.
+  //
+  // We handle Diags a little differently than the errors we print directly:
+  // we add the "not found" diagnostic to Diags even if there are pattern
+  // errors.  The reason is that we need to attach pattern errors as notes
+  // somewhere in the input, and the input search range from the "not found"
+  // diagnostic is all we have to anchor them.
   SMRange SearchRange = ProcessMatchResult(MatchTy, SM, Loc, Pat.getCheckTy(),
                                            Buffer, 0, Buffer.size(), Diags);
-  if (Diags)
+  if (Diags) {
+    SMRange NoteRange = SMRange(SearchRange.Start, SearchRange.Start);
+    for (const std::string &ErrorMsg : ErrorMsgs)
+      Diags->emplace_back(SM, Pat.getCheckTy(), Loc, MatchTy, NoteRange,
+                          ErrorMsg);
     Pat.printSubstitutions(SM, Buffer, SearchRange, MatchTy, Diags);
+  }
   if (!PrintDiag) {
-    consumeError(std::move(MatchErrors));
-    return;
+    assert(!HasError && "expected to report more diagnostics for error");
+    return HasError;
   }
 
-  MatchErrors = handleErrors(std::move(MatchErrors),
-                             [](const ErrorDiagnostic &E) { E.log(errs()); });
-
-  // No problem matching the string per se.
-  if (!MatchErrors)
-    return;
-  consumeError(std::move(MatchErrors));
-
-  // Print "not found" diagnostic.
+  // Print "not found" diagnostic, except that's implied if we already printed a
+  // pattern error.
+  if (!HasPatternError) {
     std::string Message = formatv("{0}: {1} string not found in input",
                                   Pat.getCheckTy().getDescription(Prefix),
                                   (ExpectedMatch ? "expected" : "excluded"))
                               .str();
     if (Pat.getCount() > 1)
-    Message += formatv(" ({0} out of {1})", MatchedCount, Pat.getCount()).str();
-  SM.PrintMessage(
-      Loc, ExpectedMatch ? SourceMgr::DK_Error : SourceMgr::DK_Remark, Message);
-
-  // Print the "scanning from here" line.
-  SM.PrintMessage(SearchRange.Start, SourceMgr::DK_Note, "scanning from here");
+      Message +=
+          formatv(" ({0} out of {1})", MatchedCount, Pat.getCount()).str();
+    SM.PrintMessage(Loc,
+                    ExpectedMatch ? SourceMgr::DK_Error : SourceMgr::DK_Remark,
+                    Message);
+    SM.PrintMessage(SearchRange.Start, SourceMgr::DK_Note,
+                    "scanning from here");
+  }
 
-  // Allow the pattern to print additional information if desired.
+  // Print additional information, which can be useful even after a pattern
+  // error.
   Pat.printSubstitutions(SM, Buffer, SearchRange, MatchTy, nullptr);
-
   if (ExpectedMatch)
     Pat.printFuzzyMatch(SM, Buffer, Diags);
+  return HasError;
 }
 
-static void PrintNoMatch(bool ExpectedMatch, const SourceMgr &SM,
+static bool printNoMatch(bool ExpectedMatch, const SourceMgr &SM,
                          const FileCheckString &CheckStr, int MatchedCount,
-                         StringRef Buffer, bool VerboseVerbose,
-                         std::vector<FileCheckDiag> *Diags, Error MatchErrors) {
-  PrintNoMatch(ExpectedMatch, SM, CheckStr.Prefix, CheckStr.Loc, CheckStr.Pat,
-               MatchedCount, Buffer, VerboseVerbose, Diags,
-               std::move(MatchErrors));
+                         StringRef Buffer, Error MatchError,
+                         bool VerboseVerbose,
+                         std::vector<FileCheckDiag> *Diags) {
+  return printNoMatch(ExpectedMatch, SM, CheckStr.Prefix, CheckStr.Loc,
+                      CheckStr.Pat, MatchedCount, Buffer, std::move(MatchError),
+                      VerboseVerbose, Diags);
 }
 
 /// Counts the number of newlines in the specified range.
 static unsigned CountNumNewlinesBetween(StringRef Range,
                                         const char *&FirstNewLine) {
   unsigned NumNewLines = 0;
   while (1) {
     // Scan for newline.
size_t FileCheckString::Check(const SourceMgr &SM, StringRef Buffer,
   // Match itself from the last position after matching CHECK-DAG.
   size_t LastMatchEnd = LastPos;
   size_t FirstMatchPos = 0;
   // Go match the pattern Count times. Majority of patterns only match with
   // count 1 though.
   assert(Pat.getCount() != 0 && "pattern count can not be zero");
   for (int i = 1; i <= Pat.getCount(); i++) {
     StringRef MatchBuffer = Buffer.substr(LastMatchEnd);
-    size_t CurrentMatchLen;
     // get a match at current start point
-    Expected<size_t> MatchResult = Pat.match(MatchBuffer, CurrentMatchLen, SM);
+    Pattern::MatchResult MatchResult = Pat.match(MatchBuffer, SM);
 
     // report
-    if (!MatchResult) {
-      PrintNoMatch(true, SM, *this, i, MatchBuffer, Req.VerboseVerbose, Diags,
-                   MatchResult.takeError());
+    if (!MatchResult.TheMatch) {
+      printNoMatch(/*ExpectedMatch=*/true, SM, *this, i, MatchBuffer,
+                   std::move(MatchResult.TheError), Req.VerboseVerbose, Diags);
       return StringRef::npos;
     }
-    size_t MatchPos = *MatchResult;
-    PrintMatch(true, SM, *this, i, MatchBuffer, MatchPos, CurrentMatchLen, Req,
-               Diags);
+    if (printMatch(/*ExpectedMatch=*/true, SM, *this, i, MatchBuffer,
+                   std::move(MatchResult), Req, Diags))
+      return StringRef::npos;
+
+    size_t MatchPos = MatchResult.TheMatch->Pos;
     if (i == 1)
       FirstMatchPos = LastPos + MatchPos;
 
     // move start point after the match
-    LastMatchEnd += MatchPos + CurrentMatchLen;
+    LastMatchEnd += MatchPos + MatchResult.TheMatch->Len;
   }
   // Full match len counts from first match pos.
   MatchLen = LastMatchEnd - FirstMatchPos;
 
   // Similar to the above, in "label-scan mode" we can't yet handle CHECK-NEXT
   // or CHECK-NOT
   if (!IsLabelScanMode) {
     size_t MatchPos = FirstMatchPos - LastPos;

 bool FileCheckString::CheckNot(const SourceMgr &SM, StringRef Buffer,
                                const std::vector<const Pattern *> &NotStrings,
                                const FileCheckRequest &Req,
                                std::vector<FileCheckDiag> *Diags) const {
   bool DirectiveFail = false;
   for (const Pattern *Pat : NotStrings) {
     assert((Pat->getCheckTy() == Check::CheckNot) && "Expect CHECK-NOT!");
 
-    size_t MatchLen = 0;
-    Expected<size_t> MatchResult = Pat->match(Buffer, MatchLen, SM);
+    Pattern::MatchResult MatchResult = Pat->match(Buffer, SM);
 
-    if (!MatchResult) {
-      PrintNoMatch(false, SM, Prefix, Pat->getLoc(), *Pat, 1, Buffer,
-                   Req.VerboseVerbose, Diags, MatchResult.takeError());
+    if (!MatchResult.TheMatch) {
+      if (printNoMatch(/*ExpectedMatch=*/false, SM, Prefix, Pat->getLoc(), *Pat,
+                       1, Buffer, std::move(MatchResult.TheError),
+                       Req.VerboseVerbose, Diags))
+        DirectiveFail = true;
       continue;
     }
-    size_t Pos = *MatchResult;
 
-    PrintMatch(false, SM, Prefix, Pat->getLoc(), *Pat, 1, Buffer, Pos, MatchLen,
-               Req, Diags);
+    printMatch(/*ExpectedMatch=*/false, SM, Prefix, Pat->getLoc(), *Pat, 1,
+               Buffer, std::move(MatchResult), Req, Diags);
     DirectiveFail = true;
   }
 
   return DirectiveFail;
 }
 
 size_t FileCheckString::CheckDag(const SourceMgr &SM, StringRef Buffer,
                                  std::vector<const Pattern *> &NotStrings,
for (auto PatItr = DagNotStrings.begin(), PatEnd = DagNotStrings.end();
 
     // CHECK-DAG always matches from the start.
     size_t MatchLen = 0, MatchPos = StartPos;
 
     // Search for a match that doesn't overlap a previous match in this
     // CHECK-DAG group.
     for (auto MI = MatchRanges.begin(), ME = MatchRanges.end(); true; ++MI) {
       StringRef MatchBuffer = Buffer.substr(MatchPos);
-      Expected<size_t> MatchResult = Pat.match(MatchBuffer, MatchLen, SM);
+      Pattern::MatchResult MatchResult = Pat.match(MatchBuffer, SM);
       // With a group of CHECK-DAGs, a single mismatching means the match on
       // that group of CHECK-DAGs fails immediately.
-      if (!MatchResult) {
-        PrintNoMatch(true, SM, Prefix, Pat.getLoc(), Pat, 1, MatchBuffer,
-                     Req.VerboseVerbose, Diags, MatchResult.takeError());
+      if (!MatchResult.TheMatch) {
+        printNoMatch(/*ExpectedMatch=*/true, SM, Prefix, Pat.getLoc(), Pat, 1,
+                     MatchBuffer, std::move(MatchResult.TheError),
+                     Req.VerboseVerbose, Diags);
         return StringRef::npos;
       }
-      size_t MatchPosBuf = *MatchResult;
+      size_t MatchPosBuf = MatchResult.TheMatch->Pos;
+      MatchLen = MatchResult.TheMatch->Len;
       // Re-calc it as the offset relative to the start of the original string.
       MatchPos += MatchPosBuf;
-      if (Req.VerboseVerbose)
-        PrintMatch(true, SM, Prefix, Pat.getLoc(), Pat, 1, Buffer, MatchPos,
-                   MatchLen, Req, Diags);
+      if (MatchResult.TheError || Req.VerboseVerbose) {
+        if (printMatch(/*ExpectedMatch=*/true, SM, Prefix, Pat.getLoc(), Pat, 1,
+                       Buffer,
+                       Pattern::MatchResult(MatchPos, MatchLen,
+                                            std::move(MatchResult.TheError)),
+                       Req, Diags))
+          return StringRef::npos;
+      }
       MatchRange M{MatchPos, MatchPos + MatchLen};
       if (Req.AllowDeprecatedDagOverlap) {
         // We don't need to track all matches in this mode, so we just maintain
         // one match range that encompasses the current CHECK-DAG group's
         // matches.
         if (MatchRanges.empty())
           MatchRanges.insert(MatchRanges.end(), M);
         else {
for (auto MI = MatchRanges.begin(), ME = MatchRanges.end(); true; ++MI) {
           SMLoc CheckLoc = Diags->rbegin()->CheckLoc;
           for (auto I = Diags->rbegin(), E = Diags->rend();
                I != E && I->CheckLoc == CheckLoc; ++I)
             I->MatchTy = FileCheckDiag::MatchFoundButDiscarded;
         }
       }
       MatchPos = MI->End;
     }
     if (!Req.VerboseVerbose)
-      PrintMatch(true, SM, Prefix, Pat.getLoc(), Pat, 1, Buffer, MatchPos,
-                 MatchLen, Req, Diags);
+      printMatch(/*ExpectedMatch=*/true, SM, Prefix, Pat.getLoc(), Pat, 1,
+                 Buffer,
+                 Pattern::MatchResult(MatchPos, MatchLen, Error::success()),
+                 Req, Diags);
 
     // Handle the end of a CHECK-DAG group.
     if (std::next(PatItr) == PatEnd ||
         std::next(PatItr)->getCheckTy() == Check::CheckNot) {
       if (!NotStrings.empty()) {
         // If there are CHECK-NOTs between two CHECK-DAGs or from CHECK to
         // CHECK-DAG, verify that there are no 'not' strings occurred in that
         // region.

llvm/lib/FileCheck/FileCheckImpl.h

Substitution *makeNumericSubstitution(StringRef ExpressionStr,
                                         size_t InsertIdx);
 };
 
 /// Class to represent an error holding a diagnostic with location information
 /// used when printing it.
 class ErrorDiagnostic : public ErrorInfo<ErrorDiagnostic> {
 private:
   SMDiagnostic Diagnostic;
+  SMRange Range;
 
 public:
   static char ID;
 
-  ErrorDiagnostic(SMDiagnostic &&Diag) : Diagnostic(Diag) {}
+  ErrorDiagnostic(SMDiagnostic &&Diag, SMRange Range)
+      : Diagnostic(Diag), Range(Range) {}
 
   std::error_code convertToErrorCode() const override {
     return inconvertibleErrorCode();
   }
 
   /// Print diagnostic associated with this error when printing the error.
   void log(raw_ostream &OS) const override { Diagnostic.print(nullptr, OS); }
 
-  static Error get(const SourceMgr &SM, SMLoc Loc, const Twine &ErrMsg) {
+  StringRef getMessage() const { return Diagnostic.getMessage(); }
+  SMRange getRange() const { return Range; }
+
+  static Error get(const SourceMgr &SM, SMLoc Loc, const Twine &ErrMsg,
+                   SMRange Range = None) {
     return make_error<ErrorDiagnostic>(
-        SM.GetMessage(Loc, SourceMgr::DK_Error, ErrMsg));
+        SM.GetMessage(Loc, SourceMgr::DK_Error, ErrMsg), Range);
   }
 
   static Error get(const SourceMgr &SM, StringRef Buffer, const Twine &ErrMsg) {
-    return get(SM, SMLoc::getFromPointer(Buffer.data()), ErrMsg);
+    SMLoc Start = SMLoc::getFromPointer(Buffer.data());
+    SMLoc End = SMLoc::getFromPointer(Buffer.data() + Buffer.size());
+    return get(SM, Start, ErrMsg, SMRange(Start, End));
   }
 };
 
 class NotFoundError : public ErrorInfo<NotFoundError> {
 public:
   static char ID;
 
   std::error_code convertToErrorCode() const override {
public:
   /// accordingly.
   ///
   /// \p Prefix provides which prefix is being matched, \p Req describes the
   /// global options that influence the parsing such as whitespace
   /// canonicalization, \p SM provides the SourceMgr used for error reports.
   /// \returns true in case of an error, false otherwise.
   bool parsePattern(StringRef PatternStr, StringRef Prefix, SourceMgr &SM,
                     const FileCheckRequest &Req);
-  /// Matches the pattern string against the input buffer \p Buffer
+  struct Match {
+    size_t Pos;
+    size_t Len;
+  };
+  struct MatchResult {
+    Optional<Match> TheMatch;
+    Error TheError;
+    MatchResult(size_t MatchPos, size_t MatchLen, Error E)
+        : TheMatch(Match{MatchPos, MatchLen}), TheError(std::move(E)) {}
+    MatchResult(Match M, Error E) : TheMatch(M), TheError(std::move(E)) {}
+    MatchResult(Error E) : TheError(std::move(E)) {}
+  };
+  /// Matches the pattern string against the input buffer \p Buffer.
   ///
-  /// \returns the position that is matched or an error indicating why matching
-  /// failed. If there is a match, updates \p MatchLen with the size of the
-  /// matched string.
+  /// \returns either (1) an error resulting in no match or (2) a match possibly
+  /// with an error encountered while processing the match.
   ///
   /// The GlobalVariableTable StringMap in the FileCheckPatternContext class
   /// instance provides the current values of FileCheck string variables and is
   /// updated if this match defines new values. Likewise, the
   /// GlobalNumericVariableTable StringMap in the same class provides the
   /// current values of FileCheck numeric variables and is updated if this
   /// match defines new numeric values.
-  Expected<size_t> match(StringRef Buffer, size_t &MatchLen,
-                         const SourceMgr &SM) const;
+  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.
   void printSubstitutions(const SourceMgr &SM, StringRef Buffer,
                           SMRange MatchRange, FileCheckDiag::MatchType MatchTy,
                           std::vector<FileCheckDiag> *Diags) const;
   void printFuzzyMatch(const SourceMgr &SM, StringRef Buffer,
                        std::vector<FileCheckDiag> *Diags) const;
 

llvm/test/FileCheck/match-time-error-propagation/invalid-excluded-pattern.txt

+; Check handling of match-time diagnostics for invalid patterns (e.g.,
+; substitution overflow) in the case of excluded patterns (e.g., CHECK-NOT).
+;
+; At one time, FileCheck's exit status was zero for this case.  Moreover, it
+; printed the error diagnostic only if -vv was specified and input dumps were
+; disabled.  Test every combination as the logic is hard to get right.
+;
+; FIXME: We shouldn't have: (1) the blank note at the end of the trace, and
+; (2) the redundant error in the middle of the dump.  These will be fixed in a
+; subsequent patch.
+
+RUN: echo > %t.chk \
+RUN:      'CHECK-NOT: [[#0x8000000000000000+0x8000000000000000]] [[UNDEFVAR]]'
+RUN: echo > %t.in '10000000000000000'
+
+     ERR-NOT:{{.}}
+      ERR-VV:{{.*}}: remark: implicit EOF: expected string found in input
+ ERR-VV-NEXT:CHECK-NOT: {{.*}}
+ ERR-VV-NEXT:{{ *}}^
+ ERR-VV-NEXT:{{.*}}: note: found here
+ERR-VV-EMPTY:
+ ERR-VV-NEXT:^
+     ERR-NOT:{{.}}
+         ERR:{{.*}}: error: unable to substitute variable or numeric expression: overflow error
+    ERR-NEXT:CHECK-NOT: {{.*}}
+    ERR-NEXT:{{ *}}^
+    ERR-NEXT:{{.*}}: note: 
+    ERR-NEXT:10000000000000000
+    ERR-NEXT:^
+    ERR-NEXT:<stdin>:1:1: note: uses undefined variable(s): "UNDEFVAR"
+    ERR-NEXT:10000000000000000
+    ERR-NEXT:^
+     ERR-NOT:{{error|note|remark}}:
+
+        DUMP:<<<<<<
+   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     X                  error: match failed for invalid pattern
+   DUMP-NEXT:not:1'3                        uses undefined variable(s): "UNDEFVAR"
+DUMP-VV-NEXT:         2: 
+DUMP-VV-NEXT:eof:1       ^
+   DUMP-NEXT:>>>>>>
+
+;--------------------------------------------------
+; Check -dump-input=never cases.
+;--------------------------------------------------
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=never %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=never -v %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=never -vv %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR,ERR-VV
+
+;--------------------------------------------------
+; Check -dump-input=fail cases.
+;--------------------------------------------------
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=fail %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR,DUMP
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=fail -v %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR,DUMP
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=fail -vv %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR,DUMP,DUMP-VV

llvm/test/FileCheck/match-time-error-propagation/invalid-expected-pattern.txt

+; Check handling of match-time diagnostics for invalid patterns (e.g.,
+; substitution overflow) in the case of expected patterns (e.g., CHECK).
+;
+; FIXME: We shouldn't have: (1) the blank note at the end of the trace, and
+; (2) the redundant error in the middle of the dump.  These will be fixed in a
+; subsequent patch.
+
+RUN: echo > %t.chk \
+RUN:      'CHECK: [[#0x8000000000000000+0x8000000000000000]] [[UNDEFVAR]]'
+RUN: echo > %t.in '10000000000000000'
+
+  ERR-NOT:{{.}}
+      ERR:{{.*}}: error: unable to substitute variable or numeric expression: overflow error
+ ERR-NEXT:CHECK: {{.*}}
+ ERR-NEXT:{{ *}}^
+ ERR-NEXT:{{.*}}: note: 
+ ERR-NEXT:10000000000000000
+ ERR-NEXT:^
+ ERR-NEXT:<stdin>:1:1: note: uses undefined variable(s): "UNDEFVAR"
+ ERR-NEXT:10000000000000000
+ ERR-NEXT:^
+  ERR-NOT:{{error|note|remark}}:
+
+     DUMP:<<<<<<
+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     X                  error: match failed for invalid pattern
+DUMP-NEXT:check:1'3                        uses undefined variable(s): "UNDEFVAR"
+DUMP-NEXT:>>>>>>
+
+;--------------------------------------------------
+; Check -dump-input=never cases.
+;--------------------------------------------------
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=never %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=never -v %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=never -vv %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR
+
+;--------------------------------------------------
+; Check -dump-input=fail cases.
+;--------------------------------------------------
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=fail %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR,DUMP
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=fail -v %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR,DUMP
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=fail -vv %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR,DUMP

llvm/test/FileCheck/match-time-error-propagation/matched-excluded-pattern.txt

+; Check handling of diagnostics for problematic matches (e.g., variable capture
+; overflow) in the case of excluded patterns (e.g., CHECK-NOT).
+;
+; At one time, FileCheck's exit status for the following example was zero even
+; though the excluded pattern does match (it's just capturing that fails).
+; Moreover, it printed the error diagnostic only if -vv was specified and input
+; dumps were disabled.  Test every combination as the logic is hard to get
+; right. 
+;
+; TODO: Capturing from an excluded pattern probably shouldn't be permitted
+; because it seems useless: it's captured only if the pattern matches, but then
+; FileCheck fails.  The helpfulness of reporting overflow from that capture is
+; perhaps questionable then, but it doesn't seem harmful either.  Anyway, the
+; goal of this test is simply to exercise the error propagation mechanism for a
+; matched excluded pattern.  In the future, if we have a more interesting error
+; to exercise in that case, we should instead use it in this test, and then we
+; might want to think more about where that error should be presented in the
+; list of diagnostics.
+
+RUN: echo > %t.chk 'CHECK-NOT: [[#122+1]] [[STR:abc]] [[#NUM:]]'
+RUN: echo > %t.in '123 abc 1000000000000000000000000000000000000000000000000000'
+
+     ERR-NOT:{{.}}
+      ERR-VV:{{.*}}: remark: implicit EOF: expected string found in input
+ ERR-VV-NEXT:CHECK-NOT: {{.*}}
+ ERR-VV-NEXT:{{ *}}^
+ ERR-VV-NEXT:{{.*}}: note: found here
+ERR-VV-EMPTY:
+ ERR-VV-NEXT:^
+     ERR-NOT:{{.}}
+         ERR:{{.*}}: error: CHECK-NOT: excluded string found in input
+    ERR-NEXT:CHECK-NOT: {{.*}}
+    ERR-NEXT:{{ *}}^
+    ERR-NEXT:<stdin>:1:1: note: found here
+    ERR-NEXT:123 abc 10{{0*}}
+    ERR-NEXT:^~~~~~~~~{{~*}}
+    ERR-NEXT:<stdin>:1:1: note: with "122+1" equal to "123"
+    ERR-NEXT:123 abc 10{{0*}}
+    ERR-NEXT:^
+    ERR-NEXT:<stdin>:1:5: note: captured var "STR"
+    ERR-NEXT:123 abc 10{{0*}}
+    ERR-NEXT:    ^~~
+    ERR-NEXT:<stdin>:1:9: error: unable to represent numeric value
+    ERR-NEXT:123 abc 10{{0*}}
+    ERR-NEXT:        ^
+     ERR-NOT:{{error|note|remark}}:
+
+        DUMP:<<<<<<
+   DUMP-NEXT:         1: 123 abc 10{{0*}} 
+   DUMP-NEXT:not:1'0     !~~~~~~~~~{{~*}}  error: no match expected
+   DUMP-NEXT:not:1'1                       with "122+1" equal to "123"
+   DUMP-NEXT:not:1'2         !~~           captured var "STR"
+   DUMP-NEXT:not:1'3             !~{{~*}}  error: unable to represent numeric value
+DUMP-VV-NEXT:         2: 
+DUMP-VV-NEXT:eof:1       ^
+   DUMP-NEXT:>>>>>>
+
+;--------------------------------------------------
+; Check -dump-input=never cases.
+;--------------------------------------------------
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=never %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=never -v %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=never -vv %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR,ERR-VV
+
+;--------------------------------------------------
+; Check -dump-input=fail cases.
+;--------------------------------------------------
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=fail %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR,DUMP
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=fail -v %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR,DUMP
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=fail -vv %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR,DUMP,DUMP-VV

llvm/test/FileCheck/match-time-error-propagation/matched-expected-pattern.txt

+; Check handling of diagnostics for problematic matches (e.g., variable capture
+; overflow) in the case of expected patterns (e.g., CHECK).
+
+RUN: echo > %t.chk 'CHECK: [[#122+1]] [[STR:abc]] [[#NUM:]]'
+RUN: echo > %t.in '123 abc 1000000000000000000000000000000000000000000000000000'
+
+  ERR-NOT:{{.}}
+      ERR:{{.*}}: remark: CHECK: expected string found in input
+ ERR-NEXT:CHECK: {{.*}}
+ ERR-NEXT:{{ *}}^
+ ERR-NEXT:<stdin>:1:1: note: found here
+ ERR-NEXT:123 abc 10{{0*}}
+ ERR-NEXT:^~~~~~~~~{{~*}}
+ ERR-NEXT:<stdin>:1:1: note: with "122+1" equal to "123"
+ ERR-NEXT:123 abc 10{{0*}}
+ ERR-NEXT:^
+ ERR-NEXT:<stdin>:1:5: note: captured var "STR"
+ ERR-NEXT:123 abc 10{{0*}}
+ ERR-NEXT:    ^~~
+ ERR-NEXT:<stdin>:1:9: error: unable to represent numeric value
+ ERR-NEXT:123 abc 10{{0*}}
+ ERR-NEXT:        ^
+  ERR-NOT:{{error|note|remark}}:
+
+     DUMP:<<<<<<
+DUMP-NEXT:           1: 123 abc 10{{0*}} 
+DUMP-NEXT:check:1'0     ^~~~~~~~~~{{~*}}
+DUMP-NEXT:check:1'1                       with "122+1" equal to "123"
+DUMP-NEXT:check:1'2         ^~~           captured var "STR"
+DUMP-NEXT:check:1'3             !~{{~*}}  error: unable to represent numeric value
+DUMP-NEXT:>>>>>>
+
+;--------------------------------------------------
+; Check -dump-input=never cases.
+;--------------------------------------------------
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=never %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=never -v %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=never -vv %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR
+
+;--------------------------------------------------
+; Check -dump-input=fail cases.
+;--------------------------------------------------
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=fail %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR,DUMP
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=fail -v %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR,DUMP
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -dump-input=fail -vv %t.chk < %t.in 2>&1 \
+RUN: | FileCheck %s -match-full-lines -check-prefixes=ERR,DUMP

llvm/unittests/FileCheck/FileCheckTest.cpp

public:
 
   bool parsePattern(StringRef Pattern) {
     StringRef PatBufferRef = bufferize(SM, Pattern);
     return P.parsePattern(PatBufferRef, "CHECK", SM, Req);
   }
 
   Expected<size_t> match(StringRef Buffer) {
     StringRef BufferRef = bufferize(SM, Buffer);
-    size_t MatchLen;
-    return P.match(BufferRef, MatchLen, SM);
+    Pattern::MatchResult Res = P.match(BufferRef, SM);
+    if (Res.TheError)
+      return std::move(Res.TheError);
+    return Res.TheMatch->Pos;
   }
 
   void printVariableDefs(FileCheckDiag::MatchType MatchTy,
                          std::vector<FileCheckDiag> &Diags) {
     P.printVariableDefs(SM, MatchTy, &Diags);
   }
 };
 
TEST_F(FileCheckTest, FileCheckContext) {
 
   // Create @LINE pseudo numeric variable and check it is present by matching
   // it.
   size_t LineNumber = 1;
   Pattern P(Check::CheckPlain, &Cxt, LineNumber);
   FileCheckRequest Req;
   Cxt.createLineVariable();
   ASSERT_FALSE(P.parsePattern("[[@LINE]]", "CHECK", SM, Req));
-  size_t MatchLen;
-  ASSERT_THAT_EXPECTED(P.match("1", MatchLen, SM), Succeeded());
+  Pattern::MatchResult Res = P.match("1", SM);
+  ASSERT_THAT_ERROR(std::move(Res.TheError), Succeeded());
 
 #ifndef NDEBUG
   // Recreating @LINE pseudo numeric variable fails.
   EXPECT_DEATH(Cxt.createLineVariable(),
                "@LINE pseudo numeric variable already created");
 #endif
 
   // Check defined variables are present and undefined ones are absent.

llvm/utils/FileCheck/FileCheck.cpp

case FileCheckDiag::MatchFoundButExcluded:
     return MarkerStyle('!', raw_ostream::RED, "error: no match expected",
                        /*FiltersAsError=*/true);
   case FileCheckDiag::MatchFoundButWrongLine:
     return MarkerStyle('!', raw_ostream::RED, "error: match on wrong line",
                        /*FiltersAsError=*/true);
   case FileCheckDiag::MatchFoundButDiscarded:
     return MarkerStyle('!', raw_ostream::CYAN,
                        "discard: overlaps earlier match");
+  case FileCheckDiag::MatchFoundErrorNote:
+    // Note should always be overridden within the FileCheckDiag.
+    return MarkerStyle('!', raw_ostream::RED,
+                       "error: unknown error after match",
+                       /*FiltersAsError=*/true);
   case FileCheckDiag::MatchNoneAndExcluded:
     return MarkerStyle('X', raw_ostream::GREEN);
   case FileCheckDiag::MatchNoneButExpected:
     return MarkerStyle('X', raw_ostream::RED, "error: no match found",
                        /*FiltersAsError=*/true);
+  case FileCheckDiag::MatchNoneForInvalidPattern:
+    return MarkerStyle('X', raw_ostream::RED,
+                       "error: match failed for invalid pattern",
+                       /*FiltersAsError=*/true);
   case FileCheckDiag::MatchFuzzy:
     return MarkerStyle('?', raw_ostream::MAGENTA, "possible intended match",
                        /*FiltersAsError=*/true);
   }
   llvm_unreachable_internal("unexpected match type");
 }
 
 static void DumpInputAnnotationHelp(raw_ostream &OS) {
if (!DiagItr->Note.empty()) {
       // seem to indicate where "VAR" matches, but the location we actually have
       // for the marker simply points to the start of the match/search range for
       // the full pattern of which the substitution is potentially just one
       // component.
       if (DiagItr->InputStartLine == DiagItr->InputEndLine &&
           DiagItr->InputStartCol == DiagItr->InputEndCol)
         A.Marker.Lead = ' ';
     }
+    if (DiagItr->MatchTy == FileCheckDiag::MatchFoundErrorNote) {
+      assert(!DiagItr->Note.empty() &&
+             "expected custom note for MatchFoundErrorNote");
+      A.Marker.Note = "error: " + A.Marker.Note;
+    }
     A.FoundAndExpectedMatch =
         DiagItr->MatchTy == FileCheckDiag::MatchFoundAndExpected;
 
     // Compute the mark location, and break annotation into multiple
     // annotations if it spans multiple lines.
     A.IsFirstLine = true;
     A.InputLine = DiagItr->InputStartLine;
     A.InputStartCol = DiagItr->InputStartCol;