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

624

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

llvm/include/llvm/Support/FileCheck.h

400–401

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

406

"of the numeric"
comma after "fails"

409

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.")

llvm/lib/Support/FileCheck.cpp

47–48

Omit the != nullptr on pointer assertions.

156

I think this comment should not have changed.

343

no else after return

400

variables -> variable names

427–437

= false

482–484

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

488

Omit != nullptr

1833–1837

"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

1833–1837

Rather incorrect conflict resouition on my part I suspect.

[probinson]

llvm/docs/CommandGuide/FileCheck.rst

605

"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).

611

"lines" (or "text")

llvm/lib/Support/FileCheck.cpp

487–493

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

llvm/test/FileCheck/numeric-expression.txt

89–90

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.

102–104

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

llvm/test/FileCheck/var-scope.txt

0–1

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

@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

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

590

Perhaps worth adding "or on the command line".

llvm/include/llvm/Support/FileCheck.h

43

class, variable -> class. Variable

78

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?

82–83

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.

165

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

244

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

251

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

273–274

definition -> definitions.

273–274

from -> by (as it was).

275

What is "NumericVariableDefs" referring to here?

347

See above comments re. capture parentheses

pointer to corresponding -> pointer to the corresponding

350

Should this use a StringMap?

369

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

407

I'd put a comma after "substituted"

424

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".

426

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

165

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

87–88

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.

150

See main comment.

367–368

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

400–407

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

43

the respective definition -> their respective definitions

70–74

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

83

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

105

This should return size_t to match DefLineNumber's type.

245–246

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

275

in -> in the

347

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

364

This type should match that of the earlier DefLineNumber member.

367–368

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

llvm/lib/Support/FileCheck.cpp

160

variable -> variables

205

same line -> same line as used (?)

224

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

317

Nit: delete this blank line.

351

Parse the numeric...

399–408

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."

427–437

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

525

No need for struct here.

535

Mark string -> Mark the string

536

variable -> variables

538

that -> this

539

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

660

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

670

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.

1749–1750

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.

1774

"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–13

Use double-dash like the test was before here.

22

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

28–30

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.

45–46

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.

90

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

108

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

0–1

"variable are remain defined"

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

10–13

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

[probinson]

llvm/include/llvm/Support/FileCheck.h

367–368

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

367–368

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

367–368

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

70–74

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

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?

347

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?

367–368

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?

llvm/lib/Support/FileCheck.cpp

224

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

660

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.

670

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).

1749–1750

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.

1774

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

108

Argh indeed.

llvm/test/FileCheck/var-scope.txt

10–13

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

245–246

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.

369

size_t here?

llvm/lib/Support/FileCheck.cpp

1749–1750

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

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

392

Comma after "fails"

424

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

341

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

341

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

341

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

341

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 ↗

