FileCheck [8/12]: Define numeric var from expr

[thopre]

Summary

This patch is part of a patch series to add support for FileCheck
numeric expressions. This specific patch lift the restriction for a
numeric expression to either be a variable definition or a numeric
expression to try to match.

This commit allows a numeric variable to be set to the result of the
evaluation of a numeric expression after it has been matched
successfully. When it happens, the variable is allowed to be used on
the same line since its value is known at match time.

It also makes use of this possibility to reuse the parsing code to
parse a command-line definition by crafting a mirror string of the
-D option with the equal sign replaced by a colon sign, e.g. for option
'-D#NUMVAL=10' it creates the string
'-D#NUMVAL=10 (parsed as #NUMVAL:10)' where the numeric expression
is parsed to define NUMVAL. This result in a few tests needing updating
for the location diagnostics on top of the tests for the new feature.

It also enables empty numeric expression which match any number without
defining a variable. This is done here rather than in commit #5 of the
patch series because it requires to dissociate automatic regex insertion
in RegExStr from variable definition which would make commit #5 even
bigger than it already is.

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]

Use camel case in new functions

[thopre]

Rebase

[thopre]

Make numeric variable constructor explicit

[thopre]

Rebase on latest changes

[thopre]

Rebase on latest changes

[thopre]

Run clang-format

[thopre]

Remove duplicate empty lines in numeric-expressions.txt

[thopre]

• Add checks in ParseExpr unit test to have same coverage as ParseNumericVariable
• Reorganize NumericVariableValueGetClearSet since variable can now be created undefined

[thopre]

Rebase on top of latest changes

[thopre]

Rebase on top of latest changes

[thopre]

Rebase on top of latest changes

[thopre]

Rebase on top of latest changes

[thopre]

Rebase on top of latest changes

[thopre]

Rebase on top of latest changes

[thopre]

Rebase on top of latest changes

[thopre]

rebase on top of latest changes

[thopre]

rebase on top of latest changes

[thopre]

Rebase on top of latest changes in patch series

[thopre]

Rebase on top of latest changes in the series

[thopre]

Rebase on latest changes in the patch series

[thopre]

Rebase on top of latest changes in the patch series

[thopre]

Rebase

[thopre]

Fix style for function parameter immediates

[arichardson]

llvm/include/llvm/Support/FileCheck.h

145

I would call this isKnownAtMatchTime() or isValueKnownAtMatchTime(). To me isMatchTimeKnown() sounds like "is the time of the match known?" which obviously doesn't make any sense in this context but the longer name means I know what the function does without find the documentation comment.

505

"if defining such a variable is invalid" slightly easier to parse for me.

521

Here and in the other functions: maybe use Optional<size_t> instead of a magic value of zero for line numbers?
I don't have a strong opinion here, I just think it might make it more obvious that the line number can also not be a line number but rather "this came from the command line".

llvm/lib/Support/FileCheck.cpp

45

I think !ExpressionAST is the more commonly used style.

[thopre]

• Rebase on latest changes in the patch series
• Address most comments

llvm/include/llvm/Support/FileCheck.h

521

I'll schedule a patch to that effect before this one since changing this might touch some area not currently touched by this patch. Keeping your comment open for now, will close it once I've added the patch to the patch series.

[thopre]

Rebase on top of latest changes in patch series

[thopre]

Add more unit tests

[jhenderson]

llvm/docs/CommandGuide/FileCheck.rst

621

If I'm not mistaken, the %<fmtspec> bit isn't implemented in this patch?

llvm/include/llvm/Support/FileCheck.h

113

Add to this comment what the value of the variable is if ExpressionAST is null.

llvm/lib/Support/FileCheck.cpp

28

This is unchecked in builds without assertions.

29–31

I'm not sure this second sentence is necessary?

30

I can't parse this string here. I can't even really figure out what this means to suggest an alternative.

551

You don't need the explicit StringRef(...) here. Just do MatchRegexp = "[0-9]+";

602

DefineCmdlineVariables doesn't conform to style guide. I haven't bothered looking it up, but I imagine this comment is a bit stale, or the function needs renaming?

1764

Suffix3? What happened to Suffix2?

Honestly, these names don't make a huge amount of sense to me.

Also, do we need all of them? As far as I can see, Suffix1 and Suffix3 are only used once each, so should just be inlined.

1767

DefNo -> DefIndex

("No" here is not sufficiently clear as to its context, IMO).

1777

Should this be parseNumericSubstitutionBlock?

1808

This comment sounds like a lie? You aren't skipping the error here...

1814–1823

"Parse the definition both to check that the syntax is correct and to create ..."

1827

Why is the expression being evaluated here? This probably needs a comment.

[thopre]

Address review comments

[thopre]

llvm/lib/Support/FileCheck.cpp

28

That's fine. setValue is called after substitution has happened and match was successful. Therefore ExpressionAST ought to evaluate fine. This is just in case code is reworked in the future and this invariant is broken.

[jhenderson]

llvm/lib/Support/FileCheck.cpp

28

It isn't. LLVM Errors and Expecteds HAVE to be checked. If they aren't it will lead to an abort. From the Programmer's Manual:

All Error instances, whether success or failure, must be either checked or moved from (via std::move or a return) before they are destructed. Accidentally discarding an unchecked error will cause a program abort at the point where the unchecked value’s destructor is run, making it easy to identify and fix violations of this rule.

[thopre]

Address review comments

llvm/lib/Support/FileCheck.cpp

28

Oh my bad, didn't make the connection with the Expected type. I think an abort would be fine in this case since that error should never occur in any of the executions. By that I mean that code path should be dead (that function will not be called if the expression does not evaluate). If it does, it's a bug somewhere in the code.

Does that make sense? I've added some comments on the expectation for the caller both in the declaration and just on top of the assert since that was missing.

[probinson]

llvm/docs/CommandGuide/FileCheck.rst

621

Where is <constraint> described?

llvm/lib/Support/FileCheck.cpp

31

You should check the Expected return value with something other than an assert. It is not sufficient to claim that the caller should call this only when the Expected result must be success. The contract for using Expected is that you will check the result. You could do something like

if (!EvaluatedValue || *EvaluatedValue != NewValue)
  llvm_unreachable("New value does not match expression for this variable");

484

? this should be the default, you should not have to say = StringRef() explicitly.

[jhenderson]

FTR, I haven't done a rigorous check of all the different new code paths to ensure you've got testing for them all, I'm afraid - the change is too large to sensibly do that, although I accept that it probably needs to be this large.

llvm/lib/Support/FileCheck.cpp

31

I think cantFail is intended for the kind of situation we're in here.

33

Isn't the assertion that the eval() function succeeded. In that case, let's talk about that. "Failed to evaluate expression when sanity checking variable value" or something to that effect.

1765

Won't this create a StringRef to a temporary?

1773

Append a copy of the command-line...

1784

Prefix1 + DefIndex + Prefix2 is used twice, once in each part of the if. If you factor it out into a separate variable, it'll be much easier to read, I think, and you can then get rid of Prefix1 and Prefix2 as separate variables. You can even fold them into the place where DefIndex is created, to avoid additional non-Twine string concatenations:

std::string SomeName = ("Global define #" + Twine(++I) + ": ").str();

1827

"Now evaluate the expression, whose value this variable should be set to, since the expression of a command-line variable definition should only use variables defined earlier on the command-line."

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

5

By the way, if you were to use --match-full-lines, you wouldn't need to use the regular expression in the last line of each of these checks, and your arrow would point closer to where it should be.

llvm/test/FileCheck/numeric-defines.txt

4–9

There should probably be a test case like the original, i.e. with no expression, just a literal numeric value.

llvm/test/FileCheck/numeric-expression.txt

77

This is allowed? That's surprising to me, although I suppose it's a natural consequence of the rules.

138

"when a variable"

140

Double dashes here, please.

Is it worth checking the error message in this context, to show that the failure is due to the expression match rather than due to something to do with the definition?

llvm/unittests/Support/FileCheckTest.cpp

57

Perhaps "Variable defined by numeric expression:" would make more sense.

60

"UseUnique" doesn't seem to line up with what this test is supposed to be doing, from what I can tell. What is "Unique" about it?

66

FoobarVar is the "undefined variable being set", right? In that case, I think a more purposeful name might be useful, e.g. FromVarExprVar or something like that. Happy with alternative names though.

280

I might be being dumb here, but where does the "same line" come into the following 4 EXPECTs?

343

How about EXPECT_FALSE/TRUE(Test.matchExpect(""));?

543

There are probably many other places where this is a problem already, but this should be ASSERT_TRUE, since the test cannot continue if it fails, and will crash on the next line (probably). Perhaps a separate commit to clean those up would be appropriate.

[thopre]

• Address review comments
• rebase on top of @LINE substitution fix

[jhenderson]

llvm/lib/Support/FileCheck.cpp

29

clang-format?

Rephrase the second string to "Value being set to different from variable evaluation"

1827

This comment hasn't been addressed.

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

5

This has been marked as Done, but I don't see any changes?

llvm/unittests/Support/FileCheckTest.cpp

57

I wanted the "Undefined" bit deleting. I don't think it's needed.

[thopre]

llvm/lib/Support/FileCheck.cpp

1827

I'm confused by what aspect of this you want to be explained. The reworded text seems to explain why is the expression evaluated but I thought you were asking why is it evaluated *here*. If the former, the text should be reworded further to remove the mention about errors and only say that the value of a variable can be given by an expression using variable earlier on the command line. If the latter, then the "only" becomes important since we are saying both that it is safe to evaluate now (we have the value of all variables) and that we should be doing it now (there is no input to match, all variables used are defined either from earlier command-line or from literal values).

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

5

Rather further away since the previous line would still need {{ and escaping for [ while the caret check would not need anything. It does reduce the amount of syntax for that line but it also make other lines slightly more difficult to read since I have to remote the space after the colon due to the --strict-whitespace:

NUMERRCLIFMT:Global defines:1:46: error: invalid variable name

Do you think it's worth doing? I can schedule a separate patch if yes.

llvm/test/FileCheck/numeric-expression.txt

77

As you said it's a natural consequence and I thought it could actually be useful (to check you have a number in an error message but don't care about the value) so I didn't forbid it.

llvm/unittests/Support/FileCheckTest.cpp

60

There is already a FooVarUse of type FileCheckNumericVariableUse. This one is a unique_ptr of that type. I've renamed into FooVarUsePtr.

543

Wasn't aware of ASSERT_TRUE. Another patch to schedule.

[thopre]

llvm/lib/Support/FileCheck.cpp

1827

Sorry, forgot to submit my replies. Done marks automatically get pushed when submitting a new revision.

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

5

Marked it done because I wrote a reply and was expecting an answer about whether you think it is actually worth it. In the event you think it is indeed worth it, I would only mark the answer as done once the work has been completed.

Unfortunately I forgot to submit my replies.

[thopre]

Fix more review comments

[jhenderson]

llvm/lib/Support/FileCheck.cpp

1827

The message content is fine as it stands in the current version. I've just suggested some grammar improvements. I think the meaning remains the same with them.

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

5

I guess it's not worth it. I just find the whole caret checking basically unreadable due to the regex as it stands. It feels quite fragile too.

Since you've got --strict-whitespace on, you could simplify the caret's line a little:
NUMERRCLIFMT-NEXT: {{^}} ^
(with appropriate numbers of spaces).

I don't think the trailing $ is important, really, in this case, but could be persuaded either way. In that case though, I'd put that in it's own {{$}} pattern. If you are happy with that, then please schedule a patch for that.

[probinson]

llvm/test/FileCheck/numeric-expression.txt

77

So [[#]] is equivalent to {{[0-9]+}} ? Might be worth a note in the documentation.

[thopre]

Address remaining review comments

[jhenderson]

LGTM, I think. You'd best get someone else (e.g. @probinson) to confirm too.

[probinson]

A couple of minor things, otherwise LGTM.

llvm/docs/CommandGuide/FileCheck.rst

627–629

constraint?

llvm/include/llvm/Support/FileCheck.h

114

"it is set from a value in the input" ? This is not a statement about what the ctor does, it sounds like an expectation.
"Otherwise, it should be set later from a value in the input." ?

[thopre]

• Remove reference to expression constraint, this is patch 11/12 material
• Clarify what the FileCheckNumericVariable constructor does and what is rather an expectation

[craig.topper]

llvm/trunk/lib/Support/FileCheck.cpp

345 ↗

(On Diff #211365)

gcc 7.3.1 seems to be unhappy with this line. I think it wants a std::move around ExpressionAST.

llvm/docs/CommandGuide/FileCheck.rst

.. option:: --enable-var-scope
 
   All other variables get undefined after each encountered ``CHECK-LABEL``.
 
 .. option:: -D<VAR=VALUE>
 
   Sets a filecheck pattern variable ``VAR`` with value ``VALUE`` that can be
   used in ``CHECK:`` lines.
 
-.. option:: -D#<NUMVAR>=<VALUE EXPRESSION>
+.. option:: -D#<NUMVAR>=<NUMERIC EXPRESSION>
 
   Sets a filecheck numeric variable ``NUMVAR`` to the result of evaluating
-  ``<VALUE EXPRESSION>`` that can be used in ``CHECK:`` lines. See section
-  ``FileCheck Numeric Variables and Expressions`` for details on the format
-  and meaning of ``<VALUE EXPRESSION>``.
+  ``<NUMERIC EXPRESSION>`` that can be used in ``CHECK:`` lines. See section
+  ``FileCheck Numeric Variables and Expressions`` for details on supported
+  numeric expressions.
 
 .. option:: -version
 
  Show the version number of this program.
 
 .. option:: -v
 
   Print good directive pattern matches.  However, if ``-input-dump=fail`` or

 but would not match the line:
 
 .. code-block:: llvm
 
     add r5, r5, r7
 
 due to ``7`` being unequal to ``5 + 1``.
 
+A numeric variable can also be defined to the result of a numeric expression,
+in which case the numeric expression constraint is checked and if verified the
+variable is assigned to the value. The unified syntax for both defining numeric
+variables and checking a numeric expression is thus
+``[[#%<fmtspec>,<NUMVAR>: <constraint> <expr>]]`` with each element as
+described previously.
+
 The ``--enable-var-scope`` option has the same effect on numeric variables as
 on pattern variables.
 
 Important note: In its current implementation, a numeric expression cannot use
-a numeric variable defined on the same line.
+a numeric variable with a non-empty numeric expression constraint defined on
+the same line.
 
 FileCheck Pseudo Numeric Variables
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 Sometimes there's a need to verify output that contains line numbers of the
 match file, e.g. when testing compiler diagnostics.  This introduces a certain
 fragility of the match file structure, as "``CHECK:``" lines contain absolute
 line numbers in the same file, which have to be updated whenever line numbers

llvm/include/llvm/Support/FileCheck.h

 /// 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 FileCheckNumericVariable : public FileCheckNumExprAST {
 private:
   /// Name of the numeric variable.
   StringRef Name;
 
+  /// Pointer to numeric expression defining this numeric variable. Null for
+  /// pseudo variable whose value is known at parse time (e.g. @LINE pseudo
+  /// variable) or cleared local variable. If numeric expression is empty
+  /// NumExpr points to a FileCheckNumExpr with a null AST.
+  FileCheckNumExpr *NumExpr;
+
   /// Value of numeric variable, if defined, or None otherwise.
   llvm::Optional<uint64_t> Value;
 
   /// Line number where this variable is defined. Used to determine whether a
   /// variable is defined on the same line as a given use.
   unsigned DefLineNumber;
 
 public:
-  /// Constructor for a variable \p Name defined at line \p DefLineNumber.
-  FileCheckNumericVariable(StringRef Name, unsigned DefLineNumber)
-      : Name(Name), Value(llvm::None), DefLineNumber(DefLineNumber) {}
+  /// Constructor for a variable \p Name defined at line \p DefLineNumber to
+  /// the numeric expression represented by NumExpr.
+  FileCheckNumericVariable(StringRef Name, FileCheckNumExpr *NumExpr,
+                           unsigned DefLineNumber)
+      : Name(Name), NumExpr(NumExpr), Value(llvm::None),
+        DefLineNumber(DefLineNumber) {}
 
   /// Constructor for numeric variable \p Name with a known \p Value at parse
   /// time (e.g. the @LINE numeric variable).
-  FileCheckNumericVariable(StringRef Name, uint64_t Value)
-      : Name(Name), Value(Value) {}
+  explicit FileCheckNumericVariable(StringRef Name, uint64_t Value)
+      : Name(Name), NumExpr(nullptr), Value(Value) {}
 
   /// \returns name of that numeric variable.
   StringRef getName() const { return Name; }
 
+  /// \returns numeric expression associated with this numeric variable.
+  FileCheckNumExpr *getNumExpr() const { return NumExpr; }
+
   /// Evaluates and returns the value of the expression represented by this
-  /// AST. Therefore, \returns this variable's value.
-  llvm::Optional<uint64_t> eval() const { return Value; }
+  /// AST. Therefore, \returns this variable's value or the value of its
+  /// associated numeric expression, if any.
+  llvm::Optional<uint64_t> eval() const;
+
+  /// \returns whether this variable's value is known at match time, when
+  /// performing the substitutions.
+  bool isMatchTimeKnown() const;
 
   /// Appends numeric variable's name to UndefVarNames if undefined.
   void appendUndefVarNames(std::vector<StringRef> &UndefVarNames) const;
 
   /// Sets value of this numeric variable if not defined. \returns whether the
   /// variable was already defined.
   bool setValue(uint64_t Value);
 
   /// Clears value of this numeric variable. \returns whether the variable was
   /// already undefined.
   bool clearValue();
 
   /// \returns the line number where this variable is defined.
   unsigned getDefLineNumber() { return DefLineNumber; }
 };
public:
   /// \returns whether \p C is a valid first character for a variable name.
   static bool isValidVarNameStart(char C);
   /// Verifies that the string at the start of \p Str is a well formed
   /// variable. \returns false if it is and sets \p IsPseudo to indicate if it
   /// is a pseudo variable and \p TrailIdx to the position of the last
   /// character that is part of the variable name. Otherwise, only
   /// \returns true.
   static bool parseVariable(StringRef Str, bool &IsPseudo, unsigned &TrailIdx);
-  /// Parses \p Expr for use or definition (if \p IsDefinition is true) of a
-  /// numeric variable. \returns whether parsing fails in which case errors are
-  /// reported on \p SM, unless \p AcceptFail is true and the error is in
-  /// parsing the variable name. Otherwise, sets \p Name to the name of the
-  /// parsed numeric variable.
-  bool parseNumericVariable(StringRef &Expr, StringRef &Name, bool IsDefinition,
-                            bool AcceptFail, const SourceMgr &SM) const;
   /// Parses \p Expr for a numeric expression. Parameter \p Legacy indicates
   /// whether Expr should be a legacy numeric expression. \p returns the class
   /// representing the AST of numeric expression or nullptr if parsing fails
   /// in which case errors are reported on \p SM. Sets
   /// \p DefinedNumericVariable to the pointer to the class representing the
   /// variable defined to this numeric expression, if any.
   FileCheckNumExpr *
   parseNumericExpression(StringRef Expr,
private:
   ///
   /// \p Str has to point in the beginning of the definition (right after the
   /// opening sequence). \p SM holds the SourceMgr used for error repporting.
   ///  \returns the offset of the closing sequence within Str, or npos if it
   /// was not found.
   size_t FindRegexVarEnd(StringRef Str, SourceMgr &SM);
 
   enum AllowedOperand { LegacyVar, LegacyLiteral, Any };
+  /// Parses \p Expr for use or definition (if \p IsDefinition is true) of a
+  /// numeric variable. \returns whether parsing fails in which case errors are
+  /// reported on \p SM, unless \p AcceptFail is true and the error is in
+  /// parsing the variable name. Otherwise, sets \p Name to the name of the
+  /// parsed numeric variable.
+  bool parseNumericVariable(StringRef &Expr, StringRef &Name, bool IsDefinition,
+                            bool AcceptFail, const SourceMgr &SM) const;
   /// Parses \p Expr for use of a numeric operand. Accepts both literal values
   /// and numeric variables, depending on the value of \p AllowedOperandFlag.
   /// \returns the class representing that operand in the AST of the numeric
   /// expression or nullptr if parsing fails in which case errors are reported
   /// on \p SM.
   std::shared_ptr<FileCheckNumExprAST>
   parseNumericOperand(StringRef &Expr, enum AllowedOperand AO,
                       const SourceMgr &SM) const;
   /// Parses \p Expr for a binary operation. The left operand of this binary
   /// operation is given in \p LeftOp and \p Legacy indicates whether we are
   /// parsing a legacy numeric expression. \returns the class representing this
   /// binary operation in the AST of the numeric expression or nullptr if
   /// parsing fails in which case errors are reported on \p SM.
   std::shared_ptr<FileCheckNumExprAST>
   parseFileCheckBinop(StringRef &Expr,
                       std::shared_ptr<FileCheckNumExprAST> LeftOp, bool Legacy,
                       const SourceMgr &SM) const;
 };
 
 //===----------------------------------------------------------------------===//
 /// Summary of a FileCheck diagnostic.
 //===----------------------------------------------------------------------===//
 
 struct FileCheckDiag {
   /// What is the FileCheck directive for this diagnostic?
   Check::FileCheckType CheckTy;

llvm/lib/Support/FileCheck.cpp

 #include <cstdint>
 #include <list>
 #include <map>
 #include <tuple>
 #include <utility>
 
 using namespace llvm;
 
+llvm::Optional<uint64_t> FileCheckNumericVariable::eval() const {
+  if (Value)
+    return Value;
+
+  if (NumExpr == nullptr || NumExpr->getAST() == nullptr)
+    return llvm::None;
+
+  return NumExpr->getAST()->eval();
+}
+
+bool FileCheckNumericVariable::isMatchTimeKnown() const {
+  if (Value)
+    return true;
+
+  return NumExpr != nullptr && NumExpr->getAST() != nullptr;
+}
+
 void FileCheckNumericVariable::appendUndefVarNames(
     std::vector<StringRef> &UndefVarNames) const {
   if (!Value)
     UndefVarNames.emplace_back(Name);
 }
 
 bool FileCheckNumericVariable::setValue(uint64_t NewValue) {
   if (Value)
     return true;
   Value = NewValue;
   return false;
 }
 
 bool FileCheckNumericVariable::clearValue() {
   if (!Value)
     return true;
   Value = llvm::None;
+  NumExpr = nullptr;
   return false;
 }
 
 llvm::Optional<uint64_t> FileCheckASTBinop::eval() const {
   llvm::Optional<uint64_t> LeftOp = this->LeftOp->eval();
   llvm::Optional<uint64_t> RightOp = this->RightOp->eval();
 
   // Uses undefined variable.
bool FileCheckPattern::parseNumericVariable(StringRef &Expr, StringRef &Name,
   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 true;
   }
 
   FileCheckNumericVariable *NumericVariable = VarTableIter->second.get();
-  if (!IsPseudo && NumericVariable->getDefLineNumber() == LineNumber) {
+  if (!IsPseudo && NumericVariable->getDefLineNumber() == LineNumber &&
+      !NumericVariable->isMatchTimeKnown()) {
     SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
                     "numeric variable '" + NumericVariable->getName() +
                         "' defined on the same line");
     return true;
   }
 
   return false;
 }
std::shared_ptr<FileCheckNumExprAST> FileCheckPattern::parseFileCheckBinop(
   Expr = Expr.ltrim(SpaceChars);
   return std::make_shared<FileCheckASTBinop>(EvalBinop, LeftOp, RightOp);
 }
 
 FileCheckNumExpr *FileCheckPattern::parseNumericExpression(
     StringRef Expr, FileCheckNumericVariable *&DefinedNumericVariable,
     bool Legacy, const SourceMgr &SM) const {
   std::shared_ptr<FileCheckNumExprAST> NumExprAST;
+  StringRef DefExpr = StringRef();
 
-  // Parse numeric variable definition.
   DefinedNumericVariable = nullptr;
+  // Save variable definition expression if any.
   size_t DefEnd = Expr.find(':');
   if (DefEnd != StringRef::npos) {
-    StringRef DefExpr = Expr.substr(0, DefEnd);
-    StringRef UseExpr = Expr = Expr.substr(DefEnd + 1);
-
-    DefExpr = DefExpr.ltrim(SpaceChars);
-    StringRef Name;
-    if (parseNumericVariable(DefExpr, Name, true /*IsDefinition*/,
-                             false /*AcceptFail*/, SM)) {
-      // Invalid variable definition. Error reporting done in parsing function.
-      return nullptr;
-    }
+    DefExpr = Expr.substr(0, DefEnd);
+    Expr = Expr.substr(DefEnd + 1);
 
-    DefinedNumericVariable =
-        new FileCheckNumericVariable(Name, this->LineNumber);
-
-    DefExpr = DefExpr.ltrim(SpaceChars);
-    if (!DefExpr.empty()) {
-      SM.PrintMessage(SMLoc::getFromPointer(DefExpr.data()),
-                      SourceMgr::DK_Error,
-                      "invalid numeric variable definition");
-      return nullptr;
-    }
-    UseExpr = UseExpr.ltrim(SpaceChars);
-    if (!UseExpr.empty()) {
-      SM.PrintMessage(
-          SMLoc::getFromPointer(UseExpr.data()), SourceMgr::DK_Error,
-          "unexpected string after variable definition: '" + UseExpr + "'");
-      return nullptr;
-    }
-  } else {
-    // Parse numeric expression itself.
-    Expr = Expr.ltrim(SpaceChars);
+  if (!Expr.empty()) {
     // First operand in legacy numeric expression is the @LINE pseudo variable.
     enum AllowedOperand AO = Legacy ? LegacyVar : Any;
     NumExprAST = parseNumericOperand(Expr, AO, SM);
     while (NumExprAST != nullptr && !Expr.empty()) {
       NumExprAST = parseFileCheckBinop(Expr, NumExprAST, Legacy, SM);
       // Legacy numeric expressions only allow 2 operands.
       if (Legacy && !Expr.empty()) {
         SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
                         "unexpected characters at end of numeric expression '" +
                             Expr + "'");
         return nullptr;
       }
     }
     if (NumExprAST == nullptr) {
       SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
                       "invalid operand format '" + Expr + "'");
       return nullptr;
     }
   }
 
-  return Context->makeNumExpr(NumExprAST);
+  FileCheckNumExpr *NumExpr = Context->makeNumExpr(NumExprAST);
+
+  // Parse numeric variable definition.
+  if (!DefExpr.empty()) {
+    DefExpr = DefExpr.ltrim(SpaceChars);
+    StringRef Name;
+    if (parseNumericVariable(DefExpr, Name, true /*IsDefinition*/,
+                             false /*AcceptFail*/, SM)) {
+      // Invalid variable definition. Error reporting done in parsing function.
+      return nullptr;
+    }
+
+    DefinedNumericVariable =
+        new FileCheckNumericVariable(Name, NumExpr, this->LineNumber);
+
+    DefExpr = DefExpr.ltrim(SpaceChars);
+    if (!DefExpr.empty()) {
+      SM.PrintMessage(SMLoc::getFromPointer(DefExpr.data()),
+                      SourceMgr::DK_Error,
+                      "invalid numeric variable definition");
+      return nullptr;
+    }
+  }
+
+  return NumExpr;
 }
 
 bool FileCheckPattern::ParsePattern(StringRef PatternStr, StringRef Prefix,
                                     SourceMgr &SM,
                                     const FileCheckRequest &Req) {
   bool MatchFullLinesHere = Req.MatchFullLines && CheckTy != Check::CheckNot;
 
   PatternLoc = SMLoc::getFromPointer(PatternStr.data());
if (PatternStr.startswith("{{")) {
       PatternStr = PatternStr.substr(End + 2);
       continue;
     }
 
     // Pattern and numeric expression matches. Pattern expressions come in two
     // forms: [[foo:.*]] and [[foo]]. The former matches .* (or some other
     // regex) and assigns it to the FileCheck variable 'foo'. The latter
     // substitutes foo's value. Numeric expressions start with a '#' sign after
-    // the double brackets and also have the definition and substitution forms.
-    // Both pattern and numeric variables must satisfy the regular expression
-    // "[a-zA-Z_][0-9a-zA-Z_]*" to be valid, as this helps catch some common
-    // errors.
+    // the double brackets and also have the definition and substitution forms
+    // and the two forms can be combined to set a variable to the evaluation of
+    // a numeric expression. Both pattern and numeric variables must satisfy
+    // the regular expression "[a-zA-Z_][0-9a-zA-Z_]*" to be valid, as this
+    // helps catch some common errors.
     if (PatternStr.startswith("[[")) {
       StringRef UnparsedPatternStr = PatternStr.substr(2);
       // Find the closing bracket pair ending the match.  End is going to be an
       // offset relative to the beginning of the match string.
       size_t End = FindRegexVarEnd(UnparsedPatternStr, SM);
       StringRef MatchStr = UnparsedPatternStr.substr(0, End);
       bool IsNumExpr = MatchStr.consume_front("#");
       const char *RefTypeStr =
           IsNumExpr ? "numeric expression" : "pattern variable";
 
       if (End == StringRef::npos) {
         SM.PrintMessage(
             SMLoc::getFromPointer(PatternStr.data()), SourceMgr::DK_Error,
             Twine("Invalid ") + RefTypeStr + " reference, no ]] found");
         return true;
       }
       // Strip the subtitution we are parsing. End points to the start of the
       // "]]" closing the expression so account for it in computing the index
       // of the first unparsed character.
       PatternStr = UnparsedPatternStr.substr(End + 2);
 
       bool IsVarDef;
+      bool SubstNeeded;
       bool Legacy = false;
       StringRef DefName;
       StringRef SubstStr;
-      StringRef MatchRegexp;
+      StringRef MatchRegexp = StringRef();
       size_t SubstInsertIdx = RegExStr.size();
       FileCheckNumericVariable *DefinedNumericVariable;
       FileCheckNumExpr *NumExpr;
 
       // Parse pattern variable or legacy numeric expression.
       if (!IsNumExpr) {
         size_t VarEndIdx = MatchStr.find(":");
         size_t SpacePos = MatchStr.substr(0, VarEndIdx).find_first_of(" \t");
if (PatternStr.startswith("[[")) {
                           SourceMgr::DK_Error, "invalid variable name");
           return true;
         }
 
         StringRef Name = MatchStr.substr(0, TrailIdx);
         StringRef Trailer = MatchStr.substr(TrailIdx);
         IsVarDef = (VarEndIdx != StringRef::npos);
         Legacy = IsNumExpr = IsPseudo;
+        SubstNeeded = !IsVarDef;
 
         if (IsVarDef) {
           if ((IsPseudo || !Trailer.consume_front(":"))) {
             SM.PrintMessage(SMLoc::getFromPointer(MatchStr.data()),
                             SourceMgr::DK_Error,
                             "invalid name in pattern variable definition");
             return true;
           }
if (PatternStr.startswith("[[")) {
 
       // Parse numeric expression.
       if (IsNumExpr) {
         NumExpr = parseNumericExpression(MatchStr, DefinedNumericVariable,
                                          Legacy, SM);
         if (NumExpr == nullptr)
           return true;
         IsNumExpr = true;
-        if (DefinedNumericVariable != nullptr) {
-          IsVarDef = true;
+        IsVarDef = DefinedNumericVariable != nullptr;
+        SubstNeeded = NumExpr->getAST() != nullptr;
+        if (IsVarDef)
           DefName = DefinedNumericVariable->getName();
-          MatchRegexp = StringRef("[0-9]+");
-        } else
+        if (SubstNeeded)
           SubstStr = MatchStr;
+        else
+          MatchRegexp = StringRef("[0-9]+");
       }
 
+      // Handle variable definition: [[<def>:(...)]] and [[#(...)<def>:(...)]].
+      if (IsVarDef) {
+        RegExStr += '(';
+        ++SubstInsertIdx;
+
+        if (IsNumExpr) {
+          struct FileCheckNumExprMatch NumExprDef = {DefinedNumericVariable,
+                                                     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] =
+              std::shared_ptr<FileCheckNumericVariable>(DefinedNumericVariable);
+        } else {
+          VariableDefs[DefName] = CurParen;
+          // Mark pattern variable as defined to detect collision between
+          // pattern and numeric variables 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;
+        }
+
+        ++CurParen;
+      }
+
+      if (!MatchRegexp.empty() && AddRegExToRegEx(MatchRegexp, CurParen, SM))
+        return true;
+
+      if (IsVarDef)
+        RegExStr += ')';
+
       // Handle variable use: [[foo]] and [[#<foo expr>]].
-      if (!IsVarDef) {
+      if (SubstNeeded) {
         // Handle use of pattern variables that were defined earlier on the
         // 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(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.
           FileCheckPatternSubstitution Substitution =
               IsNumExpr ? FileCheckPatternSubstitution(Context, SubstStr,
                                                        NumExpr, SubstInsertIdx)
                         : FileCheckPatternSubstitution(Context, SubstStr,
                                                        SubstInsertIdx);
           Substitutions.push_back(Substitution);
         }
-        continue;
       }
-
-      // Handle variable definition: [[<def>:(...)]] and [[#(...)<def>:(...)]].
-      if (IsNumExpr) {
-        struct FileCheckNumExprMatch NumExprDef = {DefinedNumericVariable,
-                                                   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] =
-            std::shared_ptr<FileCheckNumericVariable>(DefinedNumericVariable);
-      } else {
-        VariableDefs[DefName] = CurParen;
-        // Mark pattern variable as defined to detect collision between pattern
-        // 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(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("{{");
     FixedMatchEnd = std::min(FixedMatchEnd, PatternStr.find("[["));
     RegExStr += Regex::escape(PatternStr.substr(0, FixedMatchEnd));
     PatternStr = PatternStr.substr(FixedMatchEnd);
bool FileCheckPatternContext::defineCmdlineVariables(
   // Create a string representing the vector of command-line definitions. Each
   // definition is on its own line and prefixed with a definition number to
   // clarify which definition a given diagnostic corresponds to.
   unsigned I = 0;
   bool ErrorFound = false;
   std::string CmdlineDefsDiag;
   StringRef Prefix1 = "Global define #";
   StringRef Prefix2 = ": ";
-  for (StringRef CmdlineDef : CmdlineDefines)
+  StringRef Suffix1 = " (parsed as: [[";
+  StringRef Suffix3 = "]])";
+  SmallVector<std::pair<size_t, size_t>, 4> CmdlineDefsIndices;
+  for (StringRef CmdlineDef : CmdlineDefines) {
+    StringRef DefNo = Twine(++I).str();
+    size_t EqIdx = CmdlineDef.find('=');
+    if (EqIdx == StringRef::npos) {
+      llvm::outs() << "Missing equal sign in global definition '" << CmdlineDef
+                   << "'";
+      CmdlineDefsIndices.push_back(std::make_pair(0, 0));
+      ErrorFound = true;
+      continue;
+    }
+    // Numeric variable definition.
+    if (CmdlineDef[0] == '#') {
+      // Append a copy command-line definition adapted to use the same format
+      // as in the input file to be able to reuse ParseNumericExpression.
       CmdlineDefsDiag +=
-        (Prefix1 + Twine(++I) + Prefix2 + CmdlineDef + "\n").str();
+          (Prefix1 + DefNo + Prefix2 + CmdlineDef + Suffix1).str();
+      std::string Suffix2 = CmdlineDef;
+      Suffix2[EqIdx] = ':';
+      CmdlineDefsIndices.push_back(
+          std::make_pair(CmdlineDefsDiag.size(), Suffix2.size()));
+      CmdlineDefsDiag += (Suffix2 + Suffix3 + "\n").str();
+    } else {
+      CmdlineDefsDiag += (Prefix1 + DefNo + Prefix2).str();
+      CmdlineDefsIndices.push_back(
+          std::make_pair(CmdlineDefsDiag.size(), CmdlineDef.size()));
+      CmdlineDefsDiag += (CmdlineDef + "\n").str();
+    }
+  }
 
   // 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.
+  // Dummy pattern to call parseNumericExpression.
   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);
-    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;
+  for (std::pair<size_t, size_t> CmdlineDefIndices : CmdlineDefsIndices) {
+    StringRef CmdlineDef = CmdlineDefsDiagRef.substr(CmdlineDefIndices.first,
+                                                     CmdlineDefIndices.second);
+    // Error already handled in previous loop, skip this.
+    if (CmdlineDef.empty())
       continue;
-    }
 
     // Numeric variable definition.
     if (CmdlineDef[0] == '#') {
-      StringRef Expr = CmdlineDef.substr(1, EqIdx - 1);
-      StringRef CmdlineName;
-      SMLoc CmdlineNameLoc = SMLoc::getFromPointer(Expr.data());
-      if (P.parseNumericVariable(Expr, CmdlineName, true /*IsDefinition*/,
-                                 false /*AcceptFail*/, SM) ||
-          !Expr.empty()) {
-        SM.PrintMessage(CmdlineNameLoc, SourceMgr::DK_Error,
-                        "invalid variable name");
-        ErrorFound = true;
-        continue;
-      }
-
-      // Detect collisions between pattern and numeric variables when the
-      // latter is created later than the former.
-      if (DefinedVariableTable.find(CmdlineName) !=
-          DefinedVariableTable.end()) {
-        SM.PrintMessage(
-            SMLoc::getFromPointer(CmdlineName.data()), SourceMgr::DK_Error,
-            "pattern variable with name '" + CmdlineName + "' already exists");
+      // Now parse to both check syntax is correct and create the necessary
+      // class instance.
+      StringRef CmdlineDefExpr = CmdlineDef.substr(1);
+      FileCheckNumericVariable *DefinedNumericVariable;
+      FileCheckNumExpr *NumExpr = P.parseNumericExpression(
+          CmdlineDefExpr, DefinedNumericVariable, false, SM);
+      if (NumExpr == nullptr || DefinedNumericVariable == nullptr) {
         ErrorFound = true;
         continue;
       }
-
-      StringRef CmdlineVal = CmdlineDef.substr(EqIdx + 1);
-      uint64_t Val;
-      if (CmdlineVal.getAsInteger(10, Val)) {
-        SM.PrintMessage(SMLoc::getFromPointer(CmdlineVal.data()),
+      llvm::Optional<uint64_t> Value = NumExpr->getAST()->eval();
+      if (!Value) {
+        SM.PrintMessage(SMLoc::getFromPointer(CmdlineDefExpr.data()),
                         SourceMgr::DK_Error,
-                        "invalid value in numeric variable definition '" +
-                            CmdlineVal + "'");
+                        "unable to represent numeric value");
         ErrorFound = true;
         continue;
       }
-      auto DefinedNumericVariable =
-          std::make_shared<FileCheckNumericVariable>(CmdlineName, (unsigned)0);
-      DefinedNumericVariable->setValue(Val);
+      DefinedNumericVariable->setValue(*Value);
 
       // Record this variable definition.
       GlobalNumericVariableTable[DefinedNumericVariable->getName()] =
-          DefinedNumericVariable;
+          std::shared_ptr<FileCheckNumericVariable>(DefinedNumericVariable);
     } 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()) {

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

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

llvm/test/FileCheck/numeric-defines.txt

 ; Test functionality of -D# option: numeric variables are defined to the right
 ; value and CHECK directives using them match as expected given the value set.
 
-RUN: FileCheck -D#NUMVAL=12 --check-prefix CHECKNUM --input-file %s %s
-RUN: not FileCheck -D#NUMVAL=8 --check-prefix CHECKNUM --input-file %s %s 2>&1 \
-RUN:   | FileCheck %s --strict-whitespace --check-prefix NUMERRMSG
-RUN: not FileCheck -D#NUMVAL=12 --check-prefix NUMNOT --input-file %s %s 2>&1 \
-RUN:   | FileCheck %s --strict-whitespace --check-prefix NOT-NUMERRMSG
-RUN: FileCheck -D#NUMVAL=8 --check-prefixes NUMNOT --input-file %s %s
+RUN: FileCheck -D#NUMVAL1=8 -D#NUMVAL2='NUMVAL1 + 4' -check-prefix CHECKNUM -input-file %s %s
+RUN: not FileCheck -D#NUMVAL2=8 -check-prefix CHECKNUM -input-file %s %s 2>&1 \
+RUN:   | FileCheck %s --strict-whitespace -check-prefix NUMERRMSG
+RUN: not FileCheck -D#NUMVAL2=12 -check-prefix NUMNOT -input-file %s %s 2>&1 \
+RUN:   | FileCheck %s --strict-whitespace -check-prefix NOT-NUMERRMSG
+RUN: FileCheck -D#NUMVAL2=8 -check-prefixes NUMNOT -input-file %s %s
 
-Numeric value = 12
-CHECKNUM: Numeric value = [[#NUMVAL]]
-NUMNOT-NOT: Numeric value = [[#NUMVAL]]
+Numeric value #2 = 12
+CHECKNUM: Numeric value #2 = [[#NUMVAL2]]
+NUMNOT-NOT: Numeric value #2 = [[#NUMVAL2]]
 
 NUMERRMSG: defines.txt:[[#@LINE-3]]:11: error: CHECKNUM: expected string not found in input
 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:1:1: note: with numeric expression "NUMVAL2" equal to "8"
 NUMERRMSG: defines.txt:[[#@LINE-7]]:1: note: possible intended match here
 
 NOT-NUMERRMSG: defines.txt:[[#@LINE-7]]:13: 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"
+NOT-NUMERRMSG: defines.txt:[[#@LINE-11]]:1: note: with numeric expression "NUMVAL2" equal to "12"

llvm/test/FileCheck/numeric-expression.txt

 ; immediate interpreted as an unsigned value
 ; Note: 9223372036854775819 = 0x8000000000000000 + 11
 ;       9223372036854775808 = 0x8000000000000000
 USE UNSIGNED IMM
 9223372036854775819
 CHECK-LABEL: USE UNSIGNED IMM
 CHECK-NEXT: [[#VAR1+9223372036854775808]]
 
+; Numeric expression using a variable defined from a numeric expression
+DEF EXPR GOOD MATCH
+42
+41 43
+; CHECK-LABEL: DEF EXPR GOOD MATCH
+; CHECK-NEXT: [[# VAR42:VAR1+31]]
+; CHECK-NEXT: [[# VAR41: VAR42-1]] [[# VAR41 + 2]]
+
+; Empty numeric expression
+EMPTY NUM EXPR
+foo 104 bar
+; CHECK-LABEL: EMPTY NUM EXPR
+; CHECK-NEXT: foo [[#]] bar
+
 ; Numeric expression using undefined variable
 RUN: not FileCheck --check-prefix UNDEF-USE --input-file %s %s 2>&1 \
 RUN:   | FileCheck --strict-whitespace --check-prefix UNDEF-USE-MSG %s
 
 UNDEF VAR USE
 UNDEFVAR: 11
 UNDEF-USE-LABEL: UNDEF VAR USE
 UNDEF-USE-NEXT: UNDEFVAR: [[#UNDEFVAR]]

 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]]:22: 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: {{^                   \^$}}
+CLI-NUM-CONFLICT: Global defines:2:45: error: pattern variable with name 'PATVAR' already exists
+CLI-NUM-CONFLICT-NEXT: Global define #2: #PATVAR=42 (parsed as: {{\[\[#PATVAR:42\]\]}})
+CLI-NUM-CONFLICT-NEXT: {{^                                            \^$}}
+
+; Verify that when variable is set to an expression the expression is still
+; checked
+RUN: not FileCheck -check-prefix DEF-EXPR-FAIL -input-file %s %s
+
+DEF EXPR WRONG MATCH
+20
+43
+DEF-EXPR-FAIL-LABEL: DEF EXPR WRONG MATCH
+DEF-EXPR-FAIL-NEXT: [[# VAR20:]]
+DEF-EXPR-FAIL-NEXT: [[# VAR42: VAR20+22]]

llvm/unittests/Support/FileCheckTest.cpp

TEST_F(FileCheckTest, Literal) {
   EXPECT_TRUE(UndefVarNames.empty());
   Ten.appendUndefVarNames(UndefVarNames);
   EXPECT_TRUE(UndefVarNames.empty());
 }
 
 TEST_F(FileCheckTest, NumericVariable) {
   // Undefined variable: eval and clearValue fails, appendUndefVarNames returns
   // the variable and setValue works.
-  FileCheckNumericVariable FooVar = FileCheckNumericVariable("FOO", 1U);
+  auto NumVarExpr = FileCheckNumExpr(nullptr);
+  FileCheckNumericVariable FooVar =
+      FileCheckNumericVariable("FOO", &NumVarExpr, 1U);
   EXPECT_EQ("FOO", FooVar.getName());
   llvm::Optional<uint64_t> Value = FooVar.eval();
   EXPECT_FALSE(Value);
   EXPECT_TRUE(FooVar.clearValue());
   std::vector<StringRef> UndefVarNames;
   EXPECT_TRUE(UndefVarNames.empty());
   FooVar.appendUndefVarNames(UndefVarNames);
   EXPECT_EQ(1U, UndefVarNames.size());
   EXPECT_EQ("FOO", UndefVarNames[0]);
   EXPECT_FALSE(FooVar.setValue(42));
 
   // Defined variable: eval returns value set, setValue fails and
   // appendUndefVarNames is a nop.
   Value = FooVar.eval();
   EXPECT_TRUE(Value);
   EXPECT_EQ(42U, *Value);
   EXPECT_TRUE(FooVar.setValue(43));
   Value = FooVar.eval();
   EXPECT_TRUE(Value);
   EXPECT_EQ(42U, *Value);
   UndefVarNames.clear();
   EXPECT_TRUE(UndefVarNames.empty());
   FooVar.appendUndefVarNames(UndefVarNames);
   EXPECT_TRUE(UndefVarNames.empty());
 
   // Clearing variable: eval fails and clearValue again fails.
   // appendUndefVarNames returns the variable again.
   EXPECT_FALSE(FooVar.clearValue());
   Value = FooVar.eval();
   EXPECT_FALSE(Value);
   EXPECT_TRUE(FooVar.clearValue());
   EXPECT_TRUE(UndefVarNames.empty());
   FooVar.appendUndefVarNames(UndefVarNames);
   EXPECT_EQ(1U, UndefVarNames.size());
   EXPECT_EQ("FOO", UndefVarNames[0]);
 }
 
 uint64_t doAdd(uint64_t OpL, uint64_t OpR) { return OpL + OpR; }
 
 TEST_F(FileCheckTest, Binop) {
-  auto FooVar = std::make_shared<FileCheckNumericVariable>("FOO", (uint64_t)42);
-  auto BarVar = std::make_shared<FileCheckNumericVariable>("BAR", (uint64_t)18);
+  auto DefNumExpr = FileCheckNumExpr(nullptr);
+  auto FooVar =
+      std::make_shared<FileCheckNumericVariable>("FOO", &DefNumExpr, 1);
+  FooVar->setValue(42);
+  auto BarVar =
+      std::make_shared<FileCheckNumericVariable>("BAR", &DefNumExpr, 2);
+  BarVar->setValue(18);
   FileCheckASTBinop Binop = FileCheckASTBinop(doAdd, FooVar, BarVar);
 
   // Defined variable: eval returns right value, no undefined variable
   // returned.
   llvm::Optional<uint64_t> Value = Binop.eval();
   EXPECT_TRUE(Value);
   EXPECT_EQ(60U, *Value);
   std::vector<StringRef> UndefVarNames;
PatternTester() {
     initNextPattern();
   }
 
   void initNextPattern() {
     P = FileCheckPattern(Check::CheckPlain, &Context, LineNumber++);
   }
 
 public:
-  bool parseNumVarExpect(StringRef Expr, bool isDefinition) {
-    StringRef ExprBufferRef = bufferize(SM, Expr);
-    StringRef Name;
-    return P.parseNumericVariable(ExprBufferRef, Name, isDefinition,
-                                  false /*AcceptFail*/, SM);
-  }
-
   bool parseExprExpect(StringRef Expr) {
     StringRef ExprBufferRef = bufferize(SM, Expr);
     FileCheckNumericVariable *DefinedNumericVariable;
     return P.parseNumericExpression(ExprBufferRef, DefinedNumericVariable,
                                     false, SM) == nullptr;
   }
 
   bool parsePatternExpect(StringRef Pattern) {
     StringRef PatBufferRef = bufferize(SM, Pattern);
     return P.ParsePattern(PatBufferRef, "CHECK", SM, Req);
   }
 
   bool matchExpect(StringRef Buffer) {
     StringRef BufferRef = bufferize(SM, Buffer);
     size_t MatchLen;
     return P.match(BufferRef, MatchLen, SM);
   }
 };
 
-TEST_F(FileCheckTest, ParseNumericVariable) {
-  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) {
   PatternTester Tester;
 
   // Variable definition.
 
   // Definition of invalid variable.
   EXPECT_TRUE(Tester.parseExprExpect("10VAR:"));
   EXPECT_TRUE(Tester.parseExprExpect("@FOO:"));
   EXPECT_TRUE(Tester.parseExprExpect("@LINE:"));
 
+  // Conflict with pattern variable.
+  EXPECT_TRUE(Tester.parseExprExpect("BAR:"));
+
   // 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"));
 
   // Only valid variable.
   EXPECT_FALSE(Tester.parseExprExpect("@LINE"));
   EXPECT_FALSE(Tester.parseExprExpect("FOO"));
 
+  // Use variable defined on same line.
+  EXPECT_FALSE(Tester.parseExprExpect("LINE1VAR:"));
+  EXPECT_TRUE(Tester.parseExprExpect("LINE1VAR"));
+
   // Unsupported operator.
   EXPECT_TRUE(Tester.parseExprExpect("@LINE/2"));
 
   // Missing offset operand.
   EXPECT_TRUE(Tester.parseExprExpect("@LINE+"));
 
   // Cannot parse offset operand.
   EXPECT_TRUE(Tester.parseExprExpect("@LINE+x"));
TEST_F(FileCheckTest, ParsePattern) {
 
   // Valid pattern variable definition.
   EXPECT_FALSE(Tester.parsePatternExpect("[[PAT:[0-9]+]]"));
 
   // Invalid numeric expressions.
   EXPECT_TRUE(Tester.parsePatternExpect("[[#42INVALID]]"));
   EXPECT_TRUE(Tester.parsePatternExpect("[[#@FOO]]"));
   EXPECT_TRUE(Tester.parsePatternExpect("[[#@LINE/2]]"));
-  EXPECT_TRUE(Tester.parsePatternExpect("[[#YUP:@LINE]]"));
 
   // Valid numeric expressions and numeric variable definition.
   EXPECT_FALSE(Tester.parsePatternExpect("[[#FOO]]"));
   EXPECT_FALSE(Tester.parsePatternExpect("[[#@LINE+2]]"));
   EXPECT_FALSE(Tester.parsePatternExpect("[[#NUMVAR:]]"));
 }
 
 TEST_F(FileCheckTest, Match) {
   PatternTester Tester;
 
   // Check matching a definition only matches a number.
   Tester.parsePatternExpect("[[#NUMVAR:]]");
   EXPECT_TRUE(Tester.matchExpect("FAIL"));
   EXPECT_FALSE(Tester.matchExpect("18"));
 
   // Check matching the variable defined matches the correct number only
   Tester.initNextPattern();
   Tester.parsePatternExpect("[[#NUMVAR]] [[#NUMVAR+2]]");
   EXPECT_TRUE(Tester.matchExpect("19 21"));
   EXPECT_TRUE(Tester.matchExpect("18 21"));
   EXPECT_FALSE(Tester.matchExpect("18 20"));
 }
TEST_F(FileCheckTest, Substitution) {
   std::vector<StringRef> UndefVarNames;
   EXPECT_TRUE(UndefVarNames.empty());
   PatternSubstitution.getUndefVarNames(UndefVarNames);
   EXPECT_EQ(1U, UndefVarNames.size());
   EXPECT_EQ("VAR404", UndefVarNames[0]);
 
   // Substitution of defined numeric variable returns the right value,
   // getUndefVarNames does not return any variable.
+  auto DefNumExpr = FileCheckNumExpr(nullptr);
   auto LineVar =
-      std::make_shared<FileCheckNumericVariable>("@LINE", (uint64_t)42);
-  auto NVar = std::make_shared<FileCheckNumericVariable>("N", (uint64_t)10);
+      std::make_shared<FileCheckNumericVariable>("@LINE", &DefNumExpr, 1);
+  auto NVar = std::make_shared<FileCheckNumericVariable>("N", &DefNumExpr, 1);
+  LineVar->setValue(42);
+  NVar->setValue(10);
   FileCheckNumExpr NumExprLine = FileCheckNumExpr(LineVar);
   FileCheckNumExpr NumExprN = FileCheckNumExpr(NVar);
   FileCheckPatternSubstitution SubstitutionLine =
       FileCheckPatternSubstitution(&Context, "@LINE", &NumExprLine, 12);
   FileCheckPatternSubstitution SubstitutionN =
       FileCheckPatternSubstitution(&Context, "N", &NumExprN, 30);
   llvm::Optional<std::string> Value = SubstitutionLine.getResult();
   EXPECT_TRUE(Value);
TEST_F(FileCheckTest, FileCheckContext) {
   llvm::Optional<StringRef> EmptyVar = Cxt.getPatternVarValue(EmptyVarStr);
   llvm::Optional<StringRef> UnknownVar = Cxt.getPatternVarValue(UnknownVarStr);
   EXPECT_TRUE(LocalVar);
   EXPECT_EQ(*LocalVar, "FOO");
   EXPECT_TRUE(NumExpr);
   llvm::Optional<uint64_t> NumExprVal = NumExpr->getAST()->eval();
   EXPECT_TRUE(NumExprVal);
   EXPECT_EQ(*NumExprVal, 18U);
   EXPECT_TRUE(EmptyVar);
   EXPECT_EQ(*EmptyVar, "");
   EXPECT_FALSE(UnknownVar);
 
   // Clear local variables and check they become absent.
   Cxt.clearLocalVars();
   LocalVar = Cxt.getPatternVarValue(LocalVarStr);
   EXPECT_FALSE(LocalVar);
   // Check a numeric expression's evaluation fails if called after clearing of