FileCheck [6/12]: Introduce numeric variable definition

[thopre]

Summary

This patch is part of a patch series to add support for FileCheck
numeric expressions. This specific patch introduces support for defining
numeric variable in a CHECK directive.

This commit introduces support for defining numeric variable from a
litteral value in the input text. Numeric expressions can then use the
variable provided it is on a later line.

Copyright:

• Linaro (changes up to diff 183612 of revision D55940)
• GraphCore (changes in later versions of revision D55940 and in new revision created off D55940)

[thopre]

Rebase on top of latest changes

[thopre]

Rebase on top of latest changes

[probinson]

Starting set of comments on this one. Haven't looked at everything yet.

llvm/docs/CommandGuide/FileCheck.rst

623 ↗

(On Diff #199235)

Which makes the above example illegal, so you should rewrite the example.

llvm/include/llvm/Support/FileCheck.h

379 ↗

(On Diff #199235)

"of the numeric"
comma after "fails"

382 ↗

(On Diff #199235)

That last sentence feels awkward... maybe:
"If the expression defines a numeric variable, sets \p DefinedNumericVariable to point to the class representing the variable."
(with maybe "or nullptr if it does not define a variable.")

397 ↗

(On Diff #199235)

Need to remove LineNumber from the documentation, if it's no longer a parameter.

llvm/lib/Support/FileCheck.cpp

42 ↗

(On Diff #199235)

Omit the != nullptr on pointer assertions.

170 ↗

(On Diff #199235)

I think this comment should not have changed.

289 ↗

(On Diff #199235)

no else after return

388 ↗

(On Diff #199235)

variables -> variable names

412 ↗

(On Diff #199235)

= false

467 ↗

(On Diff #199235)

I think NumExpr and DefinedNumericVariable could be declared here, much closer to where they are used.

473 ↗

(On Diff #199235)

Omit != nullptr

1794 ↗

(On Diff #199235)

"detect the use of an undefined"
(missing rebase maybe?)

[thopre]

• Address Paul's review comments
• Rename numeric expression subsitution for numeric substitution to clearly distinguish numeric expression (the part that gets evaluated) from the whole #<something> that gets substituted (which might include a numeric variable definition)
• Rename ParsePattern into parsePattern since it is modified

[thopre]

llvm/lib/Support/FileCheck.cpp

1794 ↗

(On Diff #199235)

Rather incorrect conflict resouition on my part I suspect.

[probinson]

llvm/docs/CommandGuide/FileCheck.rst

606 ↗

(On Diff #199390)

"lines" (or maybe just "text" as I suspect you'll want to go back to the one-line example in patch 12 of this series).

613 ↗

(On Diff #199390)

"lines" (or "text")

llvm/lib/Support/FileCheck.cpp

472 ↗

(On Diff #199390)

IsNumExpr is guaranteed true here, because of the simplified 'if' expression above.

llvm/test/FileCheck/numeric-expression.txt

78 ↗

(On Diff #199390)

It appears that the only reason to include the CHECK prefix here is so it will define VAR1 for you? Maybe use a different variable instead, so the sections of the test are less intertwined.

90 ↗

(On Diff #199390)

Needs a period/full-stop at the end of the sentence.

llvm/test/FileCheck/var-scope.txt

4 ↗

(On Diff #199390)

Please add a comment noting that this first RUN doesn't enable scoping.
(I've been caught by that twice now! Clearly I need help reading tests.)

[thopre]

• Address review comments
• Add final point for all comments in numeric-expression.txt

[thopre]

llvm/test/FileCheck/numeric-expression.txt

32–33 ↗

(On Diff #194481)

@arsenm: ping?

[jhenderson]

I've barely scratched the surface of this so far, but here are some initial comments for where I've got to.

llvm/docs/CommandGuide/FileCheck.rst

576 ↗

(On Diff #199479)

Since an example usage is actually [[#FOO:]], either lose the '<' and '>' in the first bit, or add them in the second.

590 ↗

(On Diff #199479)

Perhaps worth adding "or on the command line".

llvm/include/llvm/Support/FileCheck.h

45 ↗

(On Diff #199479)

class, variable -> class. Variable

56 ↗

(On Diff #199479)

I'd be inclined to make this size_t rather than unsigned, as it's tied to the file system/memory availability. (However, see the note below).

Probably this should be assigned to 0 for command-line defined variables?

60–61 ↗

(On Diff #199479)

It is theoretically possible for a standards-compliant compiler to find this and the next constructor ambiguous, because there is nothing stopping unsigned to be the same size as uint64_t. This becomes even more likely for size_t versus uint64_t. Furthermore, a user could very easily call the wrong constructor unintentionally.

I'd therefore suggest changing the signature somehow so as to avoid the risk of confusion for users of this class. Options include writing a pair of factory methods (effectively named constructors), or reordering the signature of one. There may be other options too.

145 ↗

(On Diff #199479)

I'm not sure I understand why these comments have changed?

225 ↗

(On Diff #199479)

What is a "capture number"? It probably needs explaining.

232 ↗

(On Diff #199479)

See above comment. I have no idea what a "capture number" is or why it needs parenthesizing.

256 ↗

(On Diff #199479)

definition -> definitions.

257 ↗

(On Diff #199479)

from -> by (as it was).

259 ↗

(On Diff #199479)

What is "NumericVariableDefs" referring to here?

332 ↗

(On Diff #199479)

See above comments re. capture parentheses

pointer to corresponding -> pointer to the corresponding

335 ↗

(On Diff #199479)

Should this use a StringMap?

354 ↗

(On Diff #199479)

I'd change this name to Line, to avoid any risk of clashes with the member.

380 ↗

(On Diff #199479)

I'd put a comma after "substituted"

442 ↗

(On Diff #199479)

Commas after "expression" and "fails".

The whole first part of this sentence feels clunky. I think it's the use of "that" that's the problem. Probably just replace with "the".

444 ↗

(On Diff #199479)

Could this just be parseBinop?

[thopre]

Address review comments, in particular:

parenthesized capture number -> parenthesized subexpression number in RegExStr

(to capture the variable value)

llvm/include/llvm/Support/FileCheck.h

145 ↗

(On Diff #199479)

Prior to this patch, the only thing #<something> block could contain was a numeric expression to substitute by its value when matching. It can now contain a numeric variable definition which needs to be substituted for a suitable wildcard expression ([0-9]+), in which case there is no numeric expression. I therefore thought the name "numeric expression substitution" was inappropriate and decided to change it to "numeric substitution". While patterns variable are never involved in expression, for consistency I've decided to also rename pattern variable substitution into pattern substitution.

I think there was also some inconsistency (perhaps in later patches in the series) where #<something> used to be referred to as numeric expression, now there is a separate name for this and the numeric expression part of a numeric substitution.

[probinson]

There's still some inconsistency in the terminology, as clearly evidenced by the name and description of the IsNumExpr member of FileCheckPatternSubstitution. That class used to be where we'd find variables (now called pattern variables); but it has had numeric expressions crammed into it, except the comment wants to call it a substitution, and so we have "pattern" and "substitution" and "expression" all competing for clarity.

I'm uncertain what to suggest. It may be that "pattern variable" was not a great name, and instead it should be "string variable" to contrast more naturally with "numeric variable." And then substitutions are just substitutions, which can be either a string substitution or a numeric substitution, with the numeric substitution implicitly using a numeric expression. Following this idea, FileCheckPatternSubstitution would be renamed FileCheckSubstitution and the references to "numeric expression" within the class don't change. I don't know that I'd go so far as to have String and Numeric subclasses of FileCheckSubstitution, the abstraction of a Substitution might not really support that. But the naming can surely be improved.

What do you think? This is a discussion point, not a please-do-this comment.

llvm/include/llvm/Support/FileCheck.h

66 ↗

(On Diff #199579)

This leaves DefLineNumber uninitialized. Maybe combine the two constructors into a single 3-parameter constructor? Value can still default to None if it's the last parameter.

130 ↗

(On Diff #199579)

See main comment.

355 ↗

(On Diff #199579)

Multiple parameters means you don't need 'explicit'.

[thopre]

In D60386#1504795, @probinson wrote:

There's still some inconsistency in the terminology, as clearly evidenced by the name and description of the IsNumExpr member of FileCheckPatternSubstitution. That class used to be where we'd find variables (now called pattern variables); but it has had numeric expressions crammed into it, except the comment wants to call it a substitution, and so we have "pattern" and "substitution" and "expression" all competing for clarity.

I'm uncertain what to suggest. It may be that "pattern variable" was not a great name, and instead it should be "string variable" to contrast more naturally with "numeric variable." And then substitutions are just substitutions, which can be either a string substitution or a numeric substitution, with the numeric substitution implicitly using a numeric expression. Following this idea, FileCheckPatternSubstitution would be renamed FileCheckSubstitution and the references to "numeric expression" within the class don't change. I don't know that I'd go so far as to have String and Numeric subclasses of FileCheckSubstitution, the abstraction of a Substitution might not really support that. But the naming can surely be improved.

That's a great idea, especially the "string variable" and "string substitution". I was not pleased with the name either but the only thing I thought of was regex variable which is plain wrong, I completely missed the obvious. I think there's still a confusion left with "substitution". The class represents the use of a numeric or string variable but I also use it in place to refer to the [[]] blocks (eg. "Numeric substitutions start with a '#' sign after the double brackets and also have the definition and substitution forms"). While even definition do lead to a substitution (since the square brackets are replaced by something else) I think substitution seems to refer more naturally to the use of a variable. So I want a name for the [[]] blocks but the only thing I can come up so far is substitution block. So you'd have string substitution blocks and numeric substitution blocks (or just substitution block when the distinction does not matter) which can be either string/numeric variable definition or string/numeric substitutions. Can you think of a better naming than substitution block? If not, I'll run with it.

I think I might have a go with a class hierarchy for FileCheckSubstitution as it might avoid the isNumExpr member variable, unless you think it's overkill.

What do you think? This is a discussion point, not a please-do-this comment.

I quite like the suggestion even if that means make an extra separate patch obviously. Naming clarity is very important to make the comments and thus the code understandable. It's going to be painful changing some of the error messages and thus the tests but it's worth it IMHO.

[probinson]

That's a great idea, especially the "string variable" and "string substitution". I was not pleased with the name either but the only thing I thought of was regex variable which is plain wrong, I completely missed the obvious.

Naming is hard, I clearly didn't think of it right away either.

I think I might have a go with a class hierarchy for FileCheckSubstitution as it might avoid the isNumExpr member variable, unless you think it's overkill.

The question is really: Is there a reasonable interface common to the two cases for consumers, and can the implementation be separated cleanly into string and numeric cases? The goal here should be to make the interface usable and the implementation understandable. If a hierarchy achieves that goal, then sure.

[thopre]

Rebase after renaming patch

[thopre]

llvm/lib/Support/FileCheck.cpp

385 ↗

(On Diff #200334)

Typo: Numeric substitution* blocks

[thopre]

• Rebase on top of latest changes
• Fix typo

[thopre]

Rename parseNumericSubstitution into parseNumericSubstitutionBlock since it also parses numeric variable definition

[thopre]

Fix unit tests

[thopre]

Prevent double reporting in definedCmdlineVariables

[jhenderson]

Looked at everything up to, but not including the unit tests. Thanks for all the work on this!

llvm/include/llvm/Support/FileCheck.h

355 ↗

(On Diff #199579)

There's still an explicit here (but it's worth noting that explicit does still have an effect on multi-parameter constructor).

332 ↗

(On Diff #199479)

I continue to not understand the "capture parenethese number"...

45 ↗

(On Diff #200888)

the respective definition -> their respective definitions

52 ↗

(On Diff #200888)

Not a new thing, but does this need the llvm::?

61 ↗

(On Diff #200888)

I don't think you need to initialize Value here explicitly. The llvm::Optional default constructor should do that.

83 ↗

(On Diff #200888)

This should return size_t to match DefLineNumber's type.

245–246 ↗

(On Diff #200888)

I still don't understand what a "parenthesized subexpression number is" or what a CaptureParen is.

279 ↗

(On Diff #200888)

in -> in the

386 ↗

(On Diff #200888)

This type should match that of the earlier DefLineNumber member.

llvm/lib/Support/FileCheck.cpp

152 ↗

(On Diff #200888)

variable -> variables

182 ↗

(On Diff #200888)

same line -> same line as used (?)

201 ↗

(On Diff #200888)

Don't think this should change in this patch, but maybe this is a hint that parseNumericVariable and other similar functions should be returning an llvm::Error rather than a boolean, and for the errors to be handled much higher up the stack somewhere.

Returning booleans can result in confusion or people missing a check somewhere.

[jhenderson]

llvm/lib/Support/FileCheck.cpp

256 ↗

(On Diff #200888)

Nit: delete this blank line.

290 ↗

(On Diff #200888)

Parse the numeric...

385–387 ↗

(On Diff #200888)

I'd change this sentence to be: "Numeric substitution blocks work the same way as string ones, but start with a '#' sign after the double brackets."

409 ↗

(On Diff #200888)

I think it would be slightly clearer to just call this IsDefinition.

509 ↗

(On Diff #200888)

No need for struct here.

519 ↗

(On Diff #200888)

Mark string -> Mark the string

520 ↗

(On Diff #200888)

variable -> variables

522 ↗

(On Diff #200888)

that -> this

523 ↗

(On Diff #200888)

loose -> lose
(loose == opposite of tight, lose == opposite of win)

633 ↗

(On Diff #200888)

See earlier comments about CaptureParen etc. As a result, I don't really follow what this code is doing.

643 ↗

(On Diff #200888)

I'm not sure you've given the right error message here. Isn't the problem that MatchedValue does not represent a decimal integer? If that's the case, I'd go with an error message saying that.

1713–1714 ↗

(On Diff #200888)

This seems like a bit of a weird thing to do. Why not just call the function directly? In other words, I suspect the comment should be updated.

1738 ↗

(On Diff #200888)

"Expr test" needs to be explained more. I assume it's referring to the Expr.empty() line below, but perhaps better would be to outline what we're testing a bit more. Also, if FOO+2=10 is invalid here, but the code passes, maybe that implies we should be calling a different function (which presumably would be a subset of the parseNumericVariable function).

llvm/test/FileCheck/numeric-expression.txt

1 ↗

(On Diff #200888)

Use double-dash like the test was before here.

11 ↗

(On Diff #200888)

"in alternate spacing" is a weird phrase. Perhaps "with different spacing"

17–19 ↗

(On Diff #200888)

Do you ever use these definitions? I don't see them being used, but they should be, as otherwise you aren't really testing anything.

31–32 ↗

(On Diff #200888)

See above comment about "in alternate spacing". Also, there's a rogue full-stop at the end of line 31, but none at the end of line 32.

78 ↗

(On Diff #200888)

Put the double-dashes back, please, throughout the test.

99 ↗

(On Diff #200888)

You've used "PAT" here, presumably as an abbreviation for "pattern", but I don't think that abbreviation makes sense any more given the recent naming changes?

llvm/test/FileCheck/var-scope.txt

4 ↗

(On Diff #200888)

"variable are remain defined"

Not sure what this is saying exactly, but it isn't good English. Probably simply "variables remain..."

18–19 ↗

(On Diff #200888)

Doesn't adding the ':' change the semantics of this test? Why are you doing that?

[probinson]

llvm/include/llvm/Support/FileCheck.h

355 ↗

(On Diff #199579)

Really? What effect does explicit have on a multi-parameter constructor? My reading of the cppreference.org description is that it used to apply to T var = {args}; but that is list-initialization starting with C++11.

[jhenderson]

llvm/include/llvm/Support/FileCheck.h

355 ↗

(On Diff #199579)

The example on https://en.cppreference.com/w/cpp/language/explicit, shows that copy-list-initialization does not consider conversion constructors marked with explicit, from C++11. Take a look at the A3, A4, B3 and B4 examples. Summary version is that A var = {arg1, arg2}; will match a constructor without explicit, but not one with, whilst A var {arg1, arg2}; will work for both.

Mind you, it's not likely to actually matter in most instances, and we may not actually want the behaviour of explicit.

[probinson]

llvm/include/llvm/Support/FileCheck.h

355 ↗

(On Diff #199579)

Okay, C++ is just too weird.
My point, and I did have one, is that explicit on a multi-parameter ctor is generally not used within LLVM, because the syntax where it matters is obviously construction or initialization anyway. LLVM tends to prefer the less-verbose syntax where there's no real benefit to the more-verbose syntax. The explicit was appropriate when this was a one-parameter ctor, and isn't really useful now, so it should be removed.
Or, if @thopre thinks it is useful, say why.

[thopre]

• Address not contested review comments
• rebase on top of llvm:: prefix removal

[thopre]

In D60386#1513730, @jhenderson wrote:

Looked at everything up to, but not including the unit tests. Thanks for all the work on this!

And many thanks again for your patience and dedicated reviews. I apologies for all the change which I fail occasionally to carry over when resolving conflict in rebases and for the english and C++ rookie mistakes.

llvm/include/llvm/Support/FileCheck.h

332 ↗

(On Diff #199479)

Oops, forgot to change that one. See above for the explanation, can't come up with a more intelligible terminology. pcresyntax(3) manual refer to them as capturing group but I don't think you'll like that any better. Would "regex parenthesized subexpression number" be clearer?

355 ↗

(On Diff #199579)

I'm not sure I understand where you're getting at. Are you suggesting that explicit was only useful when the constructor only had the Ty parameter?

52 ↗

(On Diff #200888)

No, and there's also some existing function with llvm:: prefix. Is it ok if I schedule a patch to remove all of them just after this patch (i.e. between patch 6 and 7)?

245–246 ↗

(On Diff #200888)

I took the naming from the regex (7) manual. It refers to a parenthesis block in a regex that capture a part of the result. E.g. foo([0-9]) would capture the digit 5 if matched against foo5. I don't mind changing it to another more explicit naming (though I like this one) but would be opposed to explaining the term here because I would not know which of the occurences of this name should have the explanation.

Any suggestion?

llvm/lib/Support/FileCheck.cpp

201 ↗

(On Diff #200888)

Mmmh, I'll try to schedule a patch to that effect once this patch is in.

633 ↗

(On Diff #200888)

Each numeric expression that defines a numeric variable will have its corresponding substring in RegExStr surrounded by parenthesis which will store the text that matches it in an entry of the MatchInfo array below. I thus check that the index (capture parenthesis number) is not higher than the number of entries and then record the text that was match in the numeric variable.

643 ↗

(On Diff #200888)

Not only. It could be a too big value (e.g. a value that would require more than 64 bits to represent). That error message covers both cases. Later on it also covers cases where the format of the number does not match the format expected (e.g. the number in the input is 0xAB but the variable has unsigned decimal format).

1713–1714 ↗

(On Diff #200888)

I'm not following. The function parseNumericVariable is an instance method so needs an object to invoke it. The reason for it is that it checks for collision between numeric and string variables among other things.

1738 ↗

(On Diff #200888)

That would create a function that would be called only here and contain the same code as below, wouldn't it? parseNumericVariable check whether there is a numeric variable at the start of the string it gets as parameter and verifies that this numeric variable is allowed in the context (ie. exists if it is a use, or does not conflict with a string variable if it is a definition). In this example it correctly detects FOO in FOO+2 and strip it from the original string. It is the role of the caller to decide what to do next. parseBinop for instance accepts happily to have an operator after that, but here we only want to have a variable.

This test is effectively saying: does this start with a numeric variable and is there nothing after that, i.e. is this string only a valid numeric variable.

llvm/test/FileCheck/numeric-expression.txt

99 ↗

(On Diff #200888)

Argh indeed.

llvm/test/FileCheck/var-scope.txt

18–19 ↗

(On Diff #200888)

This test is divided in 3 parts: (i) the first part defines some local and global variable, (ii) the second part checks that they can be matched before a LABEL barrier irregardless of whether --enable-var-scope is used and (iii) the third and last part checks that only global variable can be matched after a LABEL barrier when using --enable-var-scope.

Prior to this test the only way to define a numeric variable was via the -D command-line option hence the first part was actually doing a variable use for numeric variable. With this patch this bit can be turned into numeric variable definition to be consistent. -D command-line option is already tested in another testfile.

[thopre]

Rebase on new version of llvm:: prefix removal patch

[jhenderson]

I'm not likely going to have any chance to look at this again by the end of the day, and I'm away all of next week. I'm okay if this gets approved by @probinson and all my outstanding comments have been addressed for this to land without me needing to look further at it.

[thopre]

In D60386#1515944, @jhenderson wrote:

I'm not likely going to have any chance to look at this again by the end of the day, and I'm away all of next week. I'm okay if this gets approved by @probinson and all my outstanding comments have been addressed for this to land without me needing to look further at it.

Is there any chance you could acknowledge whether the responses I've given to some of your remarks are satisfying?

[jhenderson]

I've responded to as much as I have time for. Hopefully the rest @probinson will understand what I'm talking about.

llvm/include/llvm/Support/FileCheck.h

392 ↗

(On Diff #200999)

size_t here?

245–246 ↗

(On Diff #200888)

I think there's a difference between capturing groups and what this code is doing, because the rest of the check isn't a regex usually. Indeed, you could end up with actual groups inside your regex pattern e.g. [[FOO:(a group)]] which could risk confusion. Also, I feel like "group" is a far more commonly understood term in regexes.

I think part of the problem is that this description refers to "RegExStr" which isn't a class member of global variable, so the class is no longer standalone in its interpretation.

If you really think that you should use the same terminology as regular regexes do, call them "CaptureGroup" or "GroupNumber" or something. What is the Paren actually referring to in the variable name below, for example? The name to me implies it's recording a number to do with a parenthesis, but the number's meaning is unclear.

llvm/lib/Support/FileCheck.cpp

1713–1714 ↗

(On Diff #200888)

Right, what I'm saying is, could the validation logic move out of parseNumericVariable into a free function, shared by parseNumericVariable and the code here.

[thopre]

• Split parseNumericVariable into parseNumericVariableUse and parseNumericVariableDefinition since little is shared between two usages of this function
• Change API of parseVariable given that most callers extract the variable name and the trailer from TrailIdx
• Rename CaptureParen into CaptureParenGroup and hopefully clarify what this is about

[thopre]

Make parseNumericVariableDefinition private

[thopre]

llvm/test/FileCheck/numeric-expression.txt

32–33 ↗

(On Diff #194481)

Ping?

[probinson]

Grammar nits and one longer point, which is:
I understand where the "parenthesis group" term came from, but I think it's not appropriate here. While a CHECK line is implicitly a regex, and variables are a kind of back-reference, the syntax for variable references (use or def) is not parentheses and the content is not a "group" in any real sense, and so "parenthesis group" is an un-obvious and confusing term.
Because FileCheck uses square brackets, I propose that the least disruptive change at this point would be to call them "bracket expressions." The "bracket" part is obvious, and "expression" because (a) definitions will have a regex, and (b) numeric variable references are implicitly expressions. (Okay, still a bit of a stretch, but less so that "parenthesis group" IMO.)
WDYT?

Also let me apologize in advance for the review sparsity lately, and going forward. I was at a conference all last week, and I will be on holiday with limited internet through the end of next week.

llvm/include/llvm/Support/FileCheck.h

411 ↗

(On Diff #201979)

Comma after "fails"

480 ↗

(On Diff #201979)

in which case

[thopre]

In D60386#1529382, @probinson wrote:

Grammar nits and one longer point, which is:
I understand where the "parenthesis group" term came from, but I think it's not appropriate here. While a CHECK line is implicitly a regex, and variables are a kind of back-reference, the syntax for variable references (use or def) is not parentheses and the content is not a "group" in any real sense, and so "parenthesis group" is an un-obvious and confusing term.
Because FileCheck uses square brackets, I propose that the least disruptive change at this point would be to call them "bracket expressions." The "bracket" part is obvious, and "expression" because (a) definitions will have a regex, and (b) numeric variable references are implicitly expressions. (Okay, still a bit of a stretch, but less so that "parenthesis group" IMO.)
WDYT?

Actually the parenthesis group refers to the regex being generated for a given CHECK line and fed to the regex engine. Let me give an example:

CHECK: foo [[STRVAR:([a-z]+)]] #NUMVAR:

would create the regex "foo (([a-z]+)) ([0-9]+)" and record that the value for STRVAL is captured by the parenthesis group 1 (0 captures what the whole regex matches) and NUMVAR's value is captured by the parenthesis group 3. The Regex::match function takes an array where each entry takes the matched text of the corresponding parenthesis group. The FileCheckNumExprMatch structure records for numeric variable definitions what entries to look for the values in that array so parenthesis group is perfectly adequate (apart from perhaps better naming) IMHO.

Also let me apologize in advance for the review sparsity lately, and going forward. I was at a conference all last week, and I will be on holiday with limited internet through the end of next week.

Sparsity of review is a given when reviewer is from a different company, I adapt my workflow around it. Likewise I try to respond to comments as fast as possible but might not always be able to.

[thopre]

Fix typos

[probinson]

Ah ha. The ParenGroup referring to the CHECK line as it has been translated for consumption by the underlying regex package... that was not clear.
In which case the terminology is okay but the commentary could be better, see inline comment.

llvm/include/llvm/Support/FileCheck.h

350 ↗

(On Diff #201979)

I think what this is trying to say is:
the pattern "foo[[bar:.*]]baz[[bar]]quux[[bar:.*]]"
is translated into some regex with parentheses,
the second set of parentheses corresponds to the last definition of "bar" on the line,
hence VariableDefs will map "bar" to 2.

Is that right? The translation of check-line-with-square-brackets-with-regex into parenthesized-regex-passed-to-regex-library is not obvious in this comment as written.

[thopre]

Clarify mapping from string variable definition to parenthesis group number

[thopre]

llvm/include/llvm/Support/FileCheck.h

350 ↗

(On Diff #201979)

Indeed. The comment was already unclear before this patch series, I think the reason is in most cases the CHECK directive does not contain parenthesis so the square bracket definition maps directly to the parenthesis group number. Hopefully the comment is now clearer.

[probinson]

I am pretty sure all @jhenderson comments have been addressed, and I'm happy, so LGTM.
As I said previously, it may be a while before I get to your next patch.

llvm/include/llvm/Support/FileCheck.h

350 ↗

(On Diff #201979)

It looks odd that the translated expression has nothing for the use of [[bar]] in the original, but the comment is much better now, thanks!

[jhenderson]

Looks good except for one thing: missing unit tests for parseNumericVariableUse?

[thopre]

Fix value of RegExStr in comment about VariableDefs and show case of use of variable defined on earlier line.

llvm/include/llvm/Support/FileCheck.h

350 ↗

(On Diff #201979)

Indeed, it should contain a backreference to the first definition. When it refers to a definition on a different line the value gets inserted in the regex just before match. I've updated the pattern to show both cases, using <quux value> to represent the value captured on a previous line of QUUX.

[thopre]

In D60386#1531043, @jhenderson wrote:

Looks good except for one thing: missing unit tests for parseNumericVariableUse?

This is a private method so I've moved the tests to ParseExpr which tests parseNumericSubstitutionBlock which calls into parseBinop (also private) which calls into parseNumericVariableUse.

[jhenderson]

In D60386#1531177, @thopre wrote:

In D60386#1531043, @jhenderson wrote:

Looks good except for one thing: missing unit tests for parseNumericVariableUse?

This is a private method so I've moved the tests to ParseExpr which tests parseNumericSubstitutionBlock which calls into parseBinop (also private) which calls into parseNumericVariableUse.

Oh, my bad, I checked before making that comment, and thought it was public. Must have misread the code. LGTM.

[hliao]

llvm/trunk/docs/CommandGuide/FileCheck.rst

630–632

Just curious why we have such a restriction. Could someone elaborate more?

That makes numeric checks on the same line must be written in split like this

CHECK: #VAR:...
CHECK-SAME: #VAR + 1

instead of

CHECK: #VAR:... #VAR + 1

That's quite verbose.

[thopre]

llvm/trunk/docs/CommandGuide/FileCheck.rst

630–632

This is not currently done for 2 reasons:

1. The first reason is that the current code assumes the value of numeric expression is known at match time but in the case #VAR: #VAR+1 the value VAR+1 is not known since VAR is not known yet. I have a patch planned to address that later in the patch series [1]
2. The more serious problem is how to deal with input such as 10 12 13 and the CHECK #VAR: {{.*}} #VAR+1 check. Here you'd want the CHECK to be successful and VAR to be assigned the value 12. However FileCheck with the aforementioned patch will ask the regex engine to match the input against the regex "([0-9]+) ([0-9]+)" and will then check the captured text to verify that the numeric expression is verified. However that regex will capture 10 and 12, being the first 2 numbers. Now given the .* how to look at all other possible matches? We would need help from the regex engine but the API only gives us a match/no match answer with captured text.

[1] https://reviews.llvm.org/D60392 which needs serious rebasing

Hope this answers your question.

[hliao]

llvm/trunk/docs/CommandGuide/FileCheck.rst

630–632

1. I wish [1] could be landed as soon as possible. So far, numeric substitution is inconsistent with the previous string substitution. The later allows the use and def in the same line. E.g. the following works ` 11 11 CHECK: [[VAR:[0-9]+]] [[VAR]] `
2. I haven't looked into the numeric support in detail. We should reference the original string block in FileCheck. A similar pattern works for the string block: ` 11 22 22 CHECK: [[VAR:[0-9]+]] [[VAR]] `

Any details on why numeric block works differently from string block?

[hliao]

llvm/trunk/docs/CommandGuide/FileCheck.rst

630–632

Fix the formatting of the examples aforesaid:

11 11
CHECK: [[VAR:[0-9]+]] [[VAR]]

11 22 22
CHECK: [[VAR:[0-9]+]] [[VAR]]

both of them work for string blocks.

[thopre]

llvm/trunk/docs/CommandGuide/FileCheck.rst

630–632

That's because it gets translated into "([0-9]+) \1" for the regex engine, i.e. string substitution can be expressed in terms of regex while numeric substitution cannot always. The current restriction is to only accept cases that can be expressed at the regex level.

The reason I've kept the patch to life part of the restriction last is that I'm not yet convinced it is a good thing to do. Take my example again but as a CHECK-NOT:

CHECK-NOT: #VAR: #VAR+1

This would be accepted after the last patch but if presented with the input "10 12 13" this would *not* give an error because the corresponding regex ("([0-9]+) ([0-9]+)") would match "10 12" which does not verify the +1 relation and thus the CHECK-NOT would be satisfied, despite the "12 13" bits. Given the potential for errors I'm leaning toward discarding the last patch at the moment.

llvm/trunk/docs/CommandGuide/FileCheck.rst

 CHECK-LABEL block. Global variables are not affected by CHECK-LABEL.
 This makes it easier to ensure that individual tests are not affected
 by variables set in preceding tests.
 
 FileCheck Numeric Substitution Blocks
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 :program:`FileCheck` also supports numeric substitution blocks that allow
-checking for numeric values that satisfy a numeric expression constraint based
-on numeric variables. This allows ``CHECK:`` directives to verify a numeric
-relation between two numbers, such as the need for consecutive registers to be
-used.
+defining numeric variables and checking for numeric values that satisfy a
+numeric expression constraint based on those variables via a numeric
+substitution. This allows ``CHECK:`` directives to verify a numeric relation
+between two numbers, such as the need for consecutive registers to be used.
+
+The syntax to define a numeric variable is ``[[#<NUMVAR>:]]`` where
+``<NUMVAR>`` is the name of the numeric variable to define to the matching
+value.
 
-The syntax of a numeric substitution block is ``[[#<NUMVAR><op><offset>]]``
-where:
+For example:
+
+.. code-block:: llvm
+
+    ; CHECK: mov r[[#REG:]], 42
 
-* ``<NUMVAR>`` is the name of a numeric variable defined on the command line.
+would match ``mov r5, 42`` and set ``REG`` to the value ``5``.
+
+The syntax of a numeric substitution is ``[[#<NUMVAR><op><offset>]]`` where:
+
+* ``<NUMVAR>`` is the name of a defined numeric variable.
 
 * ``<op>`` is an optional numeric operation to perform on the value of
   ``<NUMVAR>``. Currently supported numeric operations are ``+`` and ``-``.
 
 * ``<offset>`` is the immediate value that constitutes the second operand of
   the numeric operation <op>. It must be present if ``<op>`` is present,
   absent otherwise.
 
 Spaces are accepted before, after and between any of these elements.
 
-Unlike string substitution blocks, numeric substitution blocks only introduce
-numeric substitutions which substitute a numeric expression for its value.
 For example:
 
 .. code-block:: llvm
 
-    ; CHECK: add r[[#REG]], r[[#REG]], r[[#REG+1]]
+    ; CHECK: load r[[#REG:]], [r0]
+    ; CHECK: load r[[#REG+1]], [r1]
 
-The above example would match the line:
+The above example would match the text:
 
 .. code-block:: gas
 
-    add r5, r5, r6
+    load r5, [r0]
+    load r6, [r1]
 
-but would not match the line:
+but would not match the text:
 
 .. code-block:: gas
 
-    add r5, r5, r7
+    load r5, [r0]
+    load r7, [r1]
 
 due to ``7`` being unequal to ``5 + 1``.
 
 The ``--enable-var-scope`` option has the same effect on numeric variables as
 on string variables.
 
+Important note: In its current implementation, a numeric expression cannot use
+a numeric variable defined on the same line.
+
 FileCheck Pseudo Numeric Variables
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 Sometimes there's a need to verify output that contains line numbers of the
 match file, e.g. when testing compiler diagnostics.  This introduces a certain
 fragility of the match file structure, as "``CHECK:``" lines contain absolute
 line numbers in the same file, which have to be updated whenever line numbers
 change due to text addition or deletion.
 
-To support this case, FileCheck understands the ``@LINE`` pseudo numeric
-variable which evaluates to the line number of the CHECK pattern where it is
-found.
+To support this case, FileCheck numeric expressions understand the ``@LINE``
+pseudo numeric variable which evaluates to the line number of the CHECK pattern
+where it is found.
 
 This way match patterns can be put near the relevant test lines and include
 relative line number references, for example:
 
 .. code-block:: c++
 
    // CHECK: test.cpp:[[# @LINE + 4]]:6: error: expected ';' after top level declarator
    // CHECK-NEXT: {{^int a}}

llvm/trunk/include/llvm/Support/FileCheck.h

struct FileCheckRequest {
   bool VerboseVerbose = false;
 };
 
 //===----------------------------------------------------------------------===//
 // Numeric substitution handling code.
 //===----------------------------------------------------------------------===//
 
 /// Class representing a numeric variable with a given value in a numeric
-/// expression.
+/// expression. Each definition of a variable gets its own instance of this
+/// class. Variable uses share the same instance as their respective
+/// definition.
 class FileCheckNumericVariable {
 private:
   /// Name of the numeric variable.
   StringRef Name;
 
   /// Value of numeric variable, if defined, or None otherwise.
   Optional<uint64_t> Value;
 
+  /// Line number where this variable is defined. Used to determine whether a
+  /// variable is defined on the same line as a given use.
+  size_t DefLineNumber;
+
 public:
+  /// Constructor for a variable \p Name defined at line \p DefLineNumber.
+  FileCheckNumericVariable(size_t DefLineNumber, StringRef Name)
+      : Name(Name), DefLineNumber(DefLineNumber) {}
+
   /// Constructor for numeric variable \p Name with a known \p Value at parse
   /// time (e.g. the @LINE numeric variable).
   FileCheckNumericVariable(StringRef Name, uint64_t Value)
-      : Name(Name), Value(Value) {}
+      : Name(Name), Value(Value), DefLineNumber(0) {}
 
   /// \returns name of that numeric variable.
   StringRef getName() const { return Name; }
 
   /// \returns value of this numeric variable.
   Optional<uint64_t> getValue() const { return Value; }
 
   /// Sets value of this numeric variable if not defined. \returns whether the
   /// variable was already defined.
   bool setValue(uint64_t Value);
 
   /// Clears value of this numeric variable. \returns whether the variable was
   /// already undefined.
   bool clearValue();
+
+  /// \returns the line number where this variable is defined.
+  size_t getDefLineNumber() { return DefLineNumber; }
 };
 
 /// Type of functions evaluating a given binary operation.
 using binop_eval_t = uint64_t (*)(uint64_t, uint64_t);
 
 /// Class representing a numeric expression consisting of either a single
 /// numeric variable or a binary operation between a numeric variable and an
 /// immediate.
private:
 
   /// Map of all string variables defined so far. Used at parse time to detect
   /// a name conflict between a numeric variable and a string variable when
   /// the former is defined on a later line than the latter.
   StringMap<bool> DefinedVariableTable;
 
   /// When matching a given pattern, this holds the pointers to the classes
   /// representing the last definitions of numeric variables defined in
-  /// previous patterns. Earlier definition of the variables, if any, have
-  /// their own class instance not referenced by this table.
+  /// previous patterns. Earlier definitions of the variables, if any, have
+  /// their own class instance not referenced by this table. When matching a
+  /// pattern all definitions for that pattern are recorded in the
+  /// NumericVariableDefs table in the FileCheckPattern instance of that
+  /// pattern.
   StringMap<FileCheckNumericVariable *> GlobalNumericVariableTable;
 
   /// Vector holding pointers to all parsed numeric expressions. Used to
   /// automatically free the numeric expressions once they are guaranteed to no
   /// longer be used.
   std::vector<std::unique_ptr<FileCheckNumExpr>> NumExprs;
 
   /// Vector holding pointers to all parsed numeric variables. Used to
private:
   /// Makes a new numeric expression instance and registers it for destruction
   /// when the context is destroyed.
   FileCheckNumExpr *makeNumExpr(binop_eval_t EvalBinop,
                                 FileCheckNumericVariable *OperandLeft,
                                 uint64_t OperandRight);
 
   /// Makes a new numeric variable and registers it for destruction when the
   /// context is destroyed.
-  FileCheckNumericVariable *makeNumericVariable(StringRef Name, uint64_t Value);
+  template <class... Types>
+  FileCheckNumericVariable *makeNumericVariable(Types... args);
 
   /// Makes a new string substitution and registers it for destruction when the
   /// context is destroyed.
   FileCheckSubstitution *makeStringSubstitution(StringRef VarName,
                                                 size_t InsertIdx);
 
   /// Makes a new numeric substitution and registers it for destruction when
   /// the context is destroyed.
class FileCheckPattern {
   /// Entries in this vector represent a substitution of a string variable or a
   /// numeric expression in the RegExStr regex at match time. For example, in
   /// the case of a CHECK directive with the pattern "foo[[bar]]baz[[#N+1]]",
   /// RegExStr will contain "foobaz" and we'll get two entries in this vector
   /// that tells us to insert the value of string variable "bar" at offset 3
   /// and the value of numeric expression "N+1" at offset 6.
   std::vector<FileCheckSubstitution *> Substitutions;
 
-  /// Maps names of string variables defined in a pattern to the parenthesized
-  /// capture numbers of their last definition.
+  /// Maps names of string variables defined in a pattern to the number of
+  /// their parenthesis group in RegExStr capturing their last definition.
   ///
-  /// E.g. for the pattern "foo[[bar:.*]]baz[[bar]]quux[[bar:.*]]",
-  /// VariableDefs will map "bar" to 2 corresponding to the second definition
-  /// of "bar".
+  /// E.g. for the pattern "foo[[bar:.*]]baz([[bar]][[QUUX]][[bar:.*]])",
+  /// RegExStr will be "foo(.*)baz(\1<quux value>(.*))" where <quux value> is
+  /// the value captured for QUUX on the earlier line where it was defined, and
+  /// VariableDefs will map "bar" to the third parenthesis group which captures
+  /// the second definition of "bar".
   ///
   /// Note: uses std::map rather than StringMap to be able to get the key when
   /// iterating over values.
   std::map<StringRef, unsigned> VariableDefs;
 
+  /// Structure representing the definition of a numeric variable in a pattern.
+  /// It holds the pointer to the class representing the numeric variable whose
+  /// value is being defined and the number of the parenthesis group in
+  /// RegExStr to capture that value.
+  struct FileCheckNumExprMatch {
+    /// Pointer to class representing the numeric variable whose value is being
+    /// defined.
+    FileCheckNumericVariable *DefinedNumericVariable;
+
+    /// Number of the parenthesis group in RegExStr that captures the value of
+    /// this numeric variable definition.
+    unsigned CaptureParenGroup;
+  };
+
+  /// Holds the number of the parenthesis group in RegExStr and pointer to the
+  /// corresponding FileCheckNumericVariable class instance of all numeric
+  /// variable definitions. Used to set the matched value of all those
+  /// variables.
+  StringMap<FileCheckNumExprMatch> NumericVariableDefs;
+
   /// Pointer to a class instance holding the global state shared by all
   /// patterns:
   /// - separate tables with the values of live string and numeric variables
   ///   respectively at the start of any given CHECK line;
   /// - table holding whether a string variable has been defined at any given
   ///   point during the parsing phase.
   FileCheckPatternContext *Context;
 
   Check::FileCheckType CheckTy;
 
-  /// Contains the number of line this pattern is in.
-  unsigned LineNumber;
+  /// Line number for this CHECK pattern. Used to determine whether a variable
+  /// definition is made on an earlier line to the one with this CHECK.
+  size_t LineNumber;
 
 public:
-  explicit FileCheckPattern(Check::FileCheckType Ty,
-                            FileCheckPatternContext *Context)
-      : Context(Context), CheckTy(Ty) {}
+  FileCheckPattern(Check::FileCheckType Ty, FileCheckPatternContext *Context,
+                   size_t Line)
+      : Context(Context), CheckTy(Ty), LineNumber(Line) {}
 
   /// \returns the location in source code.
   SMLoc getLoc() const { return PatternLoc; }
 
   /// \returns the pointer to the global state for all patterns in this
   /// FileCheck instance.
   FileCheckPatternContext *getContext() const { return Context; }
 
   /// \returns whether \p C is a valid first character for a variable name.
   static bool isValidVarNameStart(char C);
-  /// Verifies that the string at the start of \p Str is a well formed
-  /// variable. \returns false if it is and sets \p IsPseudo to indicate if it
-  /// is a pseudo variable and \p TrailIdx to the position of the last
-  /// character that is part of the variable name. Otherwise, only
-  /// \returns true.
-  static bool parseVariable(StringRef Str, bool &IsPseudo, unsigned &TrailIdx);
-  /// Parses a numeric substitution involving (pseudo if \p IsPseudo is true)
-  /// variable \p Name with the string corresponding to the operation being
-  /// performed in \p Trailer. \returns the class representing the numeric
-  /// expression being substituted or nullptr if parsing fails, in which case
-  /// errors are reported on \p SM.
-  FileCheckNumExpr *parseNumericSubstitution(StringRef Name, bool IsPseudo,
-                                             StringRef Trailer,
-                                             const SourceMgr &SM) const;
+  /// Parses the string at the start of \p Str for a variable name and \returns
+  /// whether the variable name is ill-formed. If parsing succeeded, sets
+  /// \p IsPseudo to indicate if it is a pseudo variable, sets \p Name to the
+  /// parsed variable name and strips \p Str from the variable name.
+  static bool parseVariable(StringRef &Str, StringRef &Name, bool &IsPseudo);
+  /// Parses \p Expr for the definition of a numeric variable, returning an
+  /// error if \p Context already holds a string variable with the same name.
+  /// \returns whether parsing fails, in which case errors are reported on
+  /// \p SM. Otherwise, sets \p Name to the name of the parsed numeric
+  /// variable.
+  static bool parseNumericVariableDefinition(StringRef &Expr, StringRef &Name,
+                                             FileCheckPatternContext *Context,
+                                             const SourceMgr &SM);
+  /// Parses \p Expr for a numeric substitution block. \returns the class
+  /// representing the AST of the numeric expression whose value must be
+  /// substituted, or nullptr if parsing fails, in which case errors are
+  /// reported on \p SM. Sets \p DefinedNumericVariable to point to the class
+  /// representing the numeric variable defined in this numeric substitution
+  /// block, or nullptr if this block does not define any variable.
+  FileCheckNumExpr *parseNumericSubstitutionBlock(
+      StringRef Expr, FileCheckNumericVariable *&DefinedNumericVariable,
   /// Parses the pattern in \p PatternStr and initializes this FileCheckPattern
   /// instance 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,
-  /// and \p LineNumber is the line number in the input file from which the
-  /// pattern string was read. \returns true in case of an error, false
-  /// otherwise.
-  bool ParsePattern(StringRef PatternStr, StringRef Prefix, SourceMgr &SM,
-                    unsigned LineNumber, const FileCheckRequest &Req);
+  /// 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
   ///
   /// \returns the position that is matched or npos if there is no match. If
   /// there is a match, updates \p MatchLen with the size of the matched
   /// string.
   ///
   /// 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.
-  size_t match(StringRef Buffer, size_t &MatchLen) const;
+  /// 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.
+  size_t match(StringRef Buffer, size_t &MatchLen, const SourceMgr &SM) const;
   /// Prints the value of successful substitutions or the name of the undefined
   /// string or numeric variable preventing a successful substitution.
   void printSubstitutions(const SourceMgr &SM, StringRef Buffer,
                           SMRange MatchRange = None) const;
   void printFuzzyMatch(const SourceMgr &SM, StringRef Buffer,
                        std::vector<FileCheckDiag> *Diags) const;
 
   bool hasVariable() const {
private:
   unsigned computeMatchDistance(StringRef Buffer) const;
   /// Finds the closing sequence of a regex variable usage or definition.
   ///
   /// \p Str has to point in the beginning of the definition (right after the
   /// opening sequence). \p SM holds the SourceMgr used for error repporting.
   ///  \returns the offset of the closing sequence within Str, or npos if it
   /// was not found.
   size_t FindRegexVarEnd(StringRef Str, SourceMgr &SM);
+
+  /// Parses \p Expr for the use of a numeric variable. \returns the pointer to
+  /// the class instance representing that variable if successful, or nullptr
+  /// otherwise, in which case errors are reported on \p SM.
+  FileCheckNumericVariable *parseNumericVariableUse(StringRef &Expr,
+                                                    const SourceMgr &SM) const;
+  /// Parses \p Expr for a binary operation.
+  /// \returns the class representing the binary operation of the numeric
+  /// expression, or nullptr if parsing fails, in which case errors are
+  /// reported on \p SM.
+  FileCheckNumExpr *parseBinop(StringRef &Expr, const SourceMgr &SM) const;
 };
 
 //===----------------------------------------------------------------------===//
 /// Summary of a FileCheck diagnostic.
 //===----------------------------------------------------------------------===//
 
 struct FileCheckDiag {
   /// What is the FileCheck directive for this diagnostic?

llvm/trunk/lib/Support/FileCheck.cpp

 bool FileCheckNumericVariable::clearValue() {
   if (!Value)
     return true;
   Value = None;
   return false;
 }
 
 Optional<uint64_t> FileCheckNumExpr::eval() const {
-  Optional<uint64_t> LeftOp = this->LeftOp->getValue();
+  assert(LeftOp && "Evaluating an empty numeric expression");
+  Optional<uint64_t> LeftOpValue = LeftOp->getValue();
   // Variable is undefined.
-  if (!LeftOp)
+  if (!LeftOpValue)
     return None;
-  return EvalBinop(*LeftOp, RightOp);
+  return EvalBinop(*LeftOpValue, RightOp);
 }
 
 StringRef FileCheckNumExpr::getUndefVarName() const {
   if (!LeftOp->getValue())
     return LeftOp->getName();
   return StringRef();
 }
 
StringRef FileCheckStringSubstitution::getUndefVarName() const {
 
   return StringRef();
 }
 
 bool FileCheckPattern::isValidVarNameStart(char C) {
   return C == '_' || isalpha(C);
 }
 
-bool FileCheckPattern::parseVariable(StringRef Str, bool &IsPseudo,
-                                     unsigned &TrailIdx) {
+bool FileCheckPattern::parseVariable(StringRef &Str, StringRef &Name,
+                                     bool &IsPseudo) {
   if (Str.empty())
     return true;
 
   bool ParsedOneChar = false;
   unsigned I = 0;
   IsPseudo = Str[0] == '@';
 
   // Global vars start with '$'.
   if (Str[0] == '$' || IsPseudo)
     ++I;
 
   for (unsigned E = Str.size(); I != E; ++I) {
     if (!ParsedOneChar && !isValidVarNameStart(Str[I]))
       return true;
 
     // Variable names are composed of alphanumeric characters and underscores.
     if (Str[I] != '_' && !isalnum(Str[I]))
       break;
     ParsedOneChar = true;
   }
 
-  TrailIdx = I;
+  Name = Str.take_front(I);
+  Str = Str.substr(I);
   return false;
 }
 
 // StringRef holding all characters considered as horizontal whitespaces by
 // FileCheck input canonicalization.
 StringRef SpaceChars = " \t";
 
 // Parsing helper function that strips the first character in S and returns it.
 static char popFront(StringRef &S) {
   char C = S.front();
   S = S.drop_front();
   return C;
 }
 
-static uint64_t add(uint64_t LeftOp, uint64_t RightOp) {
-  return LeftOp + RightOp;
+bool FileCheckPattern::parseNumericVariableDefinition(
+    StringRef &Expr, StringRef &Name, FileCheckPatternContext *Context,
+    const SourceMgr &SM) {
+  bool IsPseudo;
+  if (parseVariable(Expr, Name, IsPseudo)) {
+    SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
+                    "invalid variable name");
+    return true;
   }
-static uint64_t sub(uint64_t LeftOp, uint64_t RightOp) {
-  return LeftOp - RightOp;
+
+  if (IsPseudo) {
+    SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
+                    "definition of pseudo numeric variable unsupported");
+    return true;
   }
 
-FileCheckNumExpr *
-FileCheckPattern::parseNumericSubstitution(StringRef Name, bool IsPseudo,
-                                           StringRef Trailer,
-                                           const SourceMgr &SM) const {
+  // Detect collisions between string and numeric variables when the latter
+  // is created later than the former.
+  if (Context->DefinedVariableTable.find(Name) !=
+      Context->DefinedVariableTable.end()) {
+    SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
+                    "string variable with name '" + Name + "' already exists");
+    return true;
+  }
+
+  return false;
+}
+
+FileCheckNumericVariable *
+FileCheckPattern::parseNumericVariableUse(StringRef &Expr,
+  bool IsPseudo;
+  StringRef Name;
+  if (parseVariable(Expr, Name, IsPseudo)) {
+    SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
+                    "invalid variable name");
+    return nullptr;
+  }
+
   if (IsPseudo && !Name.equals("@LINE")) {
     SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
                     "invalid pseudo numeric variable '" + Name + "'");
     return nullptr;
   }
 
-  // This method is indirectly called from ParsePattern for all numeric
+  // This method is indirectly called from parsePattern for all numeric
   // variable definitions and uses in the order in which they appear in the
   // CHECK pattern. For each definition, the pointer to the class instance of
   // the corresponding numeric variable definition is stored in
   // GlobalNumericVariableTable. Therefore, the pointer we get below is for the
   // class instance corresponding to the last definition of this variable use.
   auto VarTableIter = Context->GlobalNumericVariableTable.find(Name);
   if (VarTableIter == Context->GlobalNumericVariableTable.end()) {
     SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
                     "using undefined numeric variable '" + Name + "'");
     return nullptr;
   }
 
-  FileCheckNumericVariable *LeftOp = VarTableIter->second;
+  FileCheckNumericVariable *NumericVariable = VarTableIter->second;
+  if (!IsPseudo && NumericVariable->getDefLineNumber() == LineNumber) {
+    SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
+                    "numeric variable '" + Name +
+                        "' defined on the same line as used");
+    return nullptr;
+  }
+
+  return NumericVariable;
+}
+
+static uint64_t add(uint64_t LeftOp, uint64_t RightOp) {
+  return LeftOp + RightOp;
+}
+
+static uint64_t sub(uint64_t LeftOp, uint64_t RightOp) {
+  return LeftOp - RightOp;
+}
+
+FileCheckNumExpr *FileCheckPattern::parseBinop(StringRef &Expr,
+                                               const SourceMgr &SM) const {
+  FileCheckNumericVariable *LeftOp = parseNumericVariableUse(Expr, SM);
+  if (!LeftOp) {
+    // Error reporting done in parseNumericVariableUse().
+    return nullptr;
+  }
 
   // Check if this is a supported operation and select a function to perform
   // it.
-  Trailer = Trailer.ltrim(SpaceChars);
-  if (Trailer.empty()) {
+  Expr = Expr.ltrim(SpaceChars);
+  if (Expr.empty())
     return Context->makeNumExpr(add, LeftOp, 0);
-  }
-  SMLoc OpLoc = SMLoc::getFromPointer(Trailer.data());
-  char Operator = popFront(Trailer);
+  SMLoc OpLoc = SMLoc::getFromPointer(Expr.data());
+  char Operator = popFront(Expr);
   binop_eval_t EvalBinop;
   switch (Operator) {
   case '+':
     EvalBinop = add;
     break;
   case '-':
     EvalBinop = sub;
     break;
   default:
     SM.PrintMessage(OpLoc, SourceMgr::DK_Error,
                     Twine("unsupported numeric operation '") + Twine(Operator) +
                         "'");
     return nullptr;
   }
 
   // Parse right operand.
-  Trailer = Trailer.ltrim(SpaceChars);
-  if (Trailer.empty()) {
-    SM.PrintMessage(SMLoc::getFromPointer(Trailer.data()), SourceMgr::DK_Error,
+  Expr = Expr.ltrim(SpaceChars);
+  if (Expr.empty()) {
+    SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
                     "missing operand in numeric expression");
     return nullptr;
   }
   uint64_t RightOp;
-  if (Trailer.consumeInteger(10, RightOp)) {
-    SM.PrintMessage(SMLoc::getFromPointer(Trailer.data()), SourceMgr::DK_Error,
-                    "invalid offset in numeric expression '" + Trailer + "'");
+  if (Expr.consumeInteger(10, RightOp)) {
+    SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
+                    "invalid offset in numeric expression '" + Expr + "'");
     return nullptr;
   }
-  Trailer = Trailer.ltrim(SpaceChars);
-  if (!Trailer.empty()) {
-    SM.PrintMessage(SMLoc::getFromPointer(Trailer.data()), SourceMgr::DK_Error,
+  Expr = Expr.ltrim(SpaceChars);
+  if (!Expr.empty()) {
+    SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
                     "unexpected characters at end of numeric expression '" +
-                        Trailer + "'");
+                        Expr + "'");
     return nullptr;
   }
 
   return Context->makeNumExpr(EvalBinop, LeftOp, RightOp);
 }
 
-bool FileCheckPattern::ParsePattern(StringRef PatternStr, StringRef Prefix,
-                                    SourceMgr &SM, unsigned LineNumber,
+FileCheckNumExpr *FileCheckPattern::parseNumericSubstitutionBlock(
+    StringRef Expr, FileCheckNumericVariable *&DefinedNumericVariable,
+    const SourceMgr &SM) const {
+  // Parse the numeric variable definition.
+  DefinedNumericVariable = nullptr;
+  size_t DefEnd = Expr.find(':');
+  if (DefEnd != StringRef::npos) {
+    StringRef DefExpr = Expr.substr(0, DefEnd);
+    StringRef UseExpr = Expr = Expr.substr(DefEnd + 1);
+
+    DefExpr = DefExpr.ltrim(SpaceChars);
+    StringRef Name;
+    if (parseNumericVariableDefinition(DefExpr, Name, Context, SM)) {
+      // Invalid variable definition. Error reporting done in parsing function.
+      return nullptr;
+    }
+
+    DefinedNumericVariable =
+        Context->makeNumericVariable(this->LineNumber, Name);
+
+    DefExpr = DefExpr.ltrim(SpaceChars);
+    if (!DefExpr.empty()) {
+      SM.PrintMessage(SMLoc::getFromPointer(DefExpr.data()),
+                      SourceMgr::DK_Error,
+                      "invalid numeric variable definition");
+      return nullptr;
+    }
+    UseExpr = UseExpr.ltrim(SpaceChars);
+    if (!UseExpr.empty()) {
+      SM.PrintMessage(
+          SMLoc::getFromPointer(UseExpr.data()), SourceMgr::DK_Error,
+          "unexpected string after variable definition: '" + UseExpr + "'");
+      return nullptr;
+    }
+    return Context->makeNumExpr(add, nullptr, 0);
+  }
+
+  // Parse the numeric expression itself.
+  Expr = Expr.ltrim(SpaceChars);
+  return parseBinop(Expr, SM);
+}
+
+bool FileCheckPattern::parsePattern(StringRef PatternStr, StringRef Prefix,
+                                    SourceMgr &SM,
                                     const FileCheckRequest &Req) {
   bool MatchFullLinesHere = Req.MatchFullLines && CheckTy != Check::CheckNot;
 
-  this->LineNumber = LineNumber;
   PatternLoc = SMLoc::getFromPointer(PatternStr.data());
 
   // Create fake @LINE pseudo variable definition.
   StringRef LinePseudo = "@LINE";
   uint64_t LineNumber64 = LineNumber;
   FileCheckNumericVariable *LinePseudoVar =
       Context->makeNumericVariable(LinePseudo, LineNumber64);
   Context->GlobalNumericVariableTable[LinePseudo] = LinePseudoVar;
if (PatternStr.startswith("{{")) {
 
       PatternStr = PatternStr.substr(End + 2);
       continue;
     }
 
     // String and numeric substitution blocks. String substitution blocks come
     // in two forms: [[foo:.*]] and [[foo]]. The former matches .* (or some
     // other regex) and assigns it to the string variable 'foo'. The latter
-    // substitutes foo's value. Numeric substitution blocks start with a
-    // '#' sign after the double brackets and only have the substitution form.
-    // Both string and numeric variables must satisfy the regular expression
-    // "[a-zA-Z_][0-9a-zA-Z_]*" to be valid, as this helps catch some common
-    // errors.
+    // substitutes foo's value. Numeric substitution blocks work the same way
+    // as string ones, but start with a '#' sign after the double brackets.
+    // Both string and numeric variable names must satisfy the regular
+    // expression "[a-zA-Z_][0-9a-zA-Z_]*" to be valid, as this helps catch
+    // some common errors.
     if (PatternStr.startswith("[[")) {
       StringRef UnparsedPatternStr = PatternStr.substr(2);
       // Find the closing bracket pair ending the match.  End is going to be an
       // offset relative to the beginning of the match string.
       size_t End = FindRegexVarEnd(UnparsedPatternStr, SM);
       StringRef MatchStr = UnparsedPatternStr.substr(0, End);
       bool IsNumBlock = MatchStr.consume_front("#");
 
       if (End == StringRef::npos) {
         SM.PrintMessage(SMLoc::getFromPointer(PatternStr.data()),
                         SourceMgr::DK_Error,
                         "Invalid substitution block, no ]] found");
         return true;
       }
       // Strip the substitution block we are parsing. End points to the start
       // of the "]]" closing the expression so account for it in computing the
       // index of the first unparsed character.
       PatternStr = UnparsedPatternStr.substr(End + 2);
 
+      bool IsDefinition = false;
+      StringRef DefName;
+      StringRef SubstStr;
+      StringRef MatchRegexp;
+      size_t SubstInsertIdx = RegExStr.size();
+
+      // Parse string variable or legacy numeric expression.
+      if (!IsNumBlock) {
         size_t VarEndIdx = MatchStr.find(":");
-      if (IsNumBlock)
-        MatchStr = MatchStr.ltrim(SpaceChars);
-      else {
         size_t SpacePos = MatchStr.substr(0, VarEndIdx).find_first_of(" \t");
         if (SpacePos != StringRef::npos) {
           SM.PrintMessage(SMLoc::getFromPointer(MatchStr.data() + SpacePos),
                           SourceMgr::DK_Error, "unexpected whitespace");
           return true;
         }
-      }
 
-      // Get the variable name (e.g. "foo") and verify it is well formed.
+        // Get the name (e.g. "foo") and verify it is well formed.
         bool IsPseudo;
-      unsigned TrailIdx;
-      if (parseVariable(MatchStr, IsPseudo, TrailIdx)) {
+        StringRef Name;
+        StringRef OrigMatchStr = MatchStr;
+        if (parseVariable(MatchStr, Name, IsPseudo)) {
           SM.PrintMessage(SMLoc::getFromPointer(MatchStr.data()),
                           SourceMgr::DK_Error, "invalid variable name");
           return true;
         }
 
-      size_t SubstInsertIdx = RegExStr.size();
-      FileCheckNumExpr *NumExpr;
-
-      StringRef Name = MatchStr.substr(0, TrailIdx);
-      StringRef Trailer = MatchStr.substr(TrailIdx);
-      bool IsVarDef = (VarEndIdx != StringRef::npos);
-
-      if (IsVarDef) {
-        if (IsPseudo || !Trailer.consume_front(":")) {
-          SM.PrintMessage(SMLoc::getFromPointer(MatchStr.data()),
+        IsDefinition = (VarEndIdx != StringRef::npos);
+        if (IsDefinition) {
+          if ((IsPseudo || !MatchStr.consume_front(":"))) {
+            SM.PrintMessage(SMLoc::getFromPointer(Name.data()),
                             SourceMgr::DK_Error,
                             "invalid name in string variable definition");
             return true;
           }
 
           // Detect collisions between string and numeric variables when the
           // former is created later than the latter.
           if (Context->GlobalNumericVariableTable.find(Name) !=
               Context->GlobalNumericVariableTable.end()) {
             SM.PrintMessage(
-              SMLoc::getFromPointer(MatchStr.data()), SourceMgr::DK_Error,
+                SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
                 "numeric variable with name '" + Name + "' already exists");
             return true;
           }
+          DefName = Name;
+          MatchRegexp = MatchStr;
+        } else {
+          if (IsPseudo) {
+            MatchStr = OrigMatchStr;
+            IsNumBlock = true;
+          } else
+            SubstStr = Name;
+        }
       }
 
-      if (IsNumBlock || (!IsVarDef && IsPseudo)) {
-        NumExpr = parseNumericSubstitution(Name, IsPseudo, Trailer, SM);
+      // Parse numeric substitution block.
+      FileCheckNumExpr *NumExpr;
+      FileCheckNumericVariable *DefinedNumericVariable;
+      if (IsNumBlock) {
+        NumExpr =
+            parseNumericSubstitutionBlock(MatchStr, DefinedNumericVariable, SM);
         if (NumExpr == nullptr)
           return true;
-        IsNumBlock = true;
+        if (DefinedNumericVariable) {
+          IsDefinition = true;
+          DefName = DefinedNumericVariable->getName();
+          MatchRegexp = StringRef("[0-9]+");
+        } else
+          SubstStr = MatchStr;
       }
 
       // Handle substitutions: [[foo]] and [[#<foo expr>]].
-      if (!IsVarDef) {
+      if (!IsDefinition) {
         // Handle substitution of string variables that were defined earlier on
-        // the same line by emitting a backreference.
-        if (!IsNumBlock && VariableDefs.find(Name) != VariableDefs.end()) {
-          unsigned CaptureParen = VariableDefs[Name];
-          if (CaptureParen < 1 || CaptureParen > 9) {
-            SM.PrintMessage(SMLoc::getFromPointer(Name.data()),
+        // the same line by emitting a backreference. Numeric expressions do
+        // not support substituting a numeric variable defined on the same
+        // line.
+        if (!IsNumBlock && VariableDefs.find(SubstStr) != VariableDefs.end()) {
+          unsigned CaptureParenGroup = VariableDefs[SubstStr];
+          if (CaptureParenGroup < 1 || CaptureParenGroup > 9) {
+            SM.PrintMessage(SMLoc::getFromPointer(SubstStr.data()),
                             SourceMgr::DK_Error,
                             "Can't back-reference more than 9 variables");
             return true;
           }
-          AddBackrefToRegEx(CaptureParen);
+          AddBackrefToRegEx(CaptureParenGroup);
         } else {
           // Handle substitution of string variables ([[<var>]]) defined in
           // previous CHECK patterns, and substitution of numeric expressions.
           FileCheckSubstitution *Substitution =
               IsNumBlock
-                  ? Context->makeNumericSubstitution(MatchStr, NumExpr,
+                  ? Context->makeNumericSubstitution(SubstStr, NumExpr,
                                                      SubstInsertIdx)
-                  : Context->makeStringSubstitution(MatchStr, SubstInsertIdx);
+                  : Context->makeStringSubstitution(SubstStr, SubstInsertIdx);
           Substitutions.push_back(Substitution);
         }
         continue;
       }
 
-      // Handle variable definitions: [[foo:.*]].
-      VariableDefs[Name] = CurParen;
+      // Handle variable definitions: [[<def>:(...)]] and
+      // [[#(...)<def>:(...)]].
+      if (IsNumBlock) {
+        FileCheckNumExprMatch NumExprDef = {DefinedNumericVariable, CurParen};
+        NumericVariableDefs[DefName] = NumExprDef;
+        // This store is done here rather than in match() to allow
+        // parseNumericVariableUse() to get the pointer to the class instance
+        // of the right variable definition corresponding to a given numeric
+        // variable use.
+        Context->GlobalNumericVariableTable[DefName] = DefinedNumericVariable;
+      } else {
+        VariableDefs[DefName] = CurParen;
+        // Mark the string variable as defined to detect collisions between
+        // string and numeric variables in parseNumericVariableUse() and
+        // DefineCmdlineVariables() when the latter is created later than the
+        // former. We cannot reuse GlobalVariableTable for this by populating
+        // it with an empty string since we would then lose the ability to
+        // detect the use of an undefined variable in match().
+        Context->DefinedVariableTable[DefName] = true;
+      }
       RegExStr += '(';
       ++CurParen;
 
-      if (AddRegExToRegEx(Trailer, CurParen, SM))
+      if (AddRegExToRegEx(MatchRegexp, CurParen, SM))
         return true;
 
       RegExStr += ')';
     }
 
     // Handle fixed string matches.
     // Find the end, which is the start of the next regex.
     size_t FixedMatchEnd = PatternStr.find("{{");

 }
 
 void FileCheckPattern::AddBackrefToRegEx(unsigned BackrefNum) {
   assert(BackrefNum >= 1 && BackrefNum <= 9 && "Invalid backref number");
   std::string Backref = std::string("\\") + std::string(1, '0' + BackrefNum);
   RegExStr += Backref;
 }
 
-size_t FileCheckPattern::match(StringRef Buffer, size_t &MatchLen) const {
+size_t FileCheckPattern::match(StringRef Buffer, size_t &MatchLen,
+                               const SourceMgr &SM) const {
   // If this is the EOF pattern, match it immediately.
   if (CheckTy == Check::CheckEOF) {
     MatchLen = 0;
     return Buffer.size();
   }
 
   // If this is a fixed string pattern, just match it now.
   if (!FixedStr.empty()) {
size_t FileCheckPattern::match(StringRef Buffer, size_t &MatchLen,
 
   // 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];
   }
 
+  // If this defines any numeric variables, remember their values.
+  for (const auto &NumericVariableDef : NumericVariableDefs) {
+    const FileCheckNumExprMatch &NumericVariableMatch =
+        NumericVariableDef.getValue();
+    unsigned CaptureParenGroup = NumericVariableMatch.CaptureParenGroup;
+    assert(CaptureParenGroup < MatchInfo.size() && "Internal paren error");
+    FileCheckNumericVariable *DefinedNumericVariable =
+        NumericVariableMatch.DefinedNumericVariable;
+
+    StringRef MatchedValue = MatchInfo[CaptureParenGroup];
+    uint64_t Val;
+    if (MatchedValue.getAsInteger(10, Val)) {
+      SM.PrintMessage(SMLoc::getFromPointer(MatchedValue.data()),
+                      SourceMgr::DK_Error, "Unable to represent numeric value");
+    }
+    if (DefinedNumericVariable->setValue(Val))
+      assert(false && "Numeric variable redefined");
+  }
+
   // 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;
 }
 

 FileCheckPatternContext::makeNumExpr(binop_eval_t EvalBinop,
                                      FileCheckNumericVariable *OperandLeft,
                                      uint64_t OperandRight) {
   NumExprs.push_back(llvm::make_unique<FileCheckNumExpr>(EvalBinop, OperandLeft,
                                                          OperandRight));
   return NumExprs.back().get();
 }
 
+template <class... Types>
 FileCheckNumericVariable *
-FileCheckPatternContext::makeNumericVariable(StringRef Name, uint64_t Value) {
+FileCheckPatternContext::makeNumericVariable(Types... args) {
   NumericVariables.push_back(
-      llvm::make_unique<FileCheckNumericVariable>(Name, Value));
+      llvm::make_unique<FileCheckNumericVariable>(args...));
   return NumericVariables.back().get();
 }
 
 FileCheckSubstitution *
 FileCheckPatternContext::makeStringSubstitution(StringRef VarName,
                                                 size_t InsertIdx) {
   Substitutions.push_back(
       llvm::make_unique<FileCheckStringSubstitution>(this, VarName, InsertIdx));
for (const auto &PatternString : Req.ImplicitCheckNot) {
     std::unique_ptr<MemoryBuffer> CmdLine = MemoryBuffer::getMemBufferCopy(
         Prefix + PatternString + Suffix, "command line");
 
     StringRef PatternInBuffer =
         CmdLine->getBuffer().substr(Prefix.size(), PatternString.size());
     SM.AddNewSourceBuffer(std::move(CmdLine), SMLoc());
 
     ImplicitNegativeChecks.push_back(
-        FileCheckPattern(Check::CheckNot, &PatternContext));
-    ImplicitNegativeChecks.back().ParsePattern(PatternInBuffer,
-                                               "IMPLICIT-CHECK", SM, 0, Req);
+        FileCheckPattern(Check::CheckNot, &PatternContext, 0));
+    ImplicitNegativeChecks.back().parsePattern(PatternInBuffer,
+                                               "IMPLICIT-CHECK", SM, Req);
   }
 
   std::vector<FileCheckPattern> DagNotMatches = ImplicitNegativeChecks;
 
   // LineNumber keeps track of the line on which CheckPrefix instances are
   // found.
   unsigned LineNumber = 1;
 
while (1) {
 
     // Scan ahead to the end of line.
     size_t EOL = Buffer.find_first_of("\n\r");
 
     // Remember the location of the start of the pattern, for diagnostics.
     SMLoc PatternLoc = SMLoc::getFromPointer(Buffer.data());
 
     // Parse the pattern.
-    FileCheckPattern P(CheckTy, &PatternContext);
-    if (P.ParsePattern(Buffer.substr(0, EOL), UsedPrefix, SM, LineNumber, Req))
+    FileCheckPattern P(CheckTy, &PatternContext, LineNumber);
+    if (P.parsePattern(Buffer.substr(0, EOL), UsedPrefix, SM, Req))
       return true;
 
     // Verify that CHECK-LABEL lines do not define or use variables
     if ((CheckTy == Check::CheckLabel) && P.hasVariable()) {
       SM.PrintMessage(
           SMLoc::getFromPointer(UsedPrefixStart), SourceMgr::DK_Error,
           "found '" + UsedPrefix + "-LABEL:'"
                                    " with variable definition or use");
while (1) {
     std::swap(DagNotMatches, CheckStrings.back().DagNotStrings);
     DagNotMatches = ImplicitNegativeChecks;
   }
 
   // Add an EOF pattern for any trailing CHECK-DAG/-NOTs, and use the first
   // prefix as a filler for the error message.
   if (!DagNotMatches.empty()) {
     CheckStrings.emplace_back(
-        FileCheckPattern(Check::CheckEOF, &PatternContext),
+        FileCheckPattern(Check::CheckEOF, &PatternContext, LineNumber + 1),
         *Req.CheckPrefixes.begin(), SMLoc::getFromPointer(Buffer.data()));
     std::swap(DagNotMatches, CheckStrings.back().DagNotStrings);
   }
 
   if (CheckStrings.empty()) {
     errs() << "error: no check strings found with prefix"
            << (Req.CheckPrefixes.size() > 1 ? "es " : " ");
     auto I = Req.CheckPrefixes.begin();
size_t FileCheckString::Check(const SourceMgr &SM, StringRef Buffer,
   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
-    size_t MatchPos = Pat.match(MatchBuffer, CurrentMatchLen);
+    size_t MatchPos = Pat.match(MatchBuffer, CurrentMatchLen, SM);
     if (i == 1)
       FirstMatchPos = LastPos + MatchPos;
 
     // report
     if (MatchPos == StringRef::npos) {
       PrintNoMatch(true, SM, *this, i, MatchBuffer, Req.VerboseVerbose, Diags);
       return StringRef::npos;
     }

 bool FileCheckString::CheckNot(
     const SourceMgr &SM, StringRef Buffer,
     const std::vector<const FileCheckPattern *> &NotStrings,
     const FileCheckRequest &Req, std::vector<FileCheckDiag> *Diags) const {
   for (const FileCheckPattern *Pat : NotStrings) {
     assert((Pat->getCheckTy() == Check::CheckNot) && "Expect CHECK-NOT!");
 
     size_t MatchLen = 0;
-    size_t Pos = Pat->match(Buffer, MatchLen);
+    size_t Pos = Pat->match(Buffer, MatchLen, SM);
 
     if (Pos == StringRef::npos) {
       PrintNoMatch(false, SM, Prefix, Pat->getLoc(), *Pat, 1, Buffer,
                    Req.VerboseVerbose, Diags);
       continue;
     }
 
     PrintMatch(false, SM, Prefix, Pat->getLoc(), *Pat, 1, Buffer, Pos, MatchLen,
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);
-      size_t MatchPosBuf = Pat.match(MatchBuffer, MatchLen);
+      size_t MatchPosBuf = 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 (MatchPosBuf == StringRef::npos) {
         PrintNoMatch(true, SM, Prefix, Pat.getLoc(), Pat, 1, MatchBuffer,
                      Req.VerboseVerbose, Diags);
         return StringRef::npos;
       }
       // Re-calc it as the offset relative to the start of the original string.
bool FileCheckPatternContext::defineCmdlineVariables(
   SM.AddNewSourceBuffer(std::move(CmdLineDefsDiagBuffer), SMLoc());
 
   SmallVector<StringRef, 4> CmdlineDefsDiagVec;
   CmdlineDefsDiagRef.split(CmdlineDefsDiagVec, '\n', -1 /*MaxSplit*/,
                            false /*KeepEmpty*/);
   for (StringRef CmdlineDefDiag : CmdlineDefsDiagVec) {
     unsigned DefStart = CmdlineDefDiag.find(Prefix2) + Prefix2.size();
     StringRef CmdlineDef = CmdlineDefDiag.substr(DefStart);
-    if (CmdlineDef.find('=') == StringRef::npos) {
+    size_t EqIdx = CmdlineDef.find('=');
+    if (EqIdx == StringRef::npos) {
       SM.PrintMessage(SMLoc::getFromPointer(CmdlineDef.data()),
                       SourceMgr::DK_Error,
                       "Missing equal sign in global definition");
       ErrorFound = true;
       continue;
     }
 
     // Numeric variable definition.
     if (CmdlineDef[0] == '#') {
-      bool IsPseudo;
-      unsigned TrailIdx;
-      size_t EqIdx = CmdlineDef.find('=');
       StringRef CmdlineName = CmdlineDef.substr(1, EqIdx - 1);
-      if (FileCheckPattern::parseVariable(CmdlineName, IsPseudo, TrailIdx) ||
-          IsPseudo || TrailIdx != CmdlineName.size() || CmdlineName.empty()) {
-        SM.PrintMessage(SMLoc::getFromPointer(CmdlineName.data()),
-                        SourceMgr::DK_Error,
-                        "invalid name in numeric variable definition '" +
-                            CmdlineName + "'");
+      StringRef VarName;
+      SMLoc CmdlineNameLoc = SMLoc::getFromPointer(CmdlineName.data());
+      bool ParseError = FileCheckPattern::parseNumericVariableDefinition(
+          CmdlineName, VarName, this, SM);
+      // Check that CmdlineName starts with a valid numeric variable to be
+      // defined and that it is not followed that anything. That second check
+      // detects cases like "FOO+2" in a "FOO+2=10" definition.
+      if (ParseError || !CmdlineName.empty()) {
+        if (!ParseError)
+          SM.PrintMessage(CmdlineNameLoc, SourceMgr::DK_Error,
+                          "invalid variable name");
         ErrorFound = true;
         continue;
       }
 
       // Detect collisions between string and numeric variables when the latter
       // is created later than the former.
-      if (DefinedVariableTable.find(CmdlineName) !=
-          DefinedVariableTable.end()) {
+      if (DefinedVariableTable.find(VarName) != DefinedVariableTable.end()) {
         SM.PrintMessage(
-            SMLoc::getFromPointer(CmdlineName.data()), SourceMgr::DK_Error,
-            "string variable with name '" + CmdlineName + "' already exists");
+            SMLoc::getFromPointer(VarName.data()), SourceMgr::DK_Error,
+            "string variable with name '" + VarName + "' already exists");
         ErrorFound = true;
         continue;
       }
 
       StringRef CmdlineVal = CmdlineDef.substr(EqIdx + 1);
       uint64_t Val;
       if (CmdlineVal.getAsInteger(10, Val)) {
         SM.PrintMessage(SMLoc::getFromPointer(CmdlineVal.data()),
                         SourceMgr::DK_Error,
                         "invalid value in numeric variable definition '" +
                             CmdlineVal + "'");
         ErrorFound = true;
         continue;
       }
-      auto DefinedNumericVariable = makeNumericVariable(CmdlineName, Val);
+      auto DefinedNumericVariable = makeNumericVariable(0, VarName);
+      DefinedNumericVariable->setValue(Val);
 
       // Record this variable definition.
-      GlobalNumericVariableTable[CmdlineName] = DefinedNumericVariable;
+      GlobalNumericVariableTable[DefinedNumericVariable->getName()] =
+          DefinedNumericVariable;
     } else {
       // String variable definition.
       std::pair<StringRef, StringRef> CmdlineNameVal = CmdlineDef.split('=');
-      StringRef Name = CmdlineNameVal.first;
+      StringRef CmdlineName = CmdlineNameVal.first;
+      StringRef OrigCmdlineName = CmdlineName;
+      StringRef Name;
       bool IsPseudo;
-      unsigned TrailIdx;
-      if (FileCheckPattern::parseVariable(Name, IsPseudo, TrailIdx) ||
-          IsPseudo || TrailIdx != Name.size() || Name.empty()) {
-        SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
-                        "invalid name in string variable definition '" + Name +
-                            "'");
+      if (FileCheckPattern::parseVariable(CmdlineName, Name, IsPseudo) ||
+          IsPseudo || !CmdlineName.empty()) {
+        SM.PrintMessage(SMLoc::getFromPointer(OrigCmdlineName.data()),
+                        SourceMgr::DK_Error,
+                        "invalid name in string variable definition '" +
+                            OrigCmdlineName + "'");
         ErrorFound = true;
         continue;
       }
 
       // Detect collisions between string and numeric variables when the former
       // is created later than the latter.
       if (GlobalNumericVariableTable.find(Name) !=
           GlobalNumericVariableTable.end()) {
         SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
                         "numeric variable with name '" + Name +
                             "' already exists");
         ErrorFound = true;
         continue;
       }
       GlobalVariableTable.insert(CmdlineNameVal);
       // Mark the string variable as defined to detect collisions between
       // string and numeric variables in DefineCmdlineVariables when the latter
       // is created later than the former. We cannot reuse GlobalVariableTable
-      // for that by populating it with an empty string since we would then
+      // for this by populating it with an empty string since we would then
       // lose the ability to detect the use of an undefined variable in
       // match().
       DefinedVariableTable[Name] = true;
     }
   }
 
   return ErrorFound;
 }

llvm/trunk/test/FileCheck/numeric-defines-diagnostics.txt

 ; Test incorrect syntax for -D# option is correctly diagnosed.
 
 ; Invalid variable name: starts with a digit.
 RUN: not FileCheck -D#10VALUE=10 --input-file %s %s 2>&1 \
 RUN:   | FileCheck %s --strict-whitespace --check-prefix NUMERRCLIFMT
 
-NUMERRCLIFMT: Global defines:1:20: error: invalid name in numeric variable definition '10VALUE'
+NUMERRCLIFMT: Global defines:1:20: error: invalid variable name
 NUMERRCLIFMT-NEXT: Global define #1: #10VALUE=10
 NUMERRCLIFMT-NEXT: {{^                   \^$}}
 
 ; Invalid definition of pseudo variable.
 RUN: not FileCheck -D#@VALUE=10 --input-file %s %s 2>&1 \
 RUN:   | FileCheck %s --strict-whitespace --check-prefix NUMERRCLIPSEUDO
 
-NUMERRCLIPSEUDO: Global defines:1:20: error: invalid name in numeric variable definition '@VALUE'
+NUMERRCLIPSEUDO: Global defines:1:20: error: definition of pseudo numeric variable unsupported
 NUMERRCLIPSEUDO-NEXT: Global define #1: #@VALUE=10
 NUMERRCLIPSEUDO-NEXT: {{^                   \^$}}
 
 ; Invalid definition of an expression.
 RUN: not FileCheck -D#VALUE+2=10 --input-file %s %s 2>&1 \
 RUN:   | FileCheck %s --strict-whitespace --check-prefix NUMERRCLITRAIL
 
-NUMERRCLITRAIL: Global defines:1:20: error: invalid name in numeric variable definition 'VALUE+2'
+NUMERRCLITRAIL: Global defines:1:20: error: invalid variable name
 NUMERRCLITRAIL-NEXT: Global define #1: #VALUE+2=10
 NUMERRCLITRAIL-NEXT: {{^                   \^$}}
 
 ; Invalid value: numeric expression instead of literal.
 RUN: not FileCheck -D#VALUE1=3 -D#VALUE2='VALUE1 + 2' --input-file %s %s 2>&1 \
 RUN:   | FileCheck %s --strict-whitespace --check-prefix NUMERRCLIEXPR
 
 NUMERRCLIEXPR: Global defines:2:27: error: invalid value in numeric variable definition 'VALUE1 + 2'
 NUMERRCLIEXPR-NEXT: Global define #2: #VALUE2=VALUE1 + 2
 NUMERRCLIEXPR-NEXT: {{^                          \^$}}

llvm/trunk/test/FileCheck/numeric-expression.txt

-RUN: FileCheck -D#VAR1=11 --input-file %s %s
+RUN: FileCheck --input-file %s %s
 
 ; We use CHECK-NEXT directives to force a match on all lines with digits.
 
-; Numeric expressions using variables defined on the command-line without
-; spaces
+; Numeric variable definition without spaces.
+DEF NO SPC
+11
+CHECK-LABEL: DEF NO SPC
+CHECK-NEXT: [[#VAR1:]]
+
+; Numeric variable definition with different spacing.
+DEF SPC
+11
+11
+11
+CHECK-LABEL: DEF SPC
+CHECK-NEXT: [[# VAR1a:]]
+CHECK-NEXT: [[# VAR1b :]]
+CHECK-NEXT: [[# VAR1c : ]]
+
+; Numeric expressions using variables defined on other lines without spaces.
 USE NO SPC
 11
 12
 10
-CHECK-LABEL: USE NO SPC
+11
+11
+11
+CHECK-LABEL: USE
 CHECK-NEXT: [[#VAR1]]
 CHECK-NEXT: [[#VAR1+1]]
 CHECK-NEXT: [[#VAR1-1]]
-
-; Numeric expressions using variables defined on the command-line in alternate
-; spacing
-USE ALT SPC
+CHECK-NEXT: [[#VAR1a]]
+CHECK-NEXT: [[#VAR1b]]
+CHECK-NEXT: [[#VAR1c]]
+
+; Numeric expressions using variables defined on other lines with different
+; spacing.
+USE SPC
 11
 11
 12
 12
 12
 12
 10
 10
 10
 10
-CHECK-LABEL: USE ALT SPC
+CHECK-LABEL: USE SPC
 CHECK-NEXT: [[# VAR1]]
 CHECK-NEXT: [[# VAR1 ]]
 CHECK-NEXT: [[# VAR1+1]]
 CHECK-NEXT: [[# VAR1 +1]]
 CHECK-NEXT: [[# VAR1 + 1]]
 CHECK-NEXT: [[# VAR1 + 1 ]]
 CHECK-NEXT: [[# VAR1-1]]
 CHECK-NEXT: [[# VAR1 -1]]
 CHECK-NEXT: [[# VAR1 - 1]]
 CHECK-NEXT: [[# VAR1 - 1 ]]
 
 ; Numeric expressions using variables defined on the command-line and an
-; immediate interpreted as an unsigned value
+; immediate interpreted as an unsigned value.
 ; Note: 9223372036854775819 = 0x8000000000000000 + 11
 ;       9223372036854775808 = 0x8000000000000000
 USE UNSIGNED IMM
 9223372036854775819
 CHECK-LABEL: USE UNSIGNED IMM
 CHECK-NEXT: [[#VAR1+9223372036854775808]]
 
-; Numeric expression using undefined variable
+; Numeric expression using undefined variable.
 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: [[#UNDEFVAR]]
 UNDEF-USE-MSG: numeric-expression.txt:[[#@LINE-1]]:30: error: using undefined numeric variable 'UNDEFVAR'
 UNDEF-USE-MSG-NEXT: {{U}}NDEF-USE-NEXT: UNDEFVAR: {{\[\[#UNDEFVAR\]\]}}
 UNDEF-USE-MSG-NEXT: {{^                             \^$}}
 
-; Numeric expression with unsupported operator
-RUN: not FileCheck -D#VAR1=11 --check-prefixes CHECK,INVAL-OP --input-file %s %s 2>&1 \
+; Numeric expression with unsupported operator.
+RUN: not FileCheck -D#NUMVAR=10 --check-prefix INVAL-OP --input-file %s %s 2>&1 \
 RUN:   | FileCheck --strict-whitespace --check-prefix INVAL-OP-MSG %s
 
 INVALID OPERATOR
-VAR1*2: 22
+NUMVAR*2: 22
 INVAL-OP-LABEL: INVALID OPERATOR
-INVAL-OP-NEXT: VAR1*2: [[#VAR1*2]]
-INVAL-OP-MSG: numeric-expression.txt:[[#@LINE-1]]:31: error: unsupported numeric operation '*'
-INVAL-OP-MSG-NEXT: {{I}}NVAL-OP-NEXT: VAR1*2: {{\[\[#VAR1\*2\]\]}}
-INVAL-OP-MSG-NEXT: {{^                              \^$}}
+INVAL-OP-NEXT: NUMVAR*2: [[#NUMVAR*2]]
+INVAL-OP-MSG: numeric-expression.txt:[[#@LINE-1]]:35: error: unsupported numeric operation '*'
+INVAL-OP-MSG-NEXT: {{I}}NVAL-OP-NEXT: NUMVAR*2: {{\[\[#NUMVAR\*2\]\]}}
+INVAL-OP-MSG-NEXT: {{^                                  \^$}}
 
 ; Name conflict between Numeric variable definition and string variable
-; definition
-RUN: not FileCheck -D#VAR1=11 -D#NUMVAR=42 --check-prefixes CONFLICT,CONFLICT1 --input-file %s %s 2>&1 \
-RUN:   | FileCheck --strict-whitespace --check-prefix CLI-INPUT-PAT-CONFLICT %s
-RUN: not FileCheck -D#VAR1=11 -D#NUMVAR=42 -DNUMVAR=foobar --check-prefix CONFLICT --input-file %s %s 2>&1 \
-RUN:   | FileCheck --strict-whitespace --check-prefix CLI-CLI-PAT-CONFLICT %s
-RUN: not FileCheck -D#VAR1=11 -DPATVAR=foobar -D#PATVAR=42 --check-prefix CONFLICT --input-file %s %s 2>&1 \
-RUN:   | FileCheck --strict-whitespace --check-prefix CLI-CLI-NUM-CONFLICT %s
+; definition whether from the command-line or input text.
+RUN: not FileCheck --check-prefixes CONFLICT,CONFLICT1,CONFLICT2 --input-file %s %s 2>&1 \
+RUN:   | FileCheck --strict-whitespace --check-prefix INPUT-STR-CONFLICT %s
+RUN: not FileCheck -D#NUMVAR=42 --check-prefixes CONFLICT,CONFLICT2 --input-file %s %s 2>&1 \
+RUN:   | FileCheck --strict-whitespace --check-prefix INPUT-STR-CONFLICT %s
+RUN: not FileCheck -D#NUMVAR=42 -DNUMVAR=foobar --check-prefix CONFLICT --input-file %s %s 2>&1 \
+RUN:   | FileCheck --strict-whitespace --check-prefix CLI-STR-CONFLICT %s
+RUN: not FileCheck --check-prefixes CONFLICT,CONFLICT3,CONFLICT4 --input-file %s %s 2>&1 \
+RUN:   | FileCheck --strict-whitespace --check-prefix INPUT-NUM-CONFLICT %s
+RUN: not FileCheck -DSTRVAR=foobar --check-prefixes CONFLICT,CONFLICT4 --input-file %s %s 2>&1 \
+RUN:   | FileCheck --strict-whitespace --check-prefix INPUT-NUM-CONFLICT %s
+RUN: not FileCheck -DSTRVAR=foobar -D#STRVAR=42 --check-prefix CONFLICT --input-file %s %s 2>&1 \
+RUN:   | FileCheck --strict-whitespace --check-prefix CLI-NUM-CONFLICT %s
 
-PATVAR NUMVAR CONFLICT
+STRVAR NUMVAR CONFLICT
+redef1 42
 foobar
-CONFLICT-LABEL: PATVAR NUMVAR CONFLICT
-CONFLICT1-NEXT: [[NUMVAR:foo.*]]
-CLI-INPUT-PAT-CONFLICT: numeric-expression.txt:[[#@LINE-1]]:19: error: numeric variable with name 'NUMVAR' already exists
-CLI-INPUT-PAT-CONFLICT-NEXT: {{C}}ONFLICT1-NEXT: {{\[\[NUMVAR:foo\.\*\]\]}}
-CLI-INPUT-PAT-CONFLICT-NEXT: {{^                  \^$}}
-CLI-CLI-PAT-CONFLICT: Global defines:3:19: error: numeric variable with name 'NUMVAR' already exists
-CLI-CLI-PAT-CONFLICT-NEXT: Global define #3: NUMVAR=foobar
-CLI-CLI-PAT-CONFLICT-NEXT: {{^                  \^$}}
-CLI-CLI-NUM-CONFLICT: Global defines:3:20: error: string variable with name 'PATVAR' already exists
-CLI-CLI-NUM-CONFLICT-NEXT: Global define #3: #PATVAR=42
-CLI-CLI-NUM-CONFLICT-NEXT: {{^                   \^$}}
+redef2 42
+CONFLICT-LABEL: STRVAR NUMVAR CONFLICT
+CONFLICT1-NEXT: redef1 [[#NUMVAR:]]
+CONFLICT2: [[NUMVAR:foo.*]]
+CONFLICT3: [[STRVAR:foo.*]]
+CONFLICT4: redef2 [[#STRVAR:]]
+INPUT-STR-CONFLICT: numeric-expression.txt:[[#@LINE-3]]:14: error: numeric variable with name 'NUMVAR' already exists
+INPUT-STR-CONFLICT-NEXT: {{C}}ONFLICT2: {{\[\[NUMVAR:foo\.\*\]\]}}
+INPUT-STR-CONFLICT-NEXT: {{^             \^$}}
+CLI-STR-CONFLICT: Global defines:2:19: error: numeric variable with name 'NUMVAR' already exists
+CLI-STR-CONFLICT-NEXT: Global define #2: NUMVAR=foobar
+CLI-STR-CONFLICT-NEXT: {{^                  \^$}}
+INPUT-NUM-CONFLICT: numeric-expression.txt:[[#@LINE-7]]:22: error: string variable with name 'STRVAR' already exists
+INPUT-NUM-CONFLICT-NEXT: CONFLICT4: redef2 {{\[\[#STRVAR:\]\]}}
+INPUT-NUM-CONFLICT-NEXT: {{^                     \^$}}
+CLI-NUM-CONFLICT: Global defines:2:20: error: string variable with name 'STRVAR' already exists
+CLI-NUM-CONFLICT-NEXT: Global define #2: #STRVAR=42
+CLI-NUM-CONFLICT-NEXT: {{^                   \^$}}

llvm/trunk/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.
 
-RUN: FileCheck -D#LOCNUM=1 -D'#$GLOBNUM=1' --check-prefixes CHECK,LOCAL3,GLOBAL --input-file %s %s
-RUN: FileCheck -D#LOCNUM=1 -D'#$GLOBNUM=1' --check-prefixes CHECK,GLOBAL --enable-var-scope --input-file %s %s
-RUN: not FileCheck -D#LOCNUM=1 -D#'$GLOBNUM=1' --check-prefixes CHECK,LOCAL1 --enable-var-scope --input-file %s %s 2>&1 \
+; 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: not FileCheck --check-prefixes CHECK,LOCAL1 --enable-var-scope --input-file %s %s 2>&1 \
 RUN:   | FileCheck --check-prefixes ERRUNDEF,ERRUNDEFLOCAL %s
-RUN: not FileCheck -D#LOCNUM=1 -D#'$GLOBNUM=1' --check-prefixes CHECK,LOCAL2 --enable-var-scope --input-file %s %s 2>&1 \
+RUN: not FileCheck --check-prefixes CHECK,LOCAL2 --enable-var-scope --input-file %s %s 2>&1 \
 RUN:   | FileCheck --check-prefixes ERRUNDEF,ERRUNDEFLOCNUM %s
-RUN: not FileCheck -D#LOCNUM=1 -D#'$GLOBNUM=1' --check-prefixes CHECK,LOCAL3 --enable-var-scope --input-file %s %s 2>&1 \
+RUN: not FileCheck --check-prefixes CHECK,LOCAL3 --enable-var-scope --input-file %s %s 2>&1 \
 RUN:   | FileCheck --check-prefixes ERRUNDEF,ERRUNDEFLOCAL,ERRUNDEFLOCNUM %s
 
 local1
 global1
-CHECK: [[LOCAL:loc[^[:digit:]]*]][[#LOCNUM]]
-CHECK: [[$GLOBAL:glo[^[:digit:]]*]][[#$GLOBNUM]]
+CHECK: [[LOCAL:loc[^[:digit:]]*]][[#LOCNUM:]]
+CHECK: [[$GLOBAL:glo[^[:digit:]]*]][[#$GLOBNUM:]]
 
 local2
 global2
 CHECK: [[LOCAL]][[#LOCNUM+1]]
 CHECK: [[$GLOBAL]][[#$GLOBNUM+1]]
 
 barrier:
 CHECK-LABEL: barrier

llvm/trunk/test/FileCheck/verbose.txt

-; RUN: FileCheck -D#NUMVAR=42 -input-file %s %s 2>&1 | FileCheck -check-prefix QUIET --allow-empty %s
-; RUN: FileCheck -v -D#NUMVAR=42 -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix V %s
-; RUN: FileCheck -vv -D#NUMVAR=42 -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefixes V,VV %s
+; RUN: FileCheck -input-file %s %s 2>&1 | FileCheck -check-prefix QUIET --allow-empty %s
+; RUN: FileCheck -v -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix V %s
+; RUN: FileCheck -vv -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefixes V,VV %s
 
 foo
 bar
 CHECK: foo
 CHECK-NOT: raboof
 CHECK-NEXT: bar
 
 V:      verbose.txt:[[@LINE-4]]:8: remark: {{C}}HECK: expected string found in input

 
 VV-NEXT: verbose.txt:[[@LINE-17]]:12: remark: {{C}}HECK-NOT: excluded string not found in input
 VV-NEXT: {{C}}HECK-NOT: raboof{{$}}
 VV-NEXT:  {{^           \^$}}
 VV-NEXT: verbose.txt:[[@LINE-22]]:1: note: scanning from here
 VV-NEXT: {{^}}bar{{$}}
 VV-NEXT: {{^}}^{{$}}
 
-NUMVAR:42
+NUMVAR=42
 NUMVAR - 1:41
-CHECK: NUMVAR:[[#NUMVAR]]
+CHECK: NUMVAR=[[#NUMVAR:]]
 CHECK-NOT: [[#NUMVAR + 1]]
 CHECK-NEXT: NUMVAR - 1:[[#NUMVAR - 1]]
 
 V:      verbose.txt:[[#@LINE-4]]:8: remark: {{C}}HECK: expected string found in input
-V-NEXT: {{C}}HECK: {{NUMVAR:[[][[]#NUMVAR[]][]]$}}
+V-NEXT: {{C}}HECK: {{NUMVAR=[[][[]#NUMVAR:[]][]]$}}
 V-NEXT: {{^       \^$}}
 V-NEXT: verbose.txt:[[#@LINE-9]]:1: note: found here
-V-NEXT: {{^}}NUMVAR:42{{$}}
-V-NEXT: {{^}}^~~~~~~~~{{$}}
-V-NEXT: verbose.txt:[[#@LINE-12]]:1: note: with "NUMVAR" equal to "42"
-V-NEXT: {{^}}NUMVAR:42{{$}}
+V-NEXT: {{^}}NUMVAR=42{{$}}
 V-NEXT: {{^}}^~~~~~~~~{{$}}
 
-V-NEXT: verbose.txt:[[#@LINE-12]]:13: remark: {{C}}HECK-NEXT: expected string found in input
+V-NEXT: verbose.txt:[[#@LINE-9]]:13: remark: {{C}}HECK-NEXT: expected string found in input
 V-NEXT: {{C}}HECK-NEXT: {{NUMVAR - 1:[[][[]#NUMVAR - 1[]][]]$}}
 V-NEXT: {{^            \^$}}
-V-NEXT: verbose.txt:[[#@LINE-18]]:1: note: found here
+V-NEXT: verbose.txt:[[#@LINE-15]]:1: note: found here
 V-NEXT: {{^}}NUMVAR - 1:41{{$}}
 V-NEXT: {{^}}^~~~~~~~~~~~~{{$}}
-V-NEXT: verbose.txt:[[#@LINE-21]]:1: note: with "NUMVAR - 1" equal to "41"
+V-NEXT: verbose.txt:[[#@LINE-18]]:1: note: with "NUMVAR - 1" equal to "41"
 V-NEXT: {{^}}NUMVAR - 1:41{{$}}
 V-NEXT: {{^}}^~~~~~~~~~~~~{{$}}
 
-VV-NEXT: verbose.txt:[[#@LINE-23]]:12: remark: {{C}}HECK-NOT: excluded string not found in input
+VV-NEXT: verbose.txt:[[#@LINE-20]]:12: remark: {{C}}HECK-NOT: excluded string not found in input
 VV-NEXT: {{C}}HECK-NOT: {{[[][[]#NUMVAR [+] 1[]][]]$}}
-VV-NEXT: {{^           \^$}}
-VV-NEXT: verbose.txt:[[#@LINE-28]]:1: note: scanning from here
+VV-NEXT:  {{^           \^$}}
+VV-NEXT: verbose.txt:[[#@LINE-25]]:1: note: scanning from here
 VV-NEXT: {{^}}NUMVAR - 1:41{{$}}
 VV-NEXT: {{^}}^{{$}}
 
 before empty
 
 after empty
 CHECK: before empty
 CHECK-EMPTY:

llvm/trunk/unittests/Support/FileCheckTest.cpp

 #include "gtest/gtest.h"
 
 using namespace llvm;
 namespace {
 
 class FileCheckTest : public ::testing::Test {};
 
 TEST_F(FileCheckTest, NumericVariable) {
-  FileCheckNumericVariable FooVar = FileCheckNumericVariable("FOO", 42);
+  // Undefined variable: getValue and clearValue fails, setValue works.
+  FileCheckNumericVariable FooVar = FileCheckNumericVariable(1, "FOO");
   EXPECT_EQ("FOO", FooVar.getName());
-
-  // Defined variable: getValue returns a value, setValue fails and value
-  // remains unchanged.
   llvm::Optional<uint64_t> Value = FooVar.getValue();
+  EXPECT_FALSE(Value);
+  EXPECT_TRUE(FooVar.clearValue());
+  EXPECT_FALSE(FooVar.setValue(42));
+
+  // Defined variable: getValue returns value set, setValue fails.
+  Value = FooVar.getValue();
   EXPECT_TRUE(Value);
   EXPECT_EQ(42U, *Value);
   EXPECT_TRUE(FooVar.setValue(43));
   Value = FooVar.getValue();
   EXPECT_TRUE(Value);
   EXPECT_EQ(42U, *Value);
 
   // Clearing variable: getValue fails, clearValue again fails.
   EXPECT_FALSE(FooVar.clearValue());
   Value = FooVar.getValue();
   EXPECT_FALSE(Value);
   EXPECT_TRUE(FooVar.clearValue());
-
-  // Undefined variable: setValue works, getValue returns value set.
-  EXPECT_FALSE(FooVar.setValue(43));
-  Value = FooVar.getValue();
-  EXPECT_TRUE(Value);
-  EXPECT_EQ(43U, *Value);
 }
 
 uint64_t doAdd(uint64_t OpL, uint64_t OpR) { return OpL + OpR; }
 
 TEST_F(FileCheckTest, NumExpr) {
   FileCheckNumericVariable FooVar = FileCheckNumericVariable("FOO", 42);
   FileCheckNumExpr NumExpr = FileCheckNumExpr(doAdd, &FooVar, 18);
 
TEST_F(FileCheckTest, ValidVarNameStart) {
   EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('$'));
   EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('@'));
   EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('+'));
   EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('-'));
   EXPECT_FALSE(FileCheckPattern::isValidVarNameStart(':'));
 }
 
 TEST_F(FileCheckTest, ParseVar) {
-  StringRef VarName = "GoodVar42";
+  StringRef OrigVarName = "GoodVar42";
+  StringRef VarName = OrigVarName;
+  StringRef ParsedName;
   bool IsPseudo = true;
-  unsigned TrailIdx = 0;
-  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
+  EXPECT_EQ(ParsedName, OrigVarName);
+  EXPECT_TRUE(VarName.empty());
   EXPECT_FALSE(IsPseudo);
-  EXPECT_EQ(TrailIdx, VarName.size());
 
-  VarName = "$GoodGlobalVar";
+  VarName = OrigVarName = "$GoodGlobalVar";
   IsPseudo = true;
-  TrailIdx = 0;
-  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
+  EXPECT_EQ(ParsedName, OrigVarName);
+  EXPECT_TRUE(VarName.empty());
   EXPECT_FALSE(IsPseudo);
-  EXPECT_EQ(TrailIdx, VarName.size());
 
-  VarName = "@GoodPseudoVar";
+  VarName = OrigVarName = "@GoodPseudoVar";
   IsPseudo = true;
-  TrailIdx = 0;
-  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
+  EXPECT_EQ(ParsedName, OrigVarName);
+  EXPECT_TRUE(VarName.empty());
   EXPECT_TRUE(IsPseudo);
-  EXPECT_EQ(TrailIdx, VarName.size());
 
   VarName = "42BadVar";
-  EXPECT_TRUE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_TRUE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
 
   VarName = "$@";
-  EXPECT_TRUE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_TRUE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
 
-  VarName = "B@dVar";
+  VarName = OrigVarName = "B@dVar";
   IsPseudo = true;
-  TrailIdx = 0;
-  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
+  EXPECT_EQ(VarName, OrigVarName.substr(1));
+  EXPECT_EQ(ParsedName, "B");
   EXPECT_FALSE(IsPseudo);
-  EXPECT_EQ(TrailIdx, 1U);
 
-  VarName = "B$dVar";
+  VarName = OrigVarName = "B$dVar";
   IsPseudo = true;
-  TrailIdx = 0;
-  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
+  EXPECT_EQ(VarName, OrigVarName.substr(1));
+  EXPECT_EQ(ParsedName, "B");
   EXPECT_FALSE(IsPseudo);
-  EXPECT_EQ(TrailIdx, 1U);
 
   VarName = "BadVar+";
   IsPseudo = true;
-  TrailIdx = 0;
-  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
+  EXPECT_EQ(VarName, "+");
+  EXPECT_EQ(ParsedName, "BadVar");
   EXPECT_FALSE(IsPseudo);
-  EXPECT_EQ(TrailIdx, VarName.size() - 1);
 
   VarName = "BadVar-";
   IsPseudo = true;
-  TrailIdx = 0;
-  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
+  EXPECT_EQ(VarName, "-");
+  EXPECT_EQ(ParsedName, "BadVar");
   EXPECT_FALSE(IsPseudo);
-  EXPECT_EQ(TrailIdx, VarName.size() - 1);
 
   VarName = "BadVar:";
   IsPseudo = true;
-  TrailIdx = 0;
-  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
+  EXPECT_EQ(VarName, ":");
+  EXPECT_EQ(ParsedName, "BadVar");
   EXPECT_FALSE(IsPseudo);
-  EXPECT_EQ(TrailIdx, VarName.size() - 1);
 }
 
 static StringRef bufferize(SourceMgr &SM, StringRef Str) {
   std::unique_ptr<MemoryBuffer> Buffer =
       MemoryBuffer::getMemBufferCopy(Str, "TestBuffer");
   StringRef StrBufferRef = Buffer->getBuffer();
   SM.AddNewSourceBuffer(std::move(Buffer), SMLoc());
   return StrBufferRef;
 }
 
-class ExprTester {
+class PatternTester {
 private:
+  size_t LineNumber = 1;
   SourceMgr SM;
   FileCheckRequest Req;
   FileCheckPatternContext Context;
-  FileCheckPattern P = FileCheckPattern(Check::CheckPlain, &Context);
+  FileCheckPattern P =
+      FileCheckPattern(Check::CheckPlain, &Context, LineNumber++);
 
 public:
-  ExprTester() {
+  PatternTester() {
     std::vector<std::string> GlobalDefines;
     GlobalDefines.emplace_back(std::string("#FOO=42"));
+    GlobalDefines.emplace_back(std::string("BAR=BAZ"));
     Context.defineCmdlineVariables(GlobalDefines, SM);
-    // Call ParsePattern to have @LINE defined.
-    P.ParsePattern("N/A", "CHECK", SM, 1, Req);
+    // Call parsePattern to have @LINE defined.
+    P.parsePattern("N/A", "CHECK", SM, Req);
+    // parsePattern does not expect to be called twice for the same line and
+    // will set FixedStr and RegExStr incorrectly if it is. Therefore prepare
+    // a pattern for a different line.
+    initNextPattern();
   }
 
-  bool parseExpect(std::string &VarName, std::string &Trailer) {
-    bool IsPseudo = VarName[0] == '@';
-    std::string NameTrailer = VarName + Trailer;
-    StringRef NameTrailerRef = bufferize(SM, NameTrailer);
-    StringRef VarNameRef = NameTrailerRef.substr(0, VarName.size());
-    StringRef TrailerRef = NameTrailerRef.substr(VarName.size());
-    return P.parseNumericSubstitution(VarNameRef, IsPseudo, TrailerRef, SM) ==
-           nullptr;
+  void initNextPattern() {
+    P = FileCheckPattern(Check::CheckPlain, &Context, LineNumber++);
+  }
+
+  bool parseNumVarDefExpect(StringRef Expr) {
+    StringRef ExprBufferRef = bufferize(SM, Expr);
+    StringRef Name;
+    return FileCheckPattern::parseNumericVariableDefinition(ExprBufferRef, Name,
+                                                            &Context, SM);
+  }
+
+  bool parseSubstExpect(StringRef Expr) {
+    StringRef ExprBufferRef = bufferize(SM, Expr);
+    FileCheckNumericVariable *DefinedNumericVariable;
+    return P.parseNumericSubstitutionBlock(
+               ExprBufferRef, DefinedNumericVariable, SM) == nullptr;
+  }
+
+  bool parsePatternExpect(StringRef Pattern) {
+    StringRef PatBufferRef = bufferize(SM, Pattern);
+    return P.parsePattern(PatBufferRef, "CHECK", SM, Req);
+  }
+
+  bool matchExpect(StringRef Buffer) {
+    StringRef BufferRef = bufferize(SM, Buffer);
+    size_t MatchLen;
+    return P.match(BufferRef, MatchLen, SM);
   }
 };
 
-TEST_F(FileCheckTest, ParseExpr) {
-  ExprTester Tester;
+TEST_F(FileCheckTest, ParseNumericVariableDefinition) {
+  PatternTester Tester;
+
+  // Invalid definition of pseudo.
+  EXPECT_TRUE(Tester.parseNumVarDefExpect("@LINE"));
 
-  // @LINE with offset.
-  std::string VarName = "@LINE";
-  std::string Trailer = "+3";
-  EXPECT_FALSE(Tester.parseExpect(VarName, Trailer));
-
-  // @LINE only.
-  Trailer = "";
-  EXPECT_FALSE(Tester.parseExpect(VarName, Trailer));
+  // Conflict with pattern variable.
+  EXPECT_TRUE(Tester.parseNumVarDefExpect("BAR"));
 
   // Defined variable.
-  VarName = "FOO";
-  EXPECT_FALSE(Tester.parseExpect(VarName, Trailer));
+  EXPECT_FALSE(Tester.parseNumVarDefExpect("FOO"));
+}
+
+TEST_F(FileCheckTest, ParseExpr) {
+  PatternTester Tester;
+
+  // Variable definition.
+
+  // Definition of invalid variable.
+  EXPECT_TRUE(Tester.parseSubstExpect("10VAR:"));
+  EXPECT_TRUE(Tester.parseSubstExpect("@FOO:"));
+  EXPECT_TRUE(Tester.parseSubstExpect("@LINE:"));
+
+  // Garbage after name of variable being defined.
+  EXPECT_TRUE(Tester.parseSubstExpect("VAR GARBAGE:"));
+
+  // Variable defined to numeric expression.
+  EXPECT_TRUE(Tester.parseSubstExpect("VAR1: FOO"));
+
+  // Acceptable variable definition.
+  EXPECT_FALSE(Tester.parseSubstExpect("VAR1:"));
+  EXPECT_FALSE(Tester.parseSubstExpect("  VAR2:"));
+  EXPECT_FALSE(Tester.parseSubstExpect("VAR3  :"));
+  EXPECT_FALSE(Tester.parseSubstExpect("VAR3:  "));
+
+  // Numeric expression.
 
-  // Undefined variable.
-  VarName = "UNDEF";
-  EXPECT_TRUE(Tester.parseExpect(VarName, Trailer));
-
-  // Wrong Pseudovar.
-  VarName = "@FOO";
-  EXPECT_TRUE(Tester.parseExpect(VarName, Trailer));
+  // Unacceptable variable.
+  EXPECT_TRUE(Tester.parseSubstExpect("10VAR"));
+  EXPECT_TRUE(Tester.parseSubstExpect("@FOO"));
+  EXPECT_TRUE(Tester.parseSubstExpect("UNDEF"));
+
+  // Only valid variable.
+  EXPECT_FALSE(Tester.parseSubstExpect("@LINE"));
+  EXPECT_FALSE(Tester.parseSubstExpect("FOO"));
+
+  // Use variable defined on same line.
+  EXPECT_FALSE(Tester.parsePatternExpect("[[#LINE1VAR:]]"));
+  EXPECT_TRUE(Tester.parseSubstExpect("LINE1VAR"));
 
   // Unsupported operator.
-  VarName = "@LINE";
-  Trailer = "/2";
-  EXPECT_TRUE(Tester.parseExpect(VarName, Trailer));
+  EXPECT_TRUE(Tester.parseSubstExpect("@LINE/2"));
 
   // Missing offset operand.
-  VarName = "@LINE";
-  Trailer = "+";
-  EXPECT_TRUE(Tester.parseExpect(VarName, Trailer));
+  EXPECT_TRUE(Tester.parseSubstExpect("@LINE+"));
 
   // Cannot parse offset operand.
-  VarName = "@LINE";
-  Trailer = "+x";
-  EXPECT_TRUE(Tester.parseExpect(VarName, Trailer));
+  EXPECT_TRUE(Tester.parseSubstExpect("@LINE+x"));
 
   // Unexpected string at end of numeric expression.
-  VarName = "@LINE";
-  Trailer = "+5x";
-  EXPECT_TRUE(Tester.parseExpect(VarName, Trailer));
+  EXPECT_TRUE(Tester.parseSubstExpect("@LINE+5x"));
+
+  // Valid expression.
+  EXPECT_FALSE(Tester.parseSubstExpect("@LINE+5"));
+  EXPECT_FALSE(Tester.parseSubstExpect("FOO+4"));
+}
+
+TEST_F(FileCheckTest, ParsePattern) {
+  PatternTester Tester;
+
+  // Space in pattern variable expression.
+  EXPECT_TRUE(Tester.parsePatternExpect("[[ BAR]]"));
+
+  // Invalid variable name.
+  EXPECT_TRUE(Tester.parsePatternExpect("[[42INVALID]]"));
+
+  // Invalid pattern variable definition.
+  EXPECT_TRUE(Tester.parsePatternExpect("[[@PAT:]]"));
+  EXPECT_TRUE(Tester.parsePatternExpect("[[PAT+2:]]"));
+
+  // Collision with numeric variable.
+  EXPECT_TRUE(Tester.parsePatternExpect("[[FOO:]]"));
+
+  // Valid use of pattern variable.
+  EXPECT_FALSE(Tester.parsePatternExpect("[[BAR]]"));
+
+  // Valid pattern variable definition.
+  EXPECT_FALSE(Tester.parsePatternExpect("[[PAT:[0-9]+]]"));
+
+  // Invalid numeric expressions.
+  EXPECT_TRUE(Tester.parsePatternExpect("[[#42INVALID]]"));
+  EXPECT_TRUE(Tester.parsePatternExpect("[[#@FOO]]"));
+  EXPECT_TRUE(Tester.parsePatternExpect("[[#@LINE/2]]"));
+  EXPECT_TRUE(Tester.parsePatternExpect("[[#2+@LINE]]"));
+  EXPECT_TRUE(Tester.parsePatternExpect("[[#YUP:@LINE]]"));
+
+  // Valid numeric expressions and numeric variable definition.
+  EXPECT_FALSE(Tester.parsePatternExpect("[[#FOO]]"));
+  EXPECT_FALSE(Tester.parsePatternExpect("[[#@LINE+2]]"));
+  EXPECT_FALSE(Tester.parsePatternExpect("[[#NUMVAR:]]"));
+}
+
+TEST_F(FileCheckTest, Match) {
+  PatternTester Tester;
+
+  // Check matching a definition only matches a number.
+  Tester.parsePatternExpect("[[#NUMVAR:]]");
+  EXPECT_TRUE(Tester.matchExpect("FAIL"));
+  EXPECT_FALSE(Tester.matchExpect("18"));
+
+  // Check matching the variable defined matches the correct number only
+  Tester.initNextPattern();
+  Tester.parsePatternExpect("[[#NUMVAR]] [[#NUMVAR+2]]");
+  EXPECT_TRUE(Tester.matchExpect("19 21"));
+  EXPECT_TRUE(Tester.matchExpect("18 21"));
+  EXPECT_FALSE(Tester.matchExpect("18 20"));
 }
 
 TEST_F(FileCheckTest, Substitution) {
   SourceMgr SM;
   FileCheckPatternContext Context;
   std::vector<std::string> GlobalDefines;
   GlobalDefines.emplace_back(std::string("FOO=BAR"));
   Context.defineCmdlineVariables(GlobalDefines, SM);
 
   // Substitution of an undefined string variable fails.
   FileCheckStringSubstitution StringSubstitution =
       FileCheckStringSubstitution(&Context, "VAR404", 42);
   EXPECT_FALSE(StringSubstitution.getResult());
 
   // Substitutions of defined pseudo and non-pseudo numeric variables return
   // the right value.
   FileCheckNumericVariable LineVar = FileCheckNumericVariable("@LINE", 42);
-  FileCheckNumericVariable NVar = FileCheckNumericVariable("@N", 10);
+  FileCheckNumericVariable NVar = FileCheckNumericVariable("N", 10);
   FileCheckNumExpr NumExprLine = FileCheckNumExpr(doAdd, &LineVar, 0);
   FileCheckNumExpr NumExprN = FileCheckNumExpr(doAdd, &NVar, 3);
   FileCheckNumericSubstitution SubstitutionLine =
       FileCheckNumericSubstitution(&Context, "@LINE", &NumExprLine, 12);
   FileCheckNumericSubstitution SubstitutionN =
       FileCheckNumericSubstitution(&Context, "N", &NumExprN, 30);
   llvm::Optional<std::string> Value = SubstitutionLine.getResult();
   EXPECT_TRUE(Value);
   EXPECT_EQ("42", *Value);
   Value = SubstitutionN.getResult();
   EXPECT_TRUE(Value);
   EXPECT_EQ("13", *Value);
 
   // Substitution of an undefined numeric variable fails.
   LineVar.clearValue();
   EXPECT_FALSE(SubstitutionLine.getResult());
   NVar.clearValue();
   EXPECT_FALSE(SubstitutionN.getResult());
 
   // Substitution of a defined string variable returns the right value.
-  FileCheckPattern P = FileCheckPattern(Check::CheckPlain, &Context);
+  FileCheckPattern P = FileCheckPattern(Check::CheckPlain, &Context, 1);
   StringSubstitution = FileCheckStringSubstitution(&Context, "FOO", 42);
   Value = StringSubstitution.getResult();
   EXPECT_TRUE(Value);
   EXPECT_EQ("BAR", *Value);
 }
 
 TEST_F(FileCheckTest, UndefVars) {
   SourceMgr SM;
TEST_F(FileCheckTest, FileCheckContext) {
   EXPECT_FALSE(GotError);
 
   // Check defined variables are present and undefined is absent.
   StringRef LocalVarStr = "LocalVar";
   StringRef LocalNumVarRef = bufferize(SM, "LocalNumVar");
   StringRef EmptyVarStr = "EmptyVar";
   StringRef UnknownVarStr = "UnknownVar";
   llvm::Optional<StringRef> LocalVar = Cxt.getPatternVarValue(LocalVarStr);
-  FileCheckPattern P = FileCheckPattern(Check::CheckPlain, &Cxt);
-  FileCheckNumExpr *NumExpr =
-      P.parseNumericSubstitution(LocalNumVarRef, false /*IsPseudo*/, "", SM);
+  FileCheckPattern P = FileCheckPattern(Check::CheckPlain, &Cxt, 1);
+  FileCheckNumericVariable *DefinedNumericVariable;
+  FileCheckNumExpr *NumExpr = P.parseNumericSubstitutionBlock(
+      LocalNumVarRef, DefinedNumericVariable, SM);
   llvm::Optional<StringRef> EmptyVar = Cxt.getPatternVarValue(EmptyVarStr);
   llvm::Optional<StringRef> UnknownVar = Cxt.getPatternVarValue(UnknownVarStr);
   EXPECT_TRUE(LocalVar);
   EXPECT_EQ(*LocalVar, "FOO");
   EXPECT_TRUE(NumExpr);
   llvm::Optional<uint64_t> NumExprVal = NumExpr->eval();
   EXPECT_TRUE(NumExprVal);
   EXPECT_EQ(*NumExprVal, 18U);
   EXPECT_TRUE(EmptyVar);
   EXPECT_EQ(*EmptyVar, "");
   EXPECT_FALSE(UnknownVar);
 
   // Clear local variables and check they become absent.
   Cxt.clearLocalVars();
   LocalVar = Cxt.getPatternVarValue(LocalVarStr);
   EXPECT_FALSE(LocalVar);
   // Check a numeric expression's evaluation fails if called after clearing of
   // local variables, if it was created before. This is important because local
   // variable clearing due to --enable-var-scope happens after numeric
   // expressions are linked to the numeric variables they use.
   EXPECT_FALSE(NumExpr->eval());
-  P = FileCheckPattern(Check::CheckPlain, &Cxt);
-  NumExpr =
-      P.parseNumericSubstitution(LocalNumVarRef, false /*IsPseudo*/, "", SM);
+  P = FileCheckPattern(Check::CheckPlain, &Cxt, 2);
+  NumExpr = P.parseNumericSubstitutionBlock(LocalNumVarRef,
+                                            DefinedNumericVariable, SM);
   EXPECT_FALSE(NumExpr);
   EmptyVar = Cxt.getPatternVarValue(EmptyVarStr);
   EXPECT_FALSE(EmptyVar);
 
   // Redefine global variables and check variables are defined again.
   GlobalDefines.emplace_back(std::string("$GlobalVar=BAR"));
   GlobalDefines.emplace_back(std::string("#$GlobalNumVar=36"));
   GotError = Cxt.defineCmdlineVariables(GlobalDefines, SM);
   EXPECT_FALSE(GotError);
   StringRef GlobalVarStr = "$GlobalVar";
   StringRef GlobalNumVarRef = bufferize(SM, "$GlobalNumVar");
   llvm::Optional<StringRef> GlobalVar = Cxt.getPatternVarValue(GlobalVarStr);
   EXPECT_TRUE(GlobalVar);
   EXPECT_EQ(*GlobalVar, "BAR");
-  P = FileCheckPattern(Check::CheckPlain, &Cxt);
-  NumExpr =
-      P.parseNumericSubstitution(GlobalNumVarRef, false /*IsPseudo*/, "", SM);
+  P = FileCheckPattern(Check::CheckPlain, &Cxt, 3);
+  NumExpr = P.parseNumericSubstitutionBlock(GlobalNumVarRef,
+                                            DefinedNumericVariable, SM);
   EXPECT_TRUE(NumExpr);
   NumExprVal = NumExpr->eval();
   EXPECT_TRUE(NumExprVal);
   EXPECT_EQ(*NumExprVal, 36U);
 
   // Clear local variables and check global variables remain defined.
   Cxt.clearLocalVars();
   GlobalVar = Cxt.getPatternVarValue(GlobalVarStr);
   EXPECT_TRUE(GlobalVar);
-  P = FileCheckPattern(Check::CheckPlain, &Cxt);
-  NumExpr =
-      P.parseNumericSubstitution(GlobalNumVarRef, false /*IsPseudo*/, "", SM);
+  P = FileCheckPattern(Check::CheckPlain, &Cxt, 4);
+  NumExpr = P.parseNumericSubstitutionBlock(GlobalNumVarRef,
+                                            DefinedNumericVariable, SM);
   EXPECT_TRUE(NumExpr);
   NumExprVal = NumExpr->eval();
   EXPECT_TRUE(NumExprVal);
   EXPECT_EQ(*NumExprVal, 36U);
 }
 } // namespace