(On Diff #203343)

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 ↗

(On Diff #203343)

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 ↗

(On Diff #203343)

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 ↗

(On Diff #203343)

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 ↗

(On Diff #203343)

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/docs/CommandGuide/FileCheck.rst

 local.  All local variables get undefined at the beginning of each
 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 Variables and Expressions
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-:program:`FileCheck` also allows to check for numeric values satisfying a
-numeric expression constraint based on numeric variables. This allows to
-capture numeric relations between two numbers in a text to check, such as the
-the need for consecutive registers to be used.
+:program:`FileCheck` also allows to define numeric variables and check for
+values satisfying a numeric expression constraint based on those variables.
+This allows to capture numeric relations between two numbers in a text to
+check, 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.
+
+For example:
+
+.. code-block:: llvm
+
+    ; CHECK: mov r[[#REG:]], 42
+
+would match ``mov r5, 42`` and set ``REG`` to the value ``5``.
 
 The syntax to check a numeric expression constraint is
 ``[[#<NUMVAR><op><offset>]]`` where:
 
-*``<NUMVAR>`` is the name of a numeric variable defined on the command line.
+*``<NUMVAR>`` is the name of a numeric variable defined on a previous line.
 
 *``<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.
 
 For example:
 
 .. code-block:: llvm
 
-    ; CHECK: add r[[#REG]], r[[#REG]], r[[#REG+1]]
+    ; CHECK: add r[[#REG:]], r[[#REG]], r[[#REG+1]]
 
 The above example would match the lines:
 
 .. code-block:: llvm
 
     add r5, r5, r6
 
 but would not match the lines:
 
 .. code-block:: llvm
 
     add r5, r5, r7
 
 due to ``7`` being unequal to ``5 + 1``.
 
 In the same way as for pattern variables, ``--enable-var-scope`` only consider
 global numeric variables that start with ``$`` and undefined local variables at
 the beginning of each CHECK-LABEL block.
 
+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 which 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.

llvm/include/llvm/Support/FileCheck.h

struct FileCheckRequest {
   bool Verbose = false;
   bool VerboseVerbose = false;
 };
 
 //===----------------------------------------------------------------------===//
 // Numeric expression handling code.
 //===----------------------------------------------------------------------===//
 
 class FileCheckASTBinop;
 
 /// Class representing a numeric expression.
 class FileCheckNumExpr {
 private:
   /// Pointer to AST of the numeric expression.
   std::unique_ptr<FileCheckASTBinop> AST;
 
 public:
   /// Generic constructor for a numeric expression whose equality constraint is
   /// represented by \p AST.
   FileCheckNumExpr(FileCheckASTBinop *AST) : AST(std::move(AST)) {}
 
   /// Return pointer to AST of the numeric expression. Pointer is guaranteed to
   /// be valid as long as this object is.
   FileCheckASTBinop *getAST() const { return AST.get(); }
 };
 
 /// Class representing a numeric variable with a given value in the AST of a
-/// numeric expression.
+/// numeric expression. Each definition of a variable gets its own instance of
+/// this class, variable uses share the same instance as the respective
+/// definition.
 class FileCheckNumVar {
 private:
   /// Name of the numeric variable.
   StringRef Name;
 
   /// Whether variable is defined and thus Value is set.
   bool Defined;
 
   /// Value of numeric variable if defined.
   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.
+  unsigned DefLineNumber;
+
 public:
+  /// Constructor for a variable \p Name defined at line \p DefLineNumber.
+  FileCheckNumVar(StringRef Name, unsigned DefLineNumber)
+      : Name(Name), Defined(false), DefLineNumber(DefLineNumber) {}
+
   /// Constructor for numeric variable \p Name with a known \p Value at parse
   /// time (e.g. the @LINE numeric variable).
   explicit FileCheckNumVar(StringRef Name, uint64_t Value)
       : Name(Name), Defined(true), Value(Value) {}
 
   /// Return name of that numeric variable.
   StringRef getName() const { return Name; }
 
   /// Return value of this numeric variable.
   llvm::Optional<uint64_t> getValue() const;
 
   /// Set value of this numeric variable if not defined. Return whether
   /// the variable was already defined.
   bool setValue(uint64_t Value);
 
   /// Clear value of this numeric variable. Return whether the variable was
   /// already undefined.
   bool clearValue();
+
+  /// Return line number where this variable is defined.
+  unsigned getDefLineNumber() { return DefLineNumber; }
 };
 
 /// Type of functions evaluating a given binary operation.
 using binop_eval_t = uint64_t (*)(uint64_t, uint64_t);
 
 /// Class representing a single binary operation in the AST of a numeric
 /// expression.
 class FileCheckASTBinop {

 private:
   /// Pointer to a class instance holding among other thing the table with the
   /// values of live pattern variables at the start of any given CHECK line.
   /// Used for substituting pattern variables (numeric variables have their
   /// value in the FileCheckNumExpr class instance pointed to by NumExpr).
   FileCheckPatternContext *Context;
 
   /// Whether this represents a numeric expression substitution.
   bool IsNumExpr;
 
   /// The string that needs to be substituted for something else. For a
   /// pattern variable this is its name, otherwise this is the whole numeric
   /// expression.
   StringRef SubstStr;
 
   /// If this is a numeric expression substitution, this is the pointer to the
   /// class representing that numeric expression.
   FileCheckNumExpr *NumExpr;
 
   // Index in RegExStr of where to do the substitution.
   unsigned InsertIdx;
 
 public:
   /// Constructor for a pattern variable substitution.
   FileCheckPatternSubst(FileCheckPatternContext *Context, StringRef VarName,
                         unsigned InsertIdx)
       : Context(Context), IsNumExpr(false), SubstStr(VarName),
         InsertIdx(InsertIdx) {}
 
   /// Constructor for a numeric expression substitution.
   FileCheckPatternSubst(FileCheckPatternContext *Context, StringRef Expr,
                         FileCheckNumExpr *NumExpr, unsigned InsertIdx)
public:
   FileCheckType &setCount(int C);
 
   std::string getDescription(StringRef Prefix) const;
 };
 } // namespace Check
 
 struct FileCheckDiag;
 
+/// Structure representing the definition of a numeric variable in a pattern.
+/// It holds the parenthesized capture number and the pointer to the class
+/// representing the numeric variable whose value is being defined.
+struct FileCheckNumExprMatch {
+  /// Pointer to class representing the numeric variable whose value is being
+  /// defined.
+  FileCheckNumVar *NumVarDef;
+
+  /// Parenthesized capture number for this numeric variable definition.
+  unsigned CaptureParen;
+};
+
 /// Class holding the FileCheckPattern global state, shared by all patterns:
 /// tables holding values of variables and whether they are defined or not at
 /// any given time in the matching process.
 class FileCheckPatternContext {
   friend class FileCheckPattern;
 
 private:
   /// When matching a given pattern, this holds the value of all the FileCheck
   /// pattern variables defined in previous patterns. In a pattern, only the
   /// last definition for a given variable is recorded in this table,
   /// back-references are used for uses after any the other definition.
   StringMap<StringRef> GlobalVariableTable;
 
   /// Map of all pattern variables defined so far. Used at parse time to detect
   /// a name conflict between a numeric variable and a pattern 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 AST of all numeric variables defined in previous
   /// patterns along with their values. In a pattern, only the last definition
-  /// for a given variable is recorded in this table.
+  /// for a given variable is recorded in this table. When matching a pattern
+  /// all definitions for that pattern are recorded in NumericVariableDefs
+  /// table.
   StringMap<FileCheckNumVar *> GlobalNumericVariableTable;
 
   /// Vector holding pointers to all numeric expressions parsed. 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 numeric variables parsed. Used to

 
 private:
   /// Register a numeric expression for destruction when the context is
   /// destroyed.
   FileCheckNumExpr *registerNumExpr(FileCheckASTBinop *AST);
 
   /// Make a new numeric variable and register it for destruction when the
   /// context is destroyed.
-  FileCheckNumVar *makeNumVar(StringRef Name, uint64_t Value);
+  template <class... Types> FileCheckNumVar *makeNumVar(Types... args);
 };
 
 class FileCheckPattern {
   SMLoc PatternLoc;
 
   /// A fixed string to match as the pattern or empty if this pattern requires
   /// a regex match.
   StringRef FixedStr;
class FileCheckPattern {
   /// pattern variable or a numeric expression.
   std::vector<FileCheckPatternSubst> Substs;
 
   /// Maps names of pattern variables defined in a pattern to the parenthesized
   /// capture numbers of 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".
   ///
   /// Note: uses std::map rather than StringMap to be able to get the key when
   /// iterating over values.
   std::map<StringRef, unsigned> VariableDefs;
 
+  /// Holds the capture parentheses number and pointer to corresponding
+  /// FileCheckNumVar class instance of all numeric variable definitions. Used
+  /// to set the matched value of all those variables.
+  std::map<StringRef, FileCheckNumExprMatch> NumericVariableDefs;
+
   /// Pointer to a class instance holding the global state shared by all
   /// patterns:
   /// - tables with the values of live pattern and numeric variables at
   ///   the start of any given CHECK line;
   /// - table holding whether a pattern 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.
+  /// 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.
   unsigned LineNumber;
 
 public:
   explicit FileCheckPattern(Check::FileCheckType Ty,
-                            FileCheckPatternContext *Context)
-      : Context(Context), CheckTy(Ty) {}
+                            FileCheckPatternContext *Context,
+                            unsigned LineNumber)
+      : Context(Context), CheckTy(Ty), LineNumber(LineNumber) {}
 
   /// 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; }
 
   /// Return whether \p is a valid first character for a variable name.
   static bool isValidVarNameStart(char C);
   /// Verify that the string at the start of \p Str is a well formed variable.
   /// Return false if it is and set \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 return true.
   static bool parseVariable(StringRef Str, bool &IsPseudo, unsigned &TrailIdx);
-  /// Parse a numeric expression involving (pseudo if \p IsPseudo is true)
-  /// variable \p Name with the string corresponding to the operation being
-  /// performed in \p Trailer. Return the class representing the numeric
-  /// expression or nullptr if parsing fails in which case errors are reported
-  /// on \p SM.
-  FileCheckNumExpr *parseNumericExpression(StringRef Name, bool IsPseudo,
-                                           StringRef Trailer,
+  /// Parse \p Expr for use or definition (if \p IsDefinition is true) of a
+  /// numeric variable. Return whether parsing fails in which case errors are
+  /// reported on \p SM. Otherwise, the name of the numeric variable is set in
+  /// \p Name.
+  bool parseNumericVariable(StringRef &Expr, StringRef &Name, bool IsDefinition,
+                            const SourceMgr &SM) const;
+  /// Parse \p Expr for a numeric expression. Return the class representing the
+  /// AST of numeric expression or nullptr if parsing fails in which case
+  /// errors are reported on \p SM. Set \p NumVarDef to the pointer to the
+  /// class representing the variable defined to this numeric expression if
+  /// any.
+  FileCheckNumExpr *parseNumericExpression(StringRef Expr,
+                                           FileCheckNumVar *&NumVarDef,
                                            const SourceMgr &SM) const;
   bool ParsePattern(StringRef PatternStr, StringRef Prefix, SourceMgr &SM,
-                    unsigned LineNumber, const FileCheckRequest &Req);
-  size_t match(StringRef Buffer, size_t &MatchLen) const;
+                    const FileCheckRequest &Req);
+  size_t match(StringRef Buffer, size_t &MatchLen, const SourceMgr &SM) const;
   /// Print value of successful substitutions or name of undefined pattern or
   /// numeric variables preventing such a successful substitution.
   void printSubsts(const SourceMgr &SM, StringRef Buffer,
                    SMRange MatchRange = None) const;
   void printFuzzyMatch(const SourceMgr &SM, StringRef Buffer,
                        std::vector<FileCheckDiag> *Diags) const;
 
   bool hasVariable() const { return !(Substs.empty() && VariableDefs.empty()); }
 
   Check::FileCheckType getCheckTy() const { return CheckTy; }
 
   int getCount() const { return CheckTy.getCount(); }
 
 private:
   bool AddRegExToRegEx(StringRef RS, unsigned &CurParen, SourceMgr &SM);
   void AddBackrefToRegEx(unsigned BackrefNum);
   unsigned computeMatchDistance(StringRef Buffer) const;
   size_t FindRegexVarEnd(StringRef Str, SourceMgr &SM);
+
+  /// Numeric expression parsing helpers.
+  FileCheckASTBinop *parseFileCheckBinop(StringRef &Expr,
+                                         const SourceMgr &SM) const;
 };
 
 //===----------------------------------------------------------------------===//
 /// Summary of a FileCheck diagnostic.
 //===----------------------------------------------------------------------===//
 
 struct FileCheckDiag {
   /// What is the FileCheck directive for this diagnostic?
   Check::FileCheckType CheckTy;
   /// Where is the FileCheck directive for this diagnostic?

llvm/lib/Support/FileCheck.cpp

 }
 
 bool FileCheckNumVar::clearValue() {
   if (!Defined)
     return true;
   Defined = false;
   return false;
 }
 
 llvm::Optional<uint64_t> FileCheckASTBinop::eval() const {
   llvm::Optional<uint64_t> Opl = this->Opl->getValue();
   // Variable has been undefined.
   if (!Opl)
     return llvm::None;
   return EvalBinop(*Opl, Opr);
 }
 
 StringRef FileCheckASTBinop::getUndefVarName() const {
   if (!Opl->getValue())
     return Opl->getName();
   return StringRef();
 }
 
 llvm::Optional<std::string> FileCheckPatternSubst::getSubstitute() const {
   if (IsNumExpr) {
+    assert(NumExpr->getAST() != nullptr &&
+           "Substituting empty numeric expression");
     llvm::Optional<uint64_t> EvaluatedValue = NumExpr->getAST()->eval();
     if (!EvaluatedValue)
       return llvm::None;
     return utostr(*EvaluatedValue);
   } else {
     // Look up the value and escape it so that we can put it into the
     // regex.
     llvm::Optional<StringRef> VarVal = Context->getPatternVarValue(SubstStr);

 
 // 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 doAdd(uint64_t Opl, uint64_t Opr) { return Opl + Opr; }
-static uint64_t doSub(uint64_t Opl, uint64_t Opr) { return Opl - Opr; }
-
-FileCheckNumExpr *
-FileCheckPattern::parseNumericExpression(StringRef Name, bool IsPseudo,
-                                         StringRef Trailer,
+bool FileCheckPattern::parseNumericVariable(StringRef &Expr, StringRef &Name,
+                                            bool IsDefinition,
                                             const SourceMgr &SM) const {
+  bool IsPseudo;
+  unsigned TrailIdx;
+
+  if (parseVariable(Expr, IsPseudo, TrailIdx)) {
+    SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
+                    "invalid variable name");
+    return true;
+  }
+  Name = Expr.substr(0, TrailIdx);
+  Expr = Expr.substr(TrailIdx);
+
   if (IsPseudo && !Name.equals("@LINE")) {
     SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
                     "invalid pseudo numeric variable '" + Name + "'");
-    return nullptr;
+    return true;
+  }
+  if (IsDefinition && IsPseudo) {
+    SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
+                    "definition of pseudo variable '" + Name + "' unsupported");
+    return true;
   }
 
+  if (IsDefinition) {
+    // Detect collision between pattern and numeric variable 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,
+                      "pattern variable with name '" + Name +
+                          "' already exists");
+      return true;
+    }
+
+    return false;
+  } else {
     // This method is indirectly called from ParsePattern for all numeric
     // variable definition and uses in the order in which they appear in the
     // CHECK pattern. For each definition, the pointer to the corresponding AST
     // class instance is stored in GlobalNumericVariableTable. Therefore the
     // pointer we get below is for the AST class instance corresponding to the
     // last definition of the variable before this 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;
+      return true;
+    }
+
+    FileCheckNumVar *NumVar = VarTableIter->second;
+    if (!IsPseudo && NumVar->getDefLineNumber() == LineNumber) {
+      SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
+                      "numeric variable '" + NumVar->getName() +
+                          "' defined on the same line");
+      return true;
     }
 
-  FileCheckNumVar *Opl = VarTableIter->second;
+    return false;
+  }
+}
+
+static uint64_t doAdd(uint64_t Opl, uint64_t Opr) { return Opl + Opr; }
+
+static uint64_t doSub(uint64_t Opl, uint64_t Opr) { return Opl - Opr; }
+
+/// Parse \p Expr for a binary operation.
+/// Return the class representing that binary operation in the AST of the
+/// numeric expression or nullptr if parsing fails in which case errors
+/// are reported on \p SM.
+FileCheckASTBinop *
+FileCheckPattern::parseFileCheckBinop(StringRef &Expr,
+                                      const SourceMgr &SM) const {
+  StringRef Name;
+  if (parseNumericVariable(Expr, Name, false, SM))
+    // Error reporting done in parseNumericVariable.
+    return nullptr;
+  FileCheckNumVar *Opl = Context->GlobalNumericVariableTable.find(Name)->second;
 
   // Check if this is a supported operation and selection function to perform
   // it.
-  skipWhitespace(Trailer);
-  if (Trailer.empty()) {
-    return Context->registerNumExpr(new FileCheckASTBinop(doAdd, Opl, 0));
-  }
-  SMLoc OpLoc = SMLoc::getFromPointer(Trailer.data());
-  char Operator = popFront(Trailer);
+  skipWhitespace(Expr);
+  if (Expr.empty())
+    return new FileCheckASTBinop(doAdd, Opl, 0);
+  SMLoc OpLoc = SMLoc::getFromPointer(Expr.data());
+  char Operator = popFront(Expr);
   binop_eval_t EvalBinop;
   switch (Operator) {
   case '+':
     EvalBinop = doAdd;
     break;
   case '-':
     EvalBinop = doSub;
     break;
   default:
     SM.PrintMessage(OpLoc, SourceMgr::DK_Error,
                     Twine("unsupported numeric operation '") + Twine(Operator) +
                         "'");
     return nullptr;
   }
 
   // Parse right operand.
-  skipWhitespace(Trailer);
-  if (Trailer.empty()) {
-    SM.PrintMessage(SMLoc::getFromPointer(Trailer.data()), SourceMgr::DK_Error,
+  skipWhitespace(Expr);
+  if (Expr.empty()) {
+    SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
                     "missing operand in numeric expression");
     return nullptr;
   }
   uint64_t Opr;
-  if (Trailer.consumeInteger(10, Opr)) {
-    SM.PrintMessage(SMLoc::getFromPointer(Trailer.data()), SourceMgr::DK_Error,
-                    "invalid offset in numeric expression '" + Trailer + "'");
+  if (Expr.consumeInteger(10, Opr)) {
+    SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
+                    "invalid offset in numeric expression '" + Expr + "'");
     return nullptr;
   }
-  skipWhitespace(Trailer);
-  if (!Trailer.empty()) {
-    SM.PrintMessage(SMLoc::getFromPointer(Trailer.data()), SourceMgr::DK_Error,
+  skipWhitespace(Expr);
+  if (!Expr.empty()) {
+    SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
                     "unexpected characters at end of numeric expression '" +
-                        Trailer + "'");
+                        Expr + "'");
+    return nullptr;
+  }
+
+  return new FileCheckASTBinop(EvalBinop, Opl, Opr);
+}
+
+/// Parse \p Expr for a numeric expression. Return the class representing the
+/// AST of numeric expression or nullptr if parsing fails in which case errors
+/// are reported on \p SM. Set \p NumVarDef to the pointer to the class
+/// representing the variable defined to this numeric expression if any.
+FileCheckNumExpr *FileCheckPattern::parseNumericExpression(
+    StringRef Expr, FileCheckNumVar *&NumVarDef, const SourceMgr &SM) const {
+  FileCheckASTBinop *NumExprAST = nullptr;
+
+  // Parse numeric variable definition.
+  NumVarDef = nullptr;
+  size_t DefEnd = Expr.find(':');
+  if (DefEnd != StringRef::npos) {
+    StringRef DefExpr = Expr.substr(0, DefEnd);
+    StringRef UseExpr = Expr = Expr.substr(DefEnd + 1);
+
+    skipWhitespace(DefExpr);
+    StringRef Name;
+    if (parseNumericVariable(DefExpr, Name, true /*IsDefinition*/, SM))
+      // Invalid variable definition. Error reporting done in parsing function.
+      return nullptr;
+
+    NumVarDef = Context->makeNumVar(Name, this->LineNumber);
+
+    skipWhitespace(DefExpr);
+    if (!DefExpr.empty()) {
+      SM.PrintMessage(SMLoc::getFromPointer(DefExpr.data()),
+                      SourceMgr::DK_Error,
+                      "invalid numeric variable definition");
+      return nullptr;
+    }
+    skipWhitespace(UseExpr);
+    if (!UseExpr.empty()) {
+      SM.PrintMessage(
+          SMLoc::getFromPointer(UseExpr.data()), SourceMgr::DK_Error,
+          "unexpected string after variable definition: '" + UseExpr + "'");
+      return nullptr;
+    }
+  } else {
+    // Parse numeric expression itself.
+    skipWhitespace(Expr);
+    NumExprAST = parseFileCheckBinop(Expr, SM);
+    if (NumExprAST == nullptr)
       return nullptr;
   }
 
-  return Context->registerNumExpr(new FileCheckASTBinop(EvalBinop, Opl, Opr));
+  return Context->registerNumExpr(NumExprAST);
 }
 
 /// Parses the given string into the Pattern.
 ///
 /// \p Prefix provides which prefix is being matched, \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 FileCheckPattern::ParsePattern(StringRef PatternStr, StringRef Prefix,
-                           SourceMgr &SM, unsigned LineNumber,
+                                    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;
+  uint64_t LineNumber64 = this->LineNumber;
   auto LinePseudoVar = Context->makeNumVar(LinePseudo, LineNumber64);
   Context->GlobalNumericVariableTable[LinePseudo] = LinePseudoVar;
 
   if (!(Req.NoCanonicalizeWhiteSpace && Req.MatchFullLines))
     // Ignore trailing whitespace.
     while (!PatternStr.empty() &&
            (PatternStr.back() == ' ' || PatternStr.back() == '\t'))
       PatternStr = PatternStr.substr(0, PatternStr.size() - 1);
 
   // Check that there is something on the line.
   if (PatternStr.empty() && CheckTy != Check::CheckEmpty) {
     SM.PrintMessage(PatternLoc, SourceMgr::DK_Error,
                     "found empty check string with prefix '" + Prefix + ":'");
     return true;
   }
 
   if (!PatternStr.empty() && CheckTy == Check::CheckEmpty) {
     SM.PrintMessage(
         PatternLoc, SourceMgr::DK_Error,
         "found non-empty check string for empty check with prefix '" + Prefix +
             ":'");
     return true;
   }
 
   if (CheckTy == Check::CheckEmpty) {
     RegExStr = "(\n$)";
     return false;
   }
 
   // Check to see if this is a fixed string, or if it has regex pieces.
   if (!MatchFullLinesHere &&
       (PatternStr.size() < 2 || (PatternStr.find("{{") == StringRef::npos &&
                                  PatternStr.find("[[") == StringRef::npos))) {
     FixedStr = PatternStr;
     return false;
   }
if (PatternStr.startswith("{{")) {
 
       if (AddRegExToRegEx(PatternStr.substr(2, End - 2), CurParen, SM))
         return true;
       RegExStr += ')';
 
       PatternStr = PatternStr.substr(End + 2);
       continue;
     }
 
     // Pattern variable and numeric expressions matches. Pattern variables
     // come in two forms: [[foo:.*]] which matches .* (or some other regex) and
     // assigns it to the FileCheck variable 'foo'. The second form is [[foo]]
     // which is a substitution of foo's value. Variable names themselves must
     // be of the form "[a-zA-Z_][0-9a-zA-Z_]*", otherwise we reject it. This is
-    // to catch some common errors. Numeric expressions only have the
-    // substitution mode. Numeric variable names have the same restriction as
-    // their pattern variable counterpart.
+    // to catch some common errors. Numeric expressions also have the
+    // definition and substitution modes and numeric variable names have the
+    // same restriction as their pattern variable counterpart.
     if (PatternStr.startswith("[[")) {
       StringRef MatchStr = PatternStr.substr(2);
       bool IsNumExpr = MatchStr.consume_front("#");
       const char *RefTypeStr =
           IsNumExpr ? "numeric expression" : "pattern variable";
       // 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(MatchStr, SM);
 
       if (End == StringRef::npos) {
         SM.PrintMessage(
             SMLoc::getFromPointer(PatternStr.data()), SourceMgr::DK_Error,
             std::string("Invalid ") + RefTypeStr + " reference, no ]] found");
         return true;
       }
 
       MatchStr = MatchStr.substr(0, End);
       PatternStr = PatternStr.substr(End + 4 + (int)IsNumExpr);
 
+      bool IsVarDef;
+      StringRef DefName;
+      StringRef SubstStr;
+      StringRef MatchRegexp;
+      unsigned SubstInsertIdx = RegExStr.size();
+      FileCheckNumVar *NumVarDef;
+      FileCheckNumExpr *NumExpr;
+
+      // Parse pattern variable or legacy numeric expression.
+      if (!IsNumExpr) {
         size_t VarEndIdx = MatchStr.find(":");
-      if (IsNumExpr)
-        skipWhitespace(MatchStr);
-      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 regex name (e.g. "foo") and verify it is well formed.
         bool IsPseudo;
         unsigned TrailIdx;
         if (parseVariable(MatchStr, IsPseudo, TrailIdx)) {
           SM.PrintMessage(SMLoc::getFromPointer(MatchStr.data()),
                           SourceMgr::DK_Error, "invalid variable name");
           return true;
         }
 
-      unsigned SubstInsertIdx = RegExStr.size();
-      FileCheckNumExpr *NumExpr;
-
         StringRef Name = MatchStr.substr(0, TrailIdx);
         StringRef Trailer = MatchStr.substr(TrailIdx);
-      bool IsVarDef = (VarEndIdx != StringRef::npos);
+        IsVarDef = (VarEndIdx != StringRef::npos);
+        IsNumExpr = IsPseudo;
 
         if (IsVarDef) {
-        if (IsPseudo || !Trailer.consume_front(":")) {
+          if ((IsPseudo || !Trailer.consume_front(":"))) {
             SM.PrintMessage(SMLoc::getFromPointer(MatchStr.data()),
                             SourceMgr::DK_Error,
                             "invalid name in pattern variable definition");
             return true;
           }
 
-        // Detect collision between pattern and numeric variable when the former
-        // is created later than the latter.
+          // Detect collision between pattern and numeric variable 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,
                 "numeric variable with name '" + Name + "' already exists");
             return true;
           }
+          DefName = Name;
+          MatchRegexp = Trailer;
+        } else
+          SubstStr = MatchStr;
       }
 
-      if (IsNumExpr || (!IsVarDef && IsPseudo)) {
-        NumExpr = parseNumericExpression(Name, IsPseudo, Trailer, SM);
+      // Parse numeric expression.
+      if (IsNumExpr) {
+        NumExpr = parseNumericExpression(MatchStr, NumVarDef, SM);
         if (NumExpr == nullptr)
           return true;
         IsNumExpr = true;
+        if (NumVarDef != nullptr) {
+          IsVarDef = true;
+          DefName = NumVarDef->getName();
+          MatchRegexp = StringRef("[0-9]+");
+        } else
+          SubstStr = MatchStr;
       }
 
       // Handle variable use: [[foo]] and [[#<foo expr>]].
       if (!IsVarDef) {
         // Handle use of pattern variables that were defined earlier on the
-        // same line by emitting a backreference.
-        if (!IsNumExpr && VariableDefs.find(Name) != VariableDefs.end()) {
-          unsigned CaptureParen = VariableDefs[Name];
+        // same line by emitting a backreference. Numeric expressions do not
+        // support using a numeric variable defined on the same line.
+        if (!IsNumExpr && VariableDefs.find(SubstStr) != VariableDefs.end()) {
+          unsigned CaptureParen = VariableDefs[SubstStr];
           if (CaptureParen < 1 || CaptureParen > 9) {
-            SM.PrintMessage(SMLoc::getFromPointer(Name.data()),
+            SM.PrintMessage(SMLoc::getFromPointer(SubstStr.data()),
                             SourceMgr::DK_Error,
                             "Can't back-reference more than 9 variables");
             return true;
           }
           AddBackrefToRegEx(CaptureParen);
         } else {
           // Handle use of pattern variables ([[<var>]]) defined in previous
           // CHECK pattern or use of a numeric expression.
           FileCheckPatternSubst Subst =
               IsNumExpr
-                  ? FileCheckPatternSubst(Context, MatchStr, NumExpr,
+                  ? FileCheckPatternSubst(Context, SubstStr, NumExpr,
                                           SubstInsertIdx)
-                  : FileCheckPatternSubst(Context, MatchStr, SubstInsertIdx);
+                  : FileCheckPatternSubst(Context, SubstStr, SubstInsertIdx);
           Substs.push_back(Subst);
         }
         continue;
       }
 
-      // Handle [[foo:.*]].
-      VariableDefs[Name] = CurParen;
+      // Handle variable definition: [[<def>:(...)]] and [[#(...)<def>:(...)]].
+      if (IsNumExpr) {
+        struct FileCheckNumExprMatch NumExprDef = {NumVarDef, CurParen};
+        NumericVariableDefs[DefName] = NumExprDef;
+        // This store is done here rather than in match() to allow
+        // parseNumericVariable() to get the pointer to the class instance
+        // of the right variable definition corresponding to a given numeric
+        // variable use.
+        Context->GlobalNumericVariableTable[DefName] = NumVarDef;
+      } else {
+        VariableDefs[DefName] = CurParen;
         // Mark pattern variable as defined to detect collision between pattern
-      // and numeric variable 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 loose
-      // the ability to detect use of undefined variable in Match().
-      Context->DefinedVariableTable[Name] = true;
+        // and numeric variable in parseNumericVariable and
+        // 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 loose the ability to
+        // detect use of 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("{{");

 /// Matches the pattern string against the input buffer \p Buffer
 ///
 /// This returns the position that is matched or npos if there is no match. If
 /// there is a match, the size of the matched string is returned in \p
 /// MatchLen.
 ///
 /// The GlobalVariableTable StringMap in the FileCheckPatternContext class
 /// instance provides the current values of FileCheck pattern variables and is
-/// updated if this match defines new values.
-size_t FileCheckPattern::match(StringRef Buffer, size_t &MatchLen) const {
+/// 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 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 pattern 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) {
+    assert(NumericVariableDef.second.CaptureParen < MatchInfo.size() &&
+           "Internal paren error");
+    unsigned CaptureParen = NumericVariableDef.second.CaptureParen;
+    FileCheckNumVar *NumVarDef = NumericVariableDef.second.NumVarDef;
+
+    StringRef MatchedValue = MatchInfo[CaptureParen];
+    uint64_t Val;
+    if (MatchedValue.getAsInteger(10, Val)) {
+      SM.PrintMessage(SMLoc::getFromPointer(MatchedValue.data()),
+                      SourceMgr::DK_Error, "Unable to represent numeric value");
+    }
+    if (NumVarDef->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;
 }
 

 }
 
 FileCheckNumExpr *
 FileCheckPatternContext::registerNumExpr(FileCheckASTBinop *AST) {
   NumExprs.emplace_back(new FileCheckNumExpr(AST));
   return NumExprs.back().get();
 }
 
-FileCheckNumVar *FileCheckPatternContext::makeNumVar(StringRef Name,
-                                                     uint64_t Value) {
-  NumVars.emplace_back(new FileCheckNumVar(Name, Value));
+template <class... Types>
+FileCheckNumVar *FileCheckPatternContext::makeNumVar(Types... args) {
+  NumVars.emplace_back(new FileCheckNumVar(args...));
   return NumVars.back().get();
 }
 
 /// 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). Returns the offset of the closing sequence within Str,
 /// or npos if it was not found.
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));
+        FileCheckPattern(Check::CheckNot, &PatternContext, 0));
     ImplicitNegativeChecks.back().ParsePattern(PatternInBuffer,
-                                               "IMPLICIT-CHECK", SM, 0, Req);
+                                               "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(
   // Create a buffer with fake command line content in order to display
   // parsing diagnostic with location information and point to the
   // global definition with invalid syntax.
   std::unique_ptr<MemoryBuffer> CmdLineDefsDiagBuffer =
       MemoryBuffer::getMemBufferCopy(CmdlineDefsDiag, "Global defines");
   StringRef CmdlineDefsDiagRef = CmdLineDefsDiagBuffer->getBuffer();
   SM.AddNewSourceBuffer(std::move(CmdLineDefsDiagBuffer), SMLoc());
 
+  // Dummy pattern to call parseNumericVariable.
+  FileCheckPattern P(Check::CheckPlain, this, 0);
+
   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 Expr = CmdlineDef.substr(1, EqIdx - 1);
+      StringRef CmdlineName;
+      SMLoc CmdlineNameLoc = SMLoc::getFromPointer(Expr.data());
+      if (P.parseNumericVariable(Expr, CmdlineName, true /*IsDefinition*/,
+                                 SM) ||
+          !Expr.empty()) {
+        SM.PrintMessage(CmdlineNameLoc, SourceMgr::DK_Error,
+                        "invalid variable name");
         ErrorFound = true;
         continue;
       }
 
       // Detect collision between pattern and numeric variable when the latter
       // is created later than the former.
       if (DefinedVariableTable.find(CmdlineName) !=
           DefinedVariableTable.end()) {
if (CmdlineDef[0] == '#') {
       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 NumVarDef = makeNumVar(CmdlineName, Val);
+      auto NumVarDef = makeNumVar(CmdlineName, (unsigned)0);
+      NumVarDef->setValue(Val);
 
       // Record this variable definition.
-      GlobalNumericVariableTable[CmdlineName] = NumVarDef;
+      GlobalNumericVariableTable[NumVarDef->getName()] = NumVarDef;
     } else {
       // Pattern variable definition.
       std::pair<StringRef, StringRef> CmdlineNameVal = CmdlineDef.split('=');
       StringRef Name = CmdlineNameVal.first;
       bool IsPseudo;
       unsigned TrailIdx;
       if (FileCheckPattern::parseVariable(Name, IsPseudo, TrailIdx) ||
           IsPseudo || TrailIdx != Name.size() || Name.empty()) {
if (CmdlineDef[0] == '#') {
           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 pattern variable as defined to detect collision between pattern
       // and numeric variable 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 loose
-      // the ability to detect use of undefined variable in Match().
+      // the ability to detect use of undefined variable in match().
       DefinedVariableTable[Name] = true;
     }
   }
 
   return ErrorFound;
 }
 
 void FileCheckPatternContext::clearLocalVars() {

llvm/test/FileCheck/defines.txt

 ; NUMERRMSG: defines.txt:1:1: note: scanning from here
 ; NUMERRMSG: defines.txt:1:1: note: with numeric expression "NUMVAL" equal to "8"
 ; NUMERRMSG: defines.txt:[[#@LINE-7]]:1: note: possible intended match here
 
 ; NOT-NUMERRMSG: defines.txt:[[#@LINE-7]]:15: error: {{NUMNOT}}-NOT: excluded string found in input
 ; NOT-NUMERRMSG: defines.txt:[[#@LINE-10]]:1: note: found here
 ; NOT-NUMERRMSG: defines.txt:[[#@LINE-11]]:1: note: with numeric expression "NUMVAL" equal to "12"
 
-; 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: {{^                   \^$}}
 
-; NUMERRCLIPSEUDO: Global defines:1:20: error: invalid name in numeric variable definition '@VALUE'
+; NUMERRCLIPSEUDO: Global defines:1:20: error: invalid pseudo numeric variable
 ; NUMERRCLIPSEUDO-NEXT: Global define #1: #@VALUE=10
 ; NUMERRCLIPSEUDO-NEXT: {{^                   \^$}}
 
-; 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: {{^                   \^$}}
 
 ; 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/test/FileCheck/numeric-expression.txt

-; RUN: FileCheck -D#VAR1=11 -input-file %s %s
+; RUN: FileCheck -input-file %s %s
 ; RUN: not FileCheck -check-prefix UNDEF-USE -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix UNDEF-USE-MSG %s
-; RUN: not FileCheck -D#VAR1=11 -check-prefixes CHECK,INVAL-OP -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix INVAL-OP-MSG %s
-; RUN: not FileCheck -D#VAR1=11 -D#NUMVAR=42 -check-prefixes CONFLICT,CONFLICT1 -input-file %s %s 2>&1 | 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 | 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 | FileCheck --strict-whitespace -check-prefix CLI-CLI-NUM-CONFLICT %s
+; RUN: not FileCheck -check-prefixes CHECK,INVAL-OP -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix INVAL-OP-MSG %s
+; RUN: not FileCheck -check-prefixes CONFLICT,CONFLICT1,CONFLICT2 -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix INPUT-PAT-CONFLICT %s
+; RUN: not FileCheck -D#NUMVAR=42 -check-prefixes CONFLICT,CONFLICT2 -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix INPUT-PAT-CONFLICT %s
+; RUN: not FileCheck -D#NUMVAR=42 -DNUMVAR=foobar -check-prefix CONFLICT -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix CLI-PAT-CONFLICT %s
+; RUN: not FileCheck -check-prefixes CONFLICT,CONFLICT3,CONFLICT4 -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix INPUT-NUM-CONFLICT %s
+; RUN: not FileCheck -DPATVAR=foobar -check-prefixes CONFLICT,CONFLICT4 -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix INPUT-NUM-CONFLICT %s
+; RUN: not FileCheck -DPATVAR=foobar -D#PATVAR=42 -check-prefix CONFLICT -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix CLI-NUM-CONFLICT %s
 
 
 ; We ensure we attemt to match all lines with digits by using CHECK-NEXT
 ; directives for the checks to ensure each line of input is covered.
 
 
-; 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 in alternate spacing
+DEF ALT SPC
+11
+11
+11
+; CHECK-LABEL: DEF ALT 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
+; 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
+; Numeric expressions using variables defined on other lines in alternate
 ; spacing
 USE ALT SPC
 11
 11
 12
 12
 12
 12
 10

 ; Numeric expression using undefined variable
 UNDEF VAR USE
 UNDEFVAR: 11
 ; UNDEF-USE-LABEL: UNDEF VAR USE
 ; UNDEF-USE-NEXT: UNDEFVAR: [[#UNDEFVAR]]
 ; UNDEF-USE-MSG: numeric-expression.txt:[[#@LINE-1]]:32: 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
 INVALID OPERATOR
 VAR1*2: 22
 ; INVAL-OP-LABEL: INVALID OPERATOR
 ; INVAL-OP-NEXT: VAR1*2: [[#VAR1*2]]
 ; INVAL-OP-MSG: numeric-expression.txt:[[#@LINE-1]]:33: error: unsupported numeric operation '*'
 ; INVAL-OP-MSG-NEXT: ; {{I}}NVAL-OP-NEXT: VAR1*2: {{\[\[#VAR1\*2\]\]}}
 ; INVAL-OP-MSG-NEXT: {{^                                \^$}}
 
 
 ; Name conflict between Numeric variable definition and pattern variable
-; definition
+; definition whether from the command-line or input text
 PATVAR NUMVAR CONFLICT
+redef1 42
 foobar
+redef2 42
 ; CONFLICT-LABEL: PATVAR NUMVAR CONFLICT
-; CONFLICT1-NEXT: [[NUMVAR:foo.*]]
-; CLI-INPUT-PAT-CONFLICT: numeric-expression.txt:[[#@LINE-1]]:21: 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: pattern variable with name 'PATVAR' already exists
-; CLI-CLI-NUM-CONFLICT-NEXT: Global define #3: #PATVAR=42
-; CLI-CLI-NUM-CONFLICT-NEXT: {{^                   \^$}}
+; CONFLICT1-NEXT: redef1 [[#NUMVAR:]]
+; CONFLICT2: [[NUMVAR:foo.*]]
+; CONFLICT3: [[PATVAR:foo.*]]
+; CONFLICT4: redef2 [[#PATVAR:]]
+; INPUT-PAT-CONFLICT: numeric-expression.txt:[[#@LINE-3]]:16: error: numeric variable with name 'NUMVAR' already exists
+; INPUT-PAT-CONFLICT-NEXT: ; {{C}}ONFLICT2: {{\[\[NUMVAR:foo\.\*\]\]}}
+; INPUT-PAT-CONFLICT-NEXT: {{^               \^$}}
+; CLI-PAT-CONFLICT: Global defines:2:19: error: numeric variable with name 'NUMVAR' already exists
+; CLI-PAT-CONFLICT-NEXT: Global define #2: NUMVAR=foobar
+; CLI-PAT-CONFLICT-NEXT: {{^                  \^$}}
+; INPUT-NUM-CONFLICT: numeric-expression.txt:[[#@LINE-7]]:24: error: pattern variable with name 'PATVAR' already exists
+; INPUT-NUM-CONFLICT-NEXT: ; CONFLICT4: redef2 {{\[\[#PATVAR:\]\]}}
+; INPUT-NUM-CONFLICT-NEXT: {{^                       \^$}}
+; CLI-NUM-CONFLICT: Global defines:2:20: error: pattern variable with name 'PATVAR' already exists
+; CLI-NUM-CONFLICT-NEXT: Global define #2: #PATVAR=42
+; CLI-NUM-CONFLICT-NEXT: {{^                   \^$}}

llvm/test/FileCheck/var-scope.txt

-// RUN: FileCheck -D#LOCNUM=1 -D'#$GLOBNUM=1' -input-file %s %s
-// RUN: FileCheck -D#LOCNUM=1 -D'#$GLOBNUM=1' -check-prefixes CHECK,GLOBAL -input-file %s %s
-// RUN: FileCheck -D#LOCNUM=1 -D'#$GLOBNUM=1' -check-prefixes CHECK,LOCAL3 -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
-// RUN: not FileCheck -D#LOCNUM=1 -D#$GLOBNUM=1 -check-prefixes CHECK,LOCAL2 --enable-var-scope -input-file %s %s
-// RUN: not FileCheck -D#LOCNUM=1 -D#$GLOBNUM=1 -check-prefixes CHECK,LOCAL3 --enable-var-scope -input-file %s %s
+// RUN: FileCheck -input-file %s %s
+// RUN: FileCheck -check-prefixes CHECK,GLOBAL -input-file %s %s
+// RUN: FileCheck -check-prefixes CHECK,LOCAL3 -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
+// RUN: not FileCheck -check-prefixes CHECK,LOCAL2 --enable-var-scope -input-file %s %s
+// RUN: not FileCheck -check-prefixes CHECK,LOCAL3 --enable-var-scope -input-file %s %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
 
 local3
 global3
 ; LOCAL1: [[LOCAL]]3
 ; LOCAL2: local[[#LOCNUM+2]]
 ; LOCAL3: [[LOCAL]][[#LOCNUM+2]]
 ; GLOBAL: [[$GLOBAL]][[#$GLOBNUM+2]]

llvm/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 numeric expression "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 numeric expression "NUMVAR - 1" equal to "41"
+V-NEXT: verbose.txt:[[#@LINE-18]]:1: note: with numeric expression "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/unittests/Support/FileCheckTest.cpp

 #include "gtest/gtest.h"
 
 using namespace llvm;
 namespace {
 
 class FileCheckTest : public ::testing::Test {};
 
 TEST_F(FileCheckTest, NumVarValueGetClearSet) {
-  FileCheckNumVar FooVar = FileCheckNumVar("FOO", 42);
+  FileCheckNumVar FooVar = FileCheckNumVar("FOO", (uint64_t)42);
 
   // Defined variable: getValue returns a value, setValue fails.
   llvm::Optional<uint64_t> Value = FooVar.getValue();
   EXPECT_TRUE(Value);
   EXPECT_EQ((uint64_t)42, *Value);
   EXPECT_TRUE(FooVar.setValue((uint64_t)43));
 
   // 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((uint64_t)43));
   Value = FooVar.getValue();
   EXPECT_TRUE(Value);
   EXPECT_EQ((uint64_t)43, *Value);
 }
 
 uint64_t doAdd(uint64_t OpL, uint64_t OpR) { return OpL + OpR; }
 
 TEST_F(FileCheckTest, BinopEvalUndef) {
-  auto FooVar = new FileCheckNumVar("FOO", 42);
+  auto FooVar = new FileCheckNumVar("FOO", (uint64_t)42);
   auto Binop = new FileCheckASTBinop(doAdd, FooVar, 18);
 
   // Defined variable: eval returns right value, no undef variable returned.
   llvm::Optional<uint64_t> Value = Binop->eval();
   EXPECT_TRUE(Value);
   EXPECT_EQ((uint64_t)60, *Value);
   StringRef UndefVar = Binop->getUndefVarName();
   EXPECT_EQ("", UndefVar);
TEST_F(FileCheckTest, ParseVar) {
   VarName = "BadVar:";
   IsPseudo = true;
   TrailIdx = 0;
   EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
   EXPECT_FALSE(IsPseudo);
   EXPECT_EQ(TrailIdx, VarName.size() - 1);
 }
 
-class ExprTester {
+class PatternTester {
 private:
+  unsigned LineNumber = 1;
   SourceMgr SM;
   struct 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);
+    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] == '@';
-    StringRef NameTrailer = StringRef(VarName + Trailer);
+  void initNextPattern() {
+    P = FileCheckPattern(Check::CheckPlain, &Context, LineNumber++);
+  }
+
+private:
+  StringRef bufferize(StringRef Expr) {
     std::unique_ptr<MemoryBuffer> Buffer =
-        MemoryBuffer::getMemBufferCopy(NameTrailer, "TestBuffer");
-    StringRef NameTrailerRef = Buffer->getBuffer();
+        MemoryBuffer::getMemBufferCopy(Expr, "TestBuffer");
+    StringRef ExprBufferRef = Buffer->getBuffer();
     SM.AddNewSourceBuffer(std::move(Buffer), SMLoc());
-    StringRef VarNameRef = NameTrailerRef.substr(0, VarName.size());
-    StringRef TrailerRef = NameTrailerRef.substr(VarName.size());
-    return P.parseNumericExpression(VarNameRef, IsPseudo, TrailerRef, SM) ==
-           nullptr;
+    return ExprBufferRef;
+  }
+
+public:
+  bool parseNumVarExpect(StringRef Expr, bool isDefinition) {
+    StringRef ExprBufferRef = bufferize(Expr);
+    StringRef Name;
+    return P.parseNumericVariable(ExprBufferRef, Name, isDefinition, SM);
+  }
+
+  bool parseExprExpect(StringRef Expr) {
+    StringRef ExprBufferRef = bufferize(Expr);
+    FileCheckNumVar *NumVarDef;
+    return P.parseNumericExpression(ExprBufferRef, NumVarDef, SM) == nullptr;
+  }
+
+  bool parsePatternExpect(StringRef Pattern) {
+    StringRef PatBufferRef = bufferize(Pattern);
+    return P.ParsePattern(PatBufferRef, "CHECK", SM, Req);
+  }
+
+  bool matchExpect(StringRef Buffer) {
+    StringRef BufferRef = bufferize(Buffer);
+    size_t MatchLen;
+    return P.match(BufferRef, MatchLen, SM);
   }
 };
 
+TEST_F(FileCheckTest, ParseNumVar) {
+  PatternTester Tester;
+
+  // Invalid variable name.
+  EXPECT_TRUE(Tester.parseNumVarExpect("42INVALID", false /*isDefinition*/));
+
+  // Invalid pseudo variable.
+  EXPECT_TRUE(Tester.parseNumVarExpect("@FOO", false /*isDefinition*/));
+
+  // Valid pseudo variable.
+  EXPECT_FALSE(Tester.parseNumVarExpect("@LINE", false /*isDefinition*/));
+
+  // Invalid definition of pseudo.
+  EXPECT_TRUE(Tester.parseNumVarExpect("@LINE", true /*isDefinition*/));
+
+  // Conflict with pattern variable.
+  EXPECT_TRUE(Tester.parseNumVarExpect("BAR", true /*isDefinition*/));
+
+  // Undefined variable.
+  EXPECT_TRUE(Tester.parseNumVarExpect("UNDEF", false /*isDefinition*/));
+
+  // Use variable defined on same line.
+  EXPECT_FALSE(Tester.parseExprExpect("LINE1VAR:"));
+  EXPECT_TRUE(Tester.parseNumVarExpect("LINE1VAR", false /*isDefinition*/));
+
+  // Defined variable.
+  EXPECT_FALSE(Tester.parseNumVarExpect("FOO", false /*isDefinition*/));
+}
+
 TEST_F(FileCheckTest, ParseExpr) {
-  ExprTester Tester;
+  PatternTester Tester;
+
+  // Variable definition.
+
+  // Definition of invalid variable.
+  EXPECT_TRUE(Tester.parseExprExpect("10VAR:"));
+  EXPECT_TRUE(Tester.parseExprExpect("@FOO:"));
+  EXPECT_TRUE(Tester.parseExprExpect("@LINE:"));
+
+  // Garbage after name of variable being defined.
+  EXPECT_TRUE(Tester.parseExprExpect("VAR GARBAGE:"));
+
+  // Variable defined to numeric expression.
+  EXPECT_TRUE(Tester.parseExprExpect("VAR1: FOO"));
+
+  // Acceptable variable definition.
+  EXPECT_FALSE(Tester.parseExprExpect("VAR1:"));
+  EXPECT_FALSE(Tester.parseExprExpect("  VAR2:"));
+  EXPECT_FALSE(Tester.parseExprExpect("VAR3  :"));
+  EXPECT_FALSE(Tester.parseExprExpect("VAR3:  "));
+
+  // Numeric expression.
+
+  // Unacceptable variable.
+  EXPECT_TRUE(Tester.parseExprExpect("10VAR"));
+  EXPECT_TRUE(Tester.parseExprExpect("@FOO"));
+  EXPECT_TRUE(Tester.parseExprExpect("UNDEF"));
 
-  // @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));
-
-  // Defined variable
-  VarName = "FOO";
-  EXPECT_FALSE(Tester.parseExpect(VarName, Trailer));
-
-  // Undefined variable
-  VarName = "UNDEF";
-  EXPECT_TRUE(Tester.parseExpect(VarName, Trailer));
-
-  // Wrong Pseudovar.
-  VarName = "@FOO";
-  EXPECT_TRUE(Tester.parseExpect(VarName, Trailer));
+  // Only valid variable.
+  EXPECT_FALSE(Tester.parseExprExpect("@LINE"));
+  EXPECT_FALSE(Tester.parseExprExpect("FOO"));
 
   // Unsupported operator.
-  VarName = "@LINE";
-  Trailer = "/2";
-  EXPECT_TRUE(Tester.parseExpect(VarName, Trailer));
+  EXPECT_TRUE(Tester.parseExprExpect("@LINE/2"));
 
   // Missing offset operand.
-  VarName = "@LINE";
-  Trailer = "+";
-  EXPECT_TRUE(Tester.parseExpect(VarName, Trailer));
+  EXPECT_TRUE(Tester.parseExprExpect("@LINE+"));
 
   // Cannot parse offset operand.
-  VarName = "@LINE";
-  Trailer = "+x";
-  EXPECT_TRUE(Tester.parseExpect(VarName, Trailer));
+  EXPECT_TRUE(Tester.parseExprExpect("@LINE+x"));
 
   // Unexpected string at end of numeric expression.
-  VarName = "@LINE";
-  Trailer = "+5x";
-  EXPECT_TRUE(Tester.parseExpect(VarName, Trailer));
+  EXPECT_TRUE(Tester.parseExprExpect("@LINE+5x"));
+
+  // Valid expression.
+  EXPECT_FALSE(Tester.parseExprExpect("@LINE+5"));
+  EXPECT_FALSE(Tester.parseExprExpect("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 definition.
+  EXPECT_TRUE(Tester.parsePatternExpect("[[@PAT:]]"));
+  EXPECT_TRUE(Tester.parsePatternExpect("[[PAT+2:]]"));
+
+  // Collision with numeric variable.
+  EXPECT_TRUE(Tester.parsePatternExpect("[[FOO:]]"));
+
+  // Valid use.
+  EXPECT_FALSE(Tester.parsePatternExpect("[[BAR]]"));
+
+  // Valid 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.
+  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, Substitute) {
   SourceMgr SM;
   FileCheckPatternContext Context;
   std::vector<std::string> GlobalDefines;
   GlobalDefines.emplace_back(std::string("FOO=BAR"));
   Context.defineCmdlineVariables(GlobalDefines, SM);
 
   // Substitution of undefined pattern variable fails.
   FileCheckPatternSubst Subst = FileCheckPatternSubst(&Context, "VAR404", 42);
   EXPECT_FALSE(Subst.getSubstitute());
 
   // Substitution of defined numeric variable returns the right value.
-  auto LineVar = new FileCheckNumVar("@LINE", 42);
+  auto LineVar = new FileCheckNumVar("@LINE", (uint64_t)42);
   auto Binop = new FileCheckASTBinop(doAdd, LineVar, 0);
   FileCheckNumExpr NumExpr = FileCheckNumExpr(Binop);
   Subst = FileCheckPatternSubst(&Context, "@LINE", &NumExpr, 12);
   llvm::Optional<std::string> Value = Subst.getSubstitute();
   EXPECT_TRUE(Value);
   EXPECT_EQ("42", *Value);
 
   // Substitution of undefined numeric variable fails.
   LineVar->clearValue();
   EXPECT_FALSE(Subst.getSubstitute());
 
   // Substitution of defined pattern variable returns the right value.
-  FileCheckPattern P = FileCheckPattern(Check::CheckPlain, &Context);
+  FileCheckPattern P = FileCheckPattern(Check::CheckPlain, &Context, 1);
   Subst = FileCheckPatternSubst(&Context, "FOO", 42);
   Value = Subst.getSubstitute();
   EXPECT_TRUE(Value);
   EXPECT_EQ("BAR", *Value);
 }
 
 TEST_F(FileCheckTest, UndefVars) {
   SourceMgr SM;
TEST_F(FileCheckTest, UndefVars) {
 
   // Undef var in pattern variable substitution with defined variable is empty.
   Subst = FileCheckPatternSubst(&Context, "FOO", 42);
   UndefVar = Subst.getUndefVarName();
   EXPECT_EQ("", UndefVar);
 
   // Undef var in numeric expression substitution with defined variable is
   // empty.
-  auto LineVar = new FileCheckNumVar("@LINE", 42);
+  auto LineVar = new FileCheckNumVar("@LINE", (uint64_t)42);
   auto Binop = new FileCheckASTBinop(doAdd, LineVar, 0);
   FileCheckNumExpr NumExpr = FileCheckNumExpr(Binop);
   Subst = FileCheckPatternSubst(&Context, "@LINE", &NumExpr, 12);
   UndefVar = Subst.getUndefVarName();
   EXPECT_EQ("", UndefVar);
 
   // Undef var in valid numeric expression substitution is empty.
   // Undef var in numeric expression substitution with undefined variable