Fix PR46880: Fail CHECK-NOT with undefined variable

[thopre]

Summary

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.

[thopre]

FYI: I've only looked at a couple of the testsuite failures and they are all genuine use of undefined variable. There's 59 of them in total.

[grimar]

I am not well familar with the FileCheck logic probably to verify the code,
but following tests are failing for me with this diff applied:
(e.g.: Transforms/LoopVectorize/pr34681.ll fails with "error: uses undefined variable(s): "TEST"")

LLVM :: CodeGen/AArch64/aarch64-tbz.ll
LLVM :: CodeGen/AArch64/machine-outliner-retaddr-sign-thunk.ll
LLVM :: CodeGen/AArch64/speculation-hardening.ll
LLVM :: CodeGen/AMDGPU/amdgpu.private-memory.ll
LLVM :: CodeGen/AMDGPU/private-memory-r600.ll
LLVM :: CodeGen/Hexagon/hwloop3.ll
LLVM :: CodeGen/Mips/sr1.ll
LLVM :: CodeGen/PowerPC/ctrloops-softfloat.ll
LLVM :: CodeGen/PowerPC/ppc-disable-non-volatile-cr.ll
LLVM :: CodeGen/PowerPC/ppc-shrink-wrapping.ll
LLVM :: CodeGen/PowerPC/ppc64-i128-abi.ll
LLVM :: Transforms/CodeGenPrepare/ARM/sink-add-mul-shufflevector.ll
LLVM :: Transforms/Coroutines/coro-split-sink-lifetime-02.ll
LLVM :: Transforms/GVN/PRE/load-pre-nonlocal.ll
LLVM :: Transforms/GVN/big-endian.ll
LLVM :: Transforms/HardwareLoops/ARM/structure.ll
LLVM :: Transforms/InferFunctionAttrs/annotate.ll
LLVM :: Transforms/LoopVectorize/X86/x86-pr39099.ll
LLVM :: Transforms/LoopVectorize/pr34681.ll
LLVM :: Transforms/NewGVN/big-endian.ll

looks they all have undefined variables. So, seems this patch works well!
I've leaved 2 nits inline too.

llvm/lib/Support/FileCheck.cpp

2023

Perhaps a bit cleaner would be to return false, since it it known that UndefVar is always false here.

llvm/test/FileCheck/var-scope.txt

10

So, there are no ERRUNDEF check line anymore and seems you can remove it from here (and below) and switch to --check-prefix (just here)?

[grimar]

In D86222#2227835, @thopre wrote:

FYI: I've only looked at a couple of the testsuite failures and they are all genuine use of undefined variable. There's 59 of them in total.

Oh, OK, I did not see this comment when wrote mine one.

[thopre]

• Address review comments
• rename PrintNoMatch to printNoMatch to satisfy clang-tidy

[thopre]

There seems to be a number of failures like due to pattern like (taken from clang/test/CodeGen/debug-info-extern-call.c):

DECLS-FOR-EXTERN-NOT: !DICompileUnit({{.*}}retainedTypes: ![[RETTYPES:[0-9]+]]
DECLS-FOR-EXTERN-NOT: ![[RETTYPES]] = !{

I think FileCheck should forbid variable definition in a CHECK-NOT pattern. Granted with this patch we'll catch all the uses of variables defined in CHECK-NOT but we won't catch if they are not used.

[jhenderson]

In D86222#2228193, @thopre wrote:

There seems to be a number of failures like due to pattern like (taken from clang/test/CodeGen/debug-info-extern-call.c):

DECLS-FOR-EXTERN-NOT: !DICompileUnit({{.*}}retainedTypes: ![[RETTYPES:[0-9]+]]
DECLS-FOR-EXTERN-NOT: ![[RETTYPES]] = !{

I think FileCheck should forbid variable definition in a CHECK-NOT pattern. Granted with this patch we'll catch all the uses of variables defined in CHECK-NOT but we won't catch if they are not used.

I have no idea what that is even trying to do. A definition within a CHECK-NOT doesn't make sense - either the pattern will match, enabling the capture of the variable, and FileCheck to fail (because of a positive CHECK-NOT match), or it will not match, meaning the variable can't be used. In other words, I think forbidding variable definitions in CHECK-NOT makes sense. It's probably worth bringing this up on llvm-dev and cfe-dev though, to give people a head's up/ask for opinions. There may be a way this actually is useful, but if so, I don't know it.

This commit makes match error a failure for CHECK-NOT and changes PrintNoMatch to print all CHECK-NOT failure in a CHECK-NOT block rather than just the first one.

Are these two inherently linked, or could you split them into two separate changes?

llvm/lib/Support/FileCheck.cpp

1997

I'm not a fan on using booleans to indicate success or failure of a function, for two reasons:

1. It's easy to forget to check the result, meaning errors will be missed.
2. It's not obvious without reading whether a true indicates success or failure.

Can we use an Error instead?

2002

I'm not a fan of how this variable is being used throughout this function - it feels like it would be easy to forget to check it when doing something or check it incorrectly, making this function hard to maintain.

Could we bail out early if an undefined variable is detected?

llvm/test/FileCheck/numeric-expression.txt

249

Is this change related?

[thopre]

In D86222#2228316, @jhenderson wrote:

In D86222#2228193, @thopre wrote:

There seems to be a number of failures like due to pattern like (taken from clang/test/CodeGen/debug-info-extern-call.c):

DECLS-FOR-EXTERN-NOT: !DICompileUnit({{.*}}retainedTypes: ![[RETTYPES:[0-9]+]]
DECLS-FOR-EXTERN-NOT: ![[RETTYPES]] = !{

I think FileCheck should forbid variable definition in a CHECK-NOT pattern. Granted with this patch we'll catch all the uses of variables defined in CHECK-NOT but we won't catch if they are not used.

I have no idea what that is even trying to do. A definition within a CHECK-NOT doesn't make sense - either the pattern will match, enabling the capture of the variable, and FileCheck to fail (because of a positive CHECK-NOT match), or it will not match, meaning the variable can't be used. In other words, I think forbidding variable definitions in CHECK-NOT makes sense. It's probably worth bringing this up on llvm-dev and cfe-dev though, to give people a head's up/ask for opinions. There may be a way this actually is useful, but if so, I don't know it.

I have no idea either. I've found another case of 2 different invocation of FileCheck with different prefix where one invocation sets a variable and the other use it. I understand the intent having seen the output but I have so far no idea on how to fix it.

This commit makes match error a failure for CHECK-NOT and changes PrintNoMatch to print all CHECK-NOT failure in a CHECK-NOT block rather than just the first one.

Are these two inherently linked, or could you split them into two separate changes?

I think they can be separated if that change land first.

[thopre]

• Address comments
• Restructure

[jhenderson]

llvm/lib/Support/FileCheck.cpp

2030–2031

Why is there commented out code here?

2302–2306

Aside from the DirectiveFail = true line here, this and the other two instances of this pattern look identical to me. I guess that's partly my fault for the suggestion earlier of not returning a boolean to indicate an error. Perhaps part of the problem previously was the suggestion that the printNoMatch function even returned anything. I certainly wouldn't expect it.

How about we wrap this common code in a helper function, called handleMatchFailure or something like that, and that can return a boolean saying whether the failure was due to an error or regular failure? We could invent an enum that indicates the same thing but with better words, to avoid the boolean, but that might be going further than necessary. An alternative would be to pass in a lambda OnMatchError or something, which does nothing in the two other cases, but here sets the DirectiveFail variable.

[thopre]

Address review comments

[thopre]

llvm/lib/Support/FileCheck.cpp

2030–2031

Oops.

[jhenderson]

Code mostly looks fine now, but there are still plenty of test failures being reported by the pre-merge bot. What's the situation with those currently?

llvm/lib/Support/FileCheck.cpp

2064

This might be a dumb question, but I'm not following why we're skipping '\t' and ' ' here. It certainly doesn't line up with the comment above.

2081–2096

Can't these two be folded into a single handleAllErrors call? IIRC, handleAllErrors takes any number of handler lambdas, and iterates through them until one matches, or it runs out. Thus the above code should look like the something like the suggested edit.

[thopre]

In D86222#2251434, @jhenderson wrote:

Code mostly looks fine now, but there are still plenty of test failures being reported by the pre-merge bot. What's the situation with those currently?

I've started trying to fix them but sadly most of them are non trivial which is even more difficult when one is not familiar with what the test is trying to check. It seems a fair amount are using several CHECK-NOT as a way to say "I don't want THIS followed by THAT to match the input", where THAT uses a variable defined in THIS. It basically expects a back reference. This can be fixed by merging the two CHECK-NOT when they don't use different prefix but it seems to be yet another example where a grouping directive would be useful, e.g.:

CHECK-REGION-START: REGION-A
CHECK: THIS
CHECK: THAT
CHECK-REGION-END: REGION-A
CHECK-NOT: REGION-A

This is just an example, better syntax would need to be use. The reason I suggest a region rather than a new special-purpose directive is I wanted to have something like REGION-DAG where directives in a region are CHECK: that need to appear linearly but directives in different regions can be interleaved. Anyway, my plan is to try to merge those CHECK-NOT directives and then we can discuss how to introduce new syntax to make this more readable.

llvm/lib/Support/FileCheck.cpp

2064

I'm not sure, this is copied over from PrintNoMatch. I thought it is used so that the "scanning from here" shows the first non blank line after last match but I haven't looked in details.

2081–2096

It can be merged yes. It's a relic from when I would group all undefined variables in one error message in which case the undefined variable errors needs to be processed together. I'll change that once I'm back to work on Monday.

[thopre]

Merge handleErrors in handleAllErrors below

[jhenderson]

llvm/lib/Support/FileCheck.cpp

2064

Could you investigate a bit more and then update the comment with your findings? This code at the moment also skips some whitespace, which isn't always desirable (imagine if you had a regex match looking for some explicit whitespace before some string).

[thopre]

llvm/lib/Support/FileCheck.cpp

2064

The code was added in da108b4ed4e6e7267701e76d5fd3b87609c9ab77 when CHECK-NEXT was added and at the same time as the "scanning from here" diagnostic was added. There was no CHECK-SAME at the time and I guess the goal was to avoid confusion by showing the last matching line (even though the column info would show it's the end of the line). There was no regular expression match either, so no way to match a newline.

I think this should simply be removed, in a separate patch. What is your opinion?

[jhenderson]

llvm/lib/Support/FileCheck.cpp

2064

I'm inclined to agree with you. Perhaps worth adding a TODO or FIXME note here and at the other location, referencing each other, saying that this should probably be removed?

@probinson, (or anybody else) do you have any thoughts on this?

[jdenny]

Thanks for working on this. I'm sorry to start reviewing so late. I haven't digested the implementation fully yet, but I have a few comments to get started.

llvm/include/llvm/Support/FileCheck.h

107

The name MatchError and its comment seem too general. It could easily describe many of the following enumerators. I suggest:

/// Indicates a match that can never succeed because the pattern is found to
/// be undefined at match time (e.g., contains an undefined variable).
MatchUndefined,

Undefined variables are probably the only such case we have right now, and maybe that will always be true, but I'm trying to ensure this can handle more cases if needed in the future.

Also, the middle paragraph in the preceding comments is now incorrect. Here's a possible revision:

/// A directive's supplied pattern is said to be either undefined, expected,
/// or excluded depending on whether the pattern is well formed at match time
/// and whether it must have or must not have a match in order for the 
/// directive to succeed.  For example, a CHECK directive's pattern is 
/// normally expected, a CHECK-NOT directive's pattern is normally excluded, 
/// but either directive's pattern is undefined if it contains an undefined
/// variable at match time.  The match result type MatchUndefined is for 
/// undefined patterns.  All match result types whose names end with 
/// "Excluded" are for excluded patterns.  All other match result types are 
/// for expected patterns.

Here, I'm assuming you consistently use the new enumerator for undefined variables, regardless of whether a directive is positive or negative. I haven't checked the implementation, but the test suite changes seems to confirm that.

[jdenny]

llvm/test/FileCheck/dump-input-annotations.txt

626

This test used to cover printing of variable definitions when a positive directive fails. It no longer does.

A separate test could be added to cover that case. But do we need to drop that info when a variable is undefined? It could be helpful for debugging.

llvm/utils/FileCheck/FileCheck.cpp

201

If you agree to changing MatchError to MatchUndefined, I suggest changing error: match error to error: match undefined.

[thopre]

llvm/test/FileCheck/dump-input-annotations.txt

626

We currently only print substitutions if there are no errors. I think if we want to print substitutions in case of undefined variable we should do it regardless of whether there are errors to be consistent. Obviously that means ignoring substitution errors in printSubstitutions

[jdenny]

llvm/test/FileCheck/dump-input-annotations.txt

626

What's an example of an error besides undefined variable for which we currently don't print substitutions but you're suggesting we would?

[thopre]

llvm/test/FileCheck/dump-input-annotations.txt

626

Overflow and underflow come to mind. There might be others.

% cat overflow_undef.test
BIGVAR=10000000000000000
CHECK: BIGVAR: [[#BIGVAR:0x8000000000000000+0x8000000000000000]] [[#UNDEFVAR]]
% ./bin/FileCheck --dump-input=never --input-file overflow_undef.test overflow_undef.test
overflow_undef.test:2:19: error: unable to substitute variable or numeric expression: overflow error
CHECK: BIGVAR: [[#BIGVAR:0x8000000000000000+0x8000000000000000]] [[#UNDEFVAR]]
                  ^
zsh: exit 1     ./bin/FileCheck --dump-input=never --input-file overflow_undef.test

[jdenny]

llvm/include/llvm/Support/FileCheck.h

107

Another possibility that might be clearer is MatchPatternMalformed.

llvm/test/FileCheck/dump-input-annotations.txt

626

In the case of overflow/underflow, I'm in favor of printing used variable definitions. It would be especially helpful if it would print any variable definitions used in the expression that overflowed/underflowed, but printing other used variable definitions could be helpful as well.

Your example is a bit different than the case I was pointing out originally: in your example, there are no used variable definitions to report. There is however an undefined variable. I am also in favor of printing all errors (e.g., overflow and undefined variable) from a directive instead of just the first (overflow), where error recovery makes that feasible.

Again, if supporting all this info isn't feasible, then please at least add another test so we don't lose coverage of -dump-input printing used variable definitions when a positive directive with a well formed pattern fails to match.

(By the way, I just tried a few examples (without the current patch), and I'm noticing issues with overflow errors with respect to -dump-input. The traditional error message doesn't print undefined variables, but -dump-input does. -dump-input reports error: no match found twice and doesn't report the overflow. My master is a couple of months old already, so ignore me if all this has been fixed.)

llvm/utils/FileCheck/FileCheck.cpp

201

And MatchPatternMalformed would obviously become something like error: match pattern malformed.

[thopre]

@jhenderson I've made a patch to remove that skip-past-end-of-line code: https://reviews.llvm.org/D93341. Sorry for the long delay.

[thopre]

Address most of the comments.

[jhenderson]

Given that I've gone cold on this change and need to go through it again properly, and the number of other reviews that are on my plate, in addition to higher priority internal work I need to do before the Christmas break (my last day is Friday), I doubt I'll get a chance to look at this until some time in the New Year. Sorry! If people feel it's important to land sooner than that, please don't let my lack of review block this (I can always do post-commit reviews if needed).

[thopre]

llvm/lib/Support/FileCheck.cpp

2064

I've created https://reviews.llvm.org/D93341. I'll rebase this patch on top of it once the approach is agreed upon.

[thopre]

In D86222#2457257, @jhenderson wrote:

Given that I've gone cold on this change and need to go through it again properly, and the number of other reviews that are on my plate, in addition to higher priority internal work I need to do before the Christmas break (my last day is Friday), I doubt I'll get a chance to look at this until some time in the New Year. Sorry! If people feel it's important to land sooner than that, please don't let my lack of review block this (I can always do post-commit reviews if needed).

Tests still need to be fixed prior to landing this patch anyway, so don't worry it can wait. I've started working on that and sent 3 diff for review. A lot more will be needed though.

[thopre]

Rebase and massively simply thanks to Joel's work.

llvm/test/FileCheck/dump-input-annotations.txt

626

Does this updated patch solve your concern? Ignore all the tests failures, I still haven't gotten round to look at those.

[jdenny]

I haven't looked through the tests yet, but the implementation logic looks right to me. Thanks!

llvm/lib/FileCheck/FileCheck.cpp

2130 ↗

(On Diff #333458)

Is this change intentional? There's already a NotFoundError handler below (I'd prefer to keep its comment).

llvm/lib/FileCheck/FileCheckImpl.h

232 ↗

(On Diff #333458)

When is this function called now?

759 ↗

(On Diff #333458)

It looks like everything after the "or" is no longer true.

[thopre]

Address comments

llvm/lib/FileCheck/FileCheckImpl.h

232 ↗

(On Diff #333458)

It is a virtual function and thus needs to be defined. It's not called anywhere but I cannot remove it.

[jdenny]

You say there are many test failures. Should I hold off reviewing the test changes already in this patch, or are those fine?

llvm/lib/FileCheck/FileCheckImpl.h

232 ↗

(On Diff #333458)

Makes sense. So the point of the change here is, in case this function is one day called, to have a log message that is consistent with other ErrorInfo log messages now that old message here is no longer used by printSubstitutions. Right?

[thopre]

In D86222#2652919, @jdenny wrote:

You say there are many test failures. Should I hold off reviewing the test changes already in this patch, or are those fine?

Oh no you can review, those are errors in tests that this patch uncover from what I could see (I only looked at the first 10 or so). I'll mail llvm-def to ask people's help. The only thing missing are unit tests.

llvm/lib/FileCheck/FileCheckImpl.h

232 ↗

(On Diff #333458)

This change was made because I wanted to have a general error catcher in printNoMatch that would catch both UndefVarError and ErrorDiagnostic using the log() and message() methods from ErrorInfoBase. When changing the message to fit the current one I thought the "uses" is not really needed and is not consistent with other error message and the quotes and escaping are not needed since variable names are known to be well behaved.

However UndefVarError has no source manager attached to it and thus cannot print an error message with location info because there's no SourceMgr available in places where the error is created. Therefore I had to do this translation from UndefVarError to ErrorDiagnostic. But I've just found a way to reuse the log message (see last update) so it's all good.

[thopre]

Avoid error message duplication

[jdenny]

llvm/lib/FileCheck/FileCheckImpl.h

232 ↗

(On Diff #333458)

Ah, looks good.

llvm/test/FileCheck/dump-input/annotations.txt

633 ↗

(On Diff #333558)

Please restore the correct alignment here and elsewhere in this file.

(I'm starting to think it might have been better to use -strict-whitespace in most of these tests. I find myself turning it on temporarily anyway while adjusting tests because the squashed whitespace makes debugging confusing. My original goal for leaving it off was to make test updates easier if we were to adjust formatting, but that rarely happens. What do you think?)

645 ↗

(On Diff #333558)

This fixme can go now.

[thopre]

Fix whitespace

llvm/test/FileCheck/dump-input/annotations.txt

633 ↗

(On Diff #333558)

If whitespace is important it should be on. If formatting changes then we do want things to fail where whitespace is important.

[jdenny]

It seems the main issue left to address is tests that this patch breaks.

llvm/test/FileCheck/dump-input/annotations.txt

633 ↗

(On Diff #333558)

The first test in this file checks whitespace in formatting. The remaining tests do not check whitespace, but I find them to be more legible when at least aligned, such as aligning the error and possible intended match lines above with the other diagnostics. To help maintain that and make debugging easier, maybe -strict-whitespace is worthwhile. One difficulty is that some tests involve multiple verbosity levels, across which whitespace varies, but regexes could help there.

It seems we're accumulating many stylistic changes to make to these tests, so maybe I'll write a patch one day to tackle it all.

[thopre]

Rebase

[thopre]

In D86222#2653248, @jdenny wrote:

It seems the main issue left to address is tests that this patch breaks.

A lot of the failures (e.g. llvm/test/Transforms/GVN/big-endian.ll, llvm/test/Transforms/LoopVectorize/pr34681.ll, llvm/test/Transforms/LoopVectorize/X86/x86-pr39099.ll) are tests that use several CHECK-NOT with one line defining a variable and another one using it. The intent seems to be to check that a sequence of lines are not happening. These could be converted to a combined CHECK-NOT with {{(.*[[:space:]])+}} between each of the individual CHECK-NOT pattern involved but it quickly becomes unreadable. I think this highlight the need for a new feature like "CHECK-NOT-NEXT" if that makes sense. It might not be the best name, because I'm sure there will be case where they want successive line so we'd want the CHECK-NOT equivalent of (1) two consecutive CHECK, (2) a CHECK and a CHECK-NEXT and (3) a CHECK and a CHECK-SAME.

What do you think? Is adding a new feature first the best way forward? If yes, how would you design it?

[jdenny]

In D86222#2654396, @thopre wrote:

A lot of the failures (e.g. llvm/test/Transforms/GVN/big-endian.ll, llvm/test/Transforms/LoopVectorize/pr34681.ll, llvm/test/Transforms/LoopVectorize/X86/x86-pr39099.ll) are tests that use several CHECK-NOT with one line defining a variable and another one using it.

How many tests does this patch break?

What do you think? Is adding a new feature first the best way forward? If yes, how would you design it?

Those tests are passing without this patch, right? That should mean the CHECK-NOT that defines the variable never matches, so it should be impossible for the CHECK-NOT that uses the variable to match even if we had the feature you're suggesting. A quick fix is then to remove or comment out the CHECK-NOT that uses the variable. I believe the tests would be just as robust as now.

Before trying to design a new feature, I'd pursue that quick fix and see what the test authors say.

[thopre]

In D86222#2656914, @jdenny wrote:

In D86222#2654396, @thopre wrote:

A lot of the failures (e.g. llvm/test/Transforms/GVN/big-endian.ll, llvm/test/Transforms/LoopVectorize/pr34681.ll, llvm/test/Transforms/LoopVectorize/X86/x86-pr39099.ll) are tests that use several CHECK-NOT with one line defining a variable and another one using it.

How many tests does this patch break?

60, one of them being the FileCheck unittest which I haven't adapted yet. I've already made 4 patches solving 5 tests:

https://reviews.llvm.org/D99581
https://reviews.llvm.org/D99582
https://reviews.llvm.org/D99583
https://reviews.llvm.org/D99589

What do you think? Is adding a new feature first the best way forward? If yes, how would you design it?

Those tests are passing without this patch, right? That should mean the CHECK-NOT that defines the variable never matches, so it should be impossible for the CHECK-NOT that uses the variable to match even if we had the feature you're suggesting. A quick fix is then to remove or comment out the CHECK-NOT that uses the variable. I believe the tests would be just as robust as now.

Before trying to design a new feature, I'd pursue that quick fix and see what the test authors say.

You're right, let's try that.

[thopre]

In D86222#2658492, @thopre wrote:

In D86222#2656914, @jdenny wrote:

How many tests does this patch break?

60, one of them being the FileCheck unittest which I haven't adapted yet. I've already made 4 patches solving 5 tests:

Down to 19 when applying these diffs on top of current main branch:

https://reviews.llvm.org/D99581
https://reviews.llvm.org/D99582
https://reviews.llvm.org/D99589
https://reviews.llvm.org/D99602
https://reviews.llvm.org/D99770
https://reviews.llvm.org/D99771

[thopre]

Fix unit test failure, still need to add unit tests to cover new code

[thopre]

In D86222#2665262, @thopre wrote:

In D86222#2658492, @thopre wrote:

In D86222#2656914, @jdenny wrote:

How many tests does this patch break?

60, one of them being the FileCheck unittest which I haven't adapted yet. I've already made 4 patches solving 5 tests:

All tests fixed except for 2 related ones but with a RFC review. Most fix have already landed, 12 still in review (incl. the RFC one). We're almost there. :-)

[thopre]

• Add unit test for multiple errors in match()
• Rebase

[thopre]

Rebase

[thopre]

@jdenny I have no more changes to make to this patch and the CI is now green. Are you ok with the patch?

[jdenny]

Thanks for all that work on the other tests! Still LGTM.

[thopre]

In D86222#2677680, @jdenny wrote:

Thanks for all that work on the other tests! Still LGTM.

Thanks! Note there's still 4 dependent patches waiting for review so don't be surprised if it takes me a few days before I can land this.

[thopre]

Rebase

[thopre]

Fix comment

[thopre]

Rebase

llvm/include/llvm/Support/FileCheck.h

struct FileCheckDiag {
   /// 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 an error during match.
+    MatchError,
     /// 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.

llvm/lib/Support/FileCheck.cpp

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.
+    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();
       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 printNoMatch since now we know which
         // substitution block caused the overflow.
-        Error 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");
-            });
-        return std::move(Err);
+            }));
+        continue;
       }
 
       // Plop it into the regex at the adjusted offset.
       TmpStr.insert(TmpStr.begin() + Substitution->getIndex() + InsertOffset,
                     Value->begin(), Value->end());
       InsertOffset += Value->size();
     }
+    if (Errs)
+      return std::move(Errs);
 
     // Match the newly constructed regex.
     RegExToMatch = TmpStr;
   }
 
   SmallVector<StringRef, 4> MatchInfo;
   unsigned int Flags = Regex::Newline;
   if (IgnoreCase)
void Pattern::printSubstitutions(const SourceMgr &SM, StringRef Buffer,
                                  SMRange Range,
                                  FileCheckDiag::MatchType MatchTy,
                                  std::vector<FileCheckDiag> *Diags) const {
   // Print what we know about substitutions.
   if (!Substitutions.empty()) {
     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.
-      if (!MatchedValue) {
-        bool UndefSeen = false;
-        handleAllErrors(
-            MatchedValue.takeError(), [](const NotFoundError &E) {},
-            // Handled in 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);
-            });
-      } else {
-        // Substitution succeeded. Print substituted value.
-        OS << "with \"";
-        OS.write_escaped(Substitution->getFromString()) << "\" equal to \"";
-        OS.write_escaped(*MatchedValue) << "\"";
-      }
+      std::string MatchedValue =
+          cantFail(Substitution->getResult(),
+                   "printSubstitutions called with substitution failure");
+      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
       // substitution matches or was captured from exactly that range.
       if (Diags)
         Diags->emplace_back(SM, CheckTy, getLoc(), MatchTy,
                             SMRange(Range.Start, Range.Start), OS.str());
static void PrintMatch(bool ExpectedMatch, const SourceMgr &SM,
                        const FileCheckString &CheckStr, int MatchedCount,
                        StringRef Buffer, size_t MatchPos, size_t MatchLen,
                        FileCheckRequest &Req,
                        std::vector<FileCheckDiag> *Diags) {
   PrintMatch(ExpectedMatch, SM, CheckStr.Prefix, CheckStr.Loc, CheckStr.Pat,
              MatchedCount, Buffer, MatchPos, MatchLen, Req, Diags);
 }
 
-static void PrintNoMatch(bool ExpectedMatch, const SourceMgr &SM,
+/// Print and record diagnostics about the failure to match the input.
+static void 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");
+                         bool VerboseVerbose,
+                         std::vector<FileCheckDiag> *Diags) {
   bool PrintDiag = true;
   if (!ExpectedMatch) {
-    if (!VerboseVerbose) {
-      consumeError(std::move(MatchErrors));
+    if (!VerboseVerbose)
       return;
-    }
     // 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.
     PrintDiag = !Diags;
   }
 
   // If the current position is at the end of a line, advance to the start of
   // the next line.
   Buffer = Buffer.substr(Buffer.find_first_not_of(" \t\n\r"));
   FileCheckDiag::MatchType MatchTy = ExpectedMatch
                                          ? FileCheckDiag::MatchNoneButExpected
                                          : FileCheckDiag::MatchNoneAndExcluded;
   SMRange SearchRange = ProcessMatchResult(MatchTy, SM, Loc, Pat.getCheckTy(),
                                            Buffer, 0, Buffer.size(), Diags);
   if (Diags)
     Pat.printSubstitutions(SM, Buffer, SearchRange, MatchTy, Diags);
-  if (!PrintDiag) {
-    consumeError(std::move(MatchErrors));
-    return;
-  }
-
-  MatchErrors = handleErrors(std::move(MatchErrors),
-                             [](const ErrorDiagnostic &E) { E.log(errs()); });
-
-  // No problem matching the string per se.
-  if (!MatchErrors)
+  if (!PrintDiag)
     return;
-  consumeError(std::move(MatchErrors));
 
   // Print "not found" diagnostic.
   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");
 
   // Allow the pattern to print additional information if desired.
   Pat.printSubstitutions(SM, Buffer, SearchRange, MatchTy, nullptr);
 
   if (ExpectedMatch)
     Pat.printFuzzyMatch(SM, Buffer, Diags);
 }
 
-static void 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));
+/// Handle matching failure(s) \p MatchFailures and \returns if they were in
+/// fact errors during match. In both cases, it prints and records diagnostics
+/// in \p Diags, if non null, for the failures when matching \p MatchCount
+/// times \pBuffer against the pattern \p Pat at location \p Loc. The level of
+/// diagnostics is controled by \p VerboseVerbose.
+static bool handleMatchFailures(bool ExpectedMatch, const SourceMgr &SM,
+                                StringRef Prefix, SMLoc Loc, const Pattern &Pat,
+                                int MatchCount, StringRef Buffer,
+                                Error MatchFailures, bool VerboseVerbose,
+                                std::vector<FileCheckDiag> *Diags) {
+  Error MatchErrors =
+      handleErrors(std::move(MatchFailures), [](const NotFoundError &E) {});
+  if (!MatchErrors) {
+    printNoMatch(ExpectedMatch, SM, Prefix, Loc, Pat, MatchCount, Buffer,
+                 VerboseVerbose, Diags);
+    return false;
+  }
+
+  // If the current position is at the end of a line, advance to the start of
+  // the next line.
+  Buffer = Buffer.substr(Buffer.find_first_not_of(" \t\n\r"));
+  FileCheckDiag::MatchType MatchTy = FileCheckDiag::MatchError;
+  Check::FileCheckType CheckTy = Pat.getCheckTy();
+  ProcessMatchResult(MatchTy, SM, Loc, CheckTy, Buffer, 0, Buffer.size(),
+                     Diags);
+
+  auto PrintMatchErrorDiagnostic = [&](StringRef DiagMsg, SMLoc DiagLoc) {
+    // 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
+    // substitution matches or was captured from exactly that range.
+    if (Diags)
+      Diags->emplace_back(SM, CheckTy, Loc, MatchTy, SMRange(DiagLoc, DiagLoc),
+                          DiagMsg);
+    SM.PrintMessage(DiagLoc, SourceMgr::DK_Error, DiagMsg);
+  };
+
+  // Print undefined variable diagnostics.
+  MatchErrors =
+      handleErrors(std::move(MatchErrors), [&](const UndefVarError &E) {
+        SmallString<256> Msg;
+        raw_svector_ostream OS(Msg);
+        OS << "uses undefined variable: ";
+        E.log(OS);
+        PrintMatchErrorDiagnostic(OS.str(),
+                                  SMLoc::getFromPointer(E.getVarName().data()));
+      });
+
+  // Print errors with diagnostic. No other error should exist at this stage.
+  handleAllErrors(std::move(MatchErrors),
+                  [&](const UndefVarError &E) {
+                    SmallString<256> Msg;
+                    raw_svector_ostream OS(Msg);
+                    OS << "uses undefined variable: ";
+                    E.log(OS);
+                    PrintMatchErrorDiagnostic(
+                        OS.str(), SMLoc::getFromPointer(E.getVarName().data()));
+                  },
+                  [&](const ErrorDiagnostic &E) {
+                    const SMDiagnostic &Diagnostic = E.getDiagnostic();
+                    PrintMatchErrorDiagnostic(Diagnostic.getMessage(),
+                                              Diagnostic.getLoc());
+                  });
+
+  return true;
+}
+
+/// Wrapper around the other handleMatchFailures function where the pattern,
+/// its count and location are set from the positive matching pattern referred
+/// to by \p CheckStr.
+static bool handleMatchFailures(bool ExpectedMatch, const SourceMgr &SM,
+                                const FileCheckString &CheckStr,
+                                int MatchedCount, StringRef Buffer,
+                                Error MatchFailures, bool VerboseVerbose,
+                                std::vector<FileCheckDiag> *Diags) {
+  return handleMatchFailures(ExpectedMatch, SM, CheckStr.Prefix, CheckStr.Loc,
+                             CheckStr.Pat, MatchedCount, Buffer,
+                             std::move(MatchFailures), 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,
   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);
 
     // report
     if (!MatchResult) {
-      PrintNoMatch(true, SM, *this, i, MatchBuffer, Req.VerboseVerbose, Diags,
-                   MatchResult.takeError());
+      handleMatchFailures(/*ExpectedMatch=*/true, SM, *this, i, MatchBuffer,
+                          MatchResult.takeError(), Req.VerboseVerbose, Diags);
       return StringRef::npos;
     }
     size_t MatchPos = *MatchResult;
     PrintMatch(true, SM, *this, i, MatchBuffer, MatchPos, CurrentMatchLen, Req,
                Diags);
     if (i == 1)
       FirstMatchPos = LastPos + MatchPos;
 
bool FileCheckString::CheckNot(const SourceMgr &SM, StringRef Buffer,
   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);
 
     if (!MatchResult) {
-      PrintNoMatch(false, SM, Prefix, Pat->getLoc(), *Pat, 1, Buffer,
-                   Req.VerboseVerbose, Diags, MatchResult.takeError());
+      if (handleMatchFailures(
+              /*ExpectedMatch=*/false, SM, Prefix, Pat->getLoc(), *Pat, 1,
+              Buffer, MatchResult.takeError(), Req.VerboseVerbose, Diags))
+        DirectiveFail = true;
       continue;
     }
     size_t Pos = *MatchResult;
 
     PrintMatch(false, SM, Prefix, Pat->getLoc(), *Pat, 1, Buffer, Pos, MatchLen,
                Req, Diags);
     DirectiveFail = true;
     continue;
   }
for (auto PatItr = DagNotStrings.begin(), PatEnd = DagNotStrings.end();
     // 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);
       // 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());
+        handleMatchFailures(/*ExpectedMatch=*/true, SM, Prefix, Pat.getLoc(),
+                            Pat, 1, MatchBuffer, MatchResult.takeError(),
+                            Req.VerboseVerbose, Diags);
         return StringRef::npos;
       }
       size_t MatchPosBuf = *MatchResult;
       // 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);

llvm/lib/Support/FileCheckImpl.h

public:
   static char ID;
 
   ErrorDiagnostic(SMDiagnostic &&Diag) : Diagnostic(Diag) {}
 
   std::error_code convertToErrorCode() const override {
     return inconvertibleErrorCode();
   }
 
+  const SMDiagnostic &getDiagnostic() const { return Diagnostic; }
+
   /// 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) {
     return make_error<ErrorDiagnostic>(
         SM.GetMessage(Loc, SourceMgr::DK_Error, ErrMsg));
   }
 

llvm/test/FileCheck/dump-input-annotations.txt

 ; RUN: | FileCheck -match-full-lines %s -check-prefix=SUBST-POS
 
 ;      SUBST-POS:<<<<<<
 ; SUBST-POS-NEXT:           1: def-match1 def-match2
 ; 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:           2: def-match1 def-nomatch
-; 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'3                            with "DEF_NOMATCH" equal to "foobar"
-; SUBST-POS-NEXT:check:2'4              ?             possible intended match
+; SUBST-POS-NEXT:check:2'0     X~~~~~~~~~~~~~~~~~~~~~ error: match error
+; SUBST-POS-NEXT:check:2'1                            uses undefined variable: "UNDEF"
 ; 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

 ; RUN: %ProtectFileCheckOutput \
 ; RUN: not FileCheck -dump-input=always -vv -input-file=%t.in %t.chk 2>&1 \
 ; RUN:               -DDEF_MATCH1=def-match1 -DDEF_MATCH2=def-match2 \
 ; RUN:               -DDEF_NOMATCH=foobar \
 ; 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:not:1'3                            with "DEF_NOMATCH" equal to "foobar"
+; SUBST-NEG-NEXT:not:1'0     X~~~~~~~~~~~~~~~~~~~~~ error: match error
+; SUBST-NEG-NEXT:not:1'1                            uses undefined variable: "UNDEF"
 ; 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:check:3     ^~~
 ; SUBST-NEG-NEXT:>>>>>>

llvm/test/FileCheck/numeric-expression.txt

 11  // CHECK-NEXT: {{^}}[[#==UNSI]]
 11  // CHECK-NEXT: {{^}}[[# == UNSI]]
 12  // CHECK-NEXT: {{^}}[[#UNSI2: == UNSI + 1]]
 12  // CHECK-NEXT: {{^}}[[#==UNSI2]]
 
 ; Numeric expressions in default matching format and explicit matching rule using
 ; variable defined on other lines with match failure.
 RUN: %ProtectFileCheckOutput \
 RUN: not FileCheck --check-prefixes CHECK,NUMEXPR-CONSTRAINT-NOMATCH --input-file %s %s 2>&1 \
 RUN:   | FileCheck --check-prefix NUMEXPR-CONSTRAINT-NOMATCH-MSG --strict-whitespace %s
 
 USE DEF FMT EXPL NO MATCH
 12
 NUMEXPR-CONSTRAINT-NOMATCH-LABEL: USE DEF FMT EXPL NO MATCH
 NUMEXPR-CONSTRAINT-NOMATCH-NEXT: [[#==UNSI]]
 NUMEXPR-CONSTRAINT-NOMATCH-MSG: numeric-expression.txt:[[#@LINE-1]]:34: error: {{N}}UMEXPR-CONSTRAINT-NOMATCH-NEXT: expected string not found in input
 NUMEXPR-CONSTRAINT-NOMATCH-MSG-NEXT: {{N}}UMEXPR-CONSTRAINT-NOMATCH-NEXT: {{\[\[#==UNSI\]\]}}

 RUN: %ProtectFileCheckOutput \
 RUN: not FileCheck --check-prefix UNDEF-USE --input-file %s %s 2>&1 \
 RUN:   | FileCheck --strict-whitespace --check-prefix UNDEF-USE-MSG %s
 
 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-6]]:1: note: scanning from here
-UNDEF-USE-MSG-NEXT:      UNDEFVAR: 11
-UNDEF-USE-MSG-NEXT: {{^}}^{{$}}
-UNDEF-USE-MSG-NEXT:      numeric-expression.txt:[[#@LINE-9]]:1: note: uses undefined variable(s): "UNDEFVAR1" "UNDEFVAR2"
-UNDEF-USE-MSG-NEXT:      UNDEFVAR: 11
-UNDEF-USE-MSG-NEXT: {{^}}^{{$}}
+UNDEF-USE-MSG:      numeric-expression.txt:[[#@LINE-1]]:30: error: uses 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: uses 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: uses 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: uses 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 \
 RUN: not FileCheck -D#NUMVAR=10 --check-prefix INVAL-OP \
 RUN:               --input-file %s %s 2>&1 \
 RUN:   | FileCheck --strict-whitespace --check-prefix INVAL-OP-MSG %s
 
 INVALID OPERATOR
 NUMVAR*2: 22

llvm/test/FileCheck/var-scope.txt

 ; Test that variables not starting with dollar sign get undefined after a
 ; CHECK-LABEL directive iff --enable-var-scope is used.
 
 ; Reference run: variables remain defined at all time when not using
 ; --enable-var-scope option.
 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
 CHECK: [[LOCAL:loc[^[:digit:]]*]][[#LOCNUM:]]
 CHECK: [[$GLOBAL:glo[^[:digit:]]*]][[#$GLOBNUM:]]
 
 local2
 global2
 CHECK: [[LOCAL]][[#LOCNUM+1]]
 CHECK: [[$GLOBAL]][[#$GLOBNUM+1]]
 
 barrier:
 CHECK-LABEL: barrier
 
 local3
 global3
 LOCAL1: [[LOCAL]]3
 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: uses undefined variable: "LOCAL"
+ERRUNDEFLOCNUM: uses undefined variable: "LOCNUM"

llvm/utils/FileCheck/FileCheck.cpp

MarkerStyle(char Lead, raw_ostream::Colors Color,
       : Lead(Lead), Color(Color), Note(Note), FiltersAsError(FiltersAsError) {
     assert((!FiltersAsError || !Note.empty()) &&
            "expected error diagnostic to have note");
   }
 };
 
 static MarkerStyle GetMarker(FileCheckDiag::MatchType MatchTy) {
   switch (MatchTy) {
+  case FileCheckDiag::MatchError:
+    return MarkerStyle('X', raw_ostream::RED, "error: match error",
+                       /*FiltersAsError=*/true);
   case FileCheckDiag::MatchFoundAndExpected:
     return MarkerStyle('^', raw_ostream::GREEN);
   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);