FileCheck [7/12]: Arbitrary long numeric expressions

[thopre]

Summary

This patch is part of a patch series to add support for FileCheck
numeric expressions. This specific patch extend numeric expression to
support an arbitrary number of operands, either variable or literals.

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

[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]

Get rid of the useless FileCheckNumExpr class which is only needed when matching format are introduced

[thopre]

Rebase on top of latest changes in patch series

[thopre]

Rebase on top of latest changes in the series

[arichardson]

llvm/include/llvm/Support/FileCheck.h

122

Maybe this should use Expected instead of Optional to report errors?

llvm/lib/Support/FileCheck.cpp

717

Could this be replaced with Error/Expected so that there is no more need for getUndefVarNames/appendUndefVarNames calls?

[jhenderson]

I've reviewed up to the tests, and got lots of comments for you.

llvm/docs/CommandGuide/FileCheck.rst

594–595

numeric variables previously defined -> previously defined numeric variables

llvm/include/llvm/Support/FileCheck.h

43–93

Are there other kinds of expressions except "numeric expressions"? Just wondering if we can simplify the terminology a little e.g. by simply referring to them as expressions and the FileCheckNumExprAST can become FileCheckExpressionAST.

66–67

Are there plans for signed literals later? If not, then I'd get rid of "unsigned" from the comment.

69–70

You could probably simplify this down to \returns the literal's value.

120–122

Similar comment to above. No need to explain that its evaluating the expression for a single variable. Just keep the "returns this variable's value" part.

143

Do these need to be shared_ptrs? Do multiple places need ownership, or is it sufficient that only place has it? Same question applies for all uses of shared_ptr further down.

158

Could this use a MutableArrayRef?

199–201

How about "the name of the undefined variables used in this substitution"?

Also use "Appends" instead of "Records" to show that it doesn't override the old ones.

Same comments apply to the other examples of getUndefVarNames.

210–212

What if some string variables are defined and others aren't? The comment as it stands suggests that all are stored. If that's not intended, I'd rephrase it as "the name of the undefined string variables used in this substitution"

468

What's a "legacy numeric substitution block"?

473

defined -> define

536

I don't like the terminology "Legacy", as it doesn't convey any useful meaning, unless defined somewhere clearly.

543

I don't think you need enum here. Same applies throughout the code.

llvm/lib/Support/FileCheck.cpp

80

Are you sure you want to clear this? That seems unnecessary, and confusing if you are passing in a reference. If you want an empty vector, I'd just expect the vector to be returned.

181

Get rid of this blank line.

195

Magic number?

[thopre]

• Rebase on latest changes in patch series
• Address obvious changes

[thopre]

llvm/include/llvm/Support/FileCheck.h

43–93

There's only numeric expression indeed but wouldn't the use of "expression" only induce the reader into thinking it only applies when there is an operator involved? Numeric expression as it stands also cover a single numeric variable in a numeric substitution. Also if changing to expression only, should I remove the "numeric" adjective in the documentation as well?

66–67

There are but the comment can be changed when they are introduced. Fixed.

143

Some AST nodes are referenced from a single place only (eg. literals and binary operations) while numeric variables can be shared among several numeric substitutions, eg. "add r[[#REG]], r[[#REG+1]], 42" would have 2 pointers to the class instance for REG. In the first case std::unique_ptr could be used, while in the latter case std::shared_ptr would make sense.

The problem comes from AST nodes being referred via FileCheckAST * pointers (since each operand can be either of the possible nodes) and thus I need to decide on a given pointer type for all nodes. I could go with regular pointers and keep an array of pointer in FileCheckPatternContext to all nodes created for them to be freed all at once when matching is done. Wouldn't change much over what is happening now since substitutions are only deleted once all pattern have been matched and substitutions points to the AST indirectly. Does that sound good?

468

It refers to the "legacy use of @LINE" mentioned in the section "FileCheck Pseudo Numeric Variables" of the documentation, namely [[@LINE]] and [[@LINE+offset]] instead of the same with a '#' sign. Would "indicates whether Expr is a legacy use of @LINE" be clearer? Or perhaps "legacy @LINE expression"? If not, any other suggestion?

536

See above.

[probinson]

Regarding what to call the @LINE+offset form, I think it's fine to call it "legacy" in the user-facing documentation, but it gets to be a bit much in the internals. I haven't commented every use but you will get the idea in the inline comments.

llvm/docs/CommandGuide/FileCheck.rst

593

<expr>> should be <expr>

597

There are a few different ways to describe expressions, I happen to prefer a recursive definition, something like this:

A numeric expression is recursively defined as:

• a numeric operand, or
• a numeric expression followed by an operator and a numeric operand.

A numeric operand is a previously defined numeric variable, or an integer literal. The supported operators are + and -. Spaces are accepted before, after and between any of these elements.

llvm/include/llvm/Support/FileCheck.h

43–93

I think "expression" does not necessarily imply an operator, at least not to a programmer.
You can probably also remove some uses of "numeric" in the documentation, when you are talking about expressions (but keep it when talking about variables).

468

I don't know about James but I would prefer that the commentary say "legacy @LINE expression" and the parameter could be named LineExpr.

538

These could be { LineVar, Literal, Any } perhaps.

540

I don't see a parameter named AllowedOperandFlag.

548

\p LineExpr ... parsing a @LINE expression.

llvm/lib/Support/FileCheck.cpp

189

The previous if guarantees AO is LegacyVar or Any here. So, this condition simplifies to AO == LegacyVar correct? (or LineVar if you rename as suggested.)

191

Maybe a comment here, "Ignore the error and retry parsing as a literal."

194

I think the only possibilities at this point are LegacyLiteral and Any, so this if is always true and should be removed.

244

... in a @LINE expression ...

427

IsLineExpr

719

Delete the commented-out assertion.

727

llvm_unreachable instead of assert(false)?

[thopre]

Address all comments but shared_ptr

[thopre]

llvm/include/llvm/Support/FileCheck.h

468

I went for LegacyLineExpr then since when false @LINE is also allowed and when true it limits what can be done with @LINE.

llvm/lib/Support/FileCheck.cpp

194

I've addressed this comment and the other one about AO above but I wanted to say that although I realize that LLVM code style aims at reducing clutter to make the code more readable, I feel in many case (eg. here and some of the suppression of else after return) it makes the code less explicit and more fragile.

[thopre]

Rebase

[jhenderson]

Apologies for the delay in coming back to this. It's been a busy few days.

llvm/include/llvm/Support/FileCheck.h

66–67

I think "Class to represent an undefined variable error, which quotes that variable's name when printed." would be a bit more concise.

94

definition -> definitions

120–121

If you made this eval() return an error if the variable is undefined, it would nicely separate concerns of use versus definition (see my comment a bit further down about shared_ptrs).

143

I wonder whether it might make more sense to separate the concept of variables from the AST. The AST could have a variable reference class in it, which refers by raw pointer to a variable definition. An AST expression then solely "owns" each subexpression, whilst something higher up (e.g. the Context - not sure about this), owns the variables and contains the definitions. That way each item in the tree is only owned by its parents, which feels more logical.

Does that make sense?

As things stand the fact that a single FileCheckNumericVariable can be owned by multiple places implies it's not really a tree at all. (In graph theory a tree has no cross-references - everything is owned by a single parent only).

158

This should say "returns an error if a subexpression returns an error, or ..."

468

How about IsLegacyLineExpr? It's a bit more verbose but maybe a little clearer too. Otherwise LegacyLineExpr is okay. It's just that the variable isn't actually an expression itself.

470

substitued -> substituted

537

Perhaps worth making this an enum class.

541

"against \p SM"? I'm not sure I understand this change in terminology.

llvm/lib/Support/FileCheck.cpp

197

I'm guessing there is a plan to allow other Radixes too? It seems like it would be normal to want to use hex, for example.

243–250

in a legacy

271

Err would be a more common variable name.

[jhenderson]

llvm/lib/Support/FileCheck.cpp

291

in a legacy

716–717

Trailing full stop required.

716–729

I think this is actually unnecessary. handleAllErrors terminates the program with an llvm_unreachable anyway if there are any unhandled errors.

[thopre]

Make new function parameter immediate use the /*ParamName=*/Value code style

[probinson]

I think I am happy with this, leaving the rest up to James.

llvm/include/llvm/Support/FileCheck.h

158

I prefer the description of the return value to give the expected/normal case first, and the error case afterward. So,

\returns the expression value, or an error ...

[thopre]

• Rebase on top of refactoring patches
• Address comments

llvm/include/llvm/Support/FileCheck.h

541

I've used it in other place already. I'm using "against" because the diagnostic relates to the buffer held by SM (it is created via SM.GetMessage)

llvm/lib/Support/FileCheck.cpp

197

/There is yes. See patch 9/12

[thopre]

Rebase on latest changes in patch series

[thopre]

Rebase on top of latest changes in patch series

[jhenderson]

Mostly looks good. A few nits still and one or two more significant comments. I think this structure looks much cleaner than the previous version, personally.

llvm/include/llvm/Support/FileCheck.h

54

As things stand, this only can represent an unsigned literal. I think you'll want to support negative literals or unary negation operands fairly quickly. Otherwise, the expression "-1" doesn't work.

I'm also slightly concerned in the current form that somebody could try to use FileCheckExpressionLiteral for negative numbers, and get surprising results. Perhaps worth updating the comment to say that the class represents an unsigned integer literal.

60

Perhaps "Construct a literal with the specified value."

llvm/lib/Support/FileCheck.cpp

58

Might be worth making this comment more generic e.g. "Handle a failed operation, e.g. due to an undefined variable." That'll make the comment robust to other possible future failure types.

243–250

is always a literal

The comment should probably start with "The second..."

309

The comment should probably start with "The first..."

716–717

You've changed this message implying it can have more than one undefined variable, but I don't see any lit test case change that has more than one. You should add/update one.

llvm/unittests/Support/FileCheckTest.cpp

17

I think it would be a good idea to have a test case showing that this can take std::numeric_limits<uint64_t>::max().

25

const std::set &?

30

Perhaps worth having this print what names are still remaining if not empty. I think you can do something like EXPECT_TRUE(ExpectedUndefVarNames.empty()) << ExpectedUndefVarNames.

52–53

getValue not getvalue.

390

In other places you've used bool(SubstValue) for this sort of conversion. I think that might be nicer, but don't care that much. Regardless, it should be consistent throughout.

473–475

unique_ptr?

[thopre]

Address review comments.

[thopre]

llvm/include/llvm/Support/FileCheck.h

541

Is my explanation satisfying or do you want to use another phrasing?

llvm/lib/Support/FileCheck.cpp

716–717

There was a unit test already but it doesn't test that message indeed. I've added 1 lit test and updated another one, the former for correct substitution and the latter for undefined variables.

llvm/unittests/Support/FileCheckTest.cpp

25

This cannot be const since erase is called on that set. Making it a reference means the set needs to be created in all the caller when now the caller can provide an initializer as is done in expectUndefError below.

[thopre]

Run clang-format-diff

[jhenderson]

llvm/include/llvm/Support/FileCheck.h

60

litteral -> literal

541

Yes, it's fine. Not the phrasing I would use but I don't care much about it.

llvm/lib/Support/FileCheck.cpp

58

eg. -> e.g.
either "an undefined variable" or "undefined variables"

llvm/unittests/Support/FileCheckTest.cpp

19

Why not FileCheckExpressionLiteral Ten(10)?

25

Why not FileCheckExpressionLiteral Max(...)?

33

Other instances of overloading operator<< in the LLVM unit tests tend to just be in the llvm namespace, not in std. See for example ADT/StringRefTest.cpp.

The variables in this function don't use LLVM naming style.

35

StringRef?

47

Use std::unordered_set?

[grimar]

llvm/lib/Support/FileCheck.cpp

56

Doesn't look like you need this->.

184

btw it was a bit confusing for me to find there is a method that take bool IsPseudo
and method that take bool &IsPseudo, i.e. that one of them can change this flag, but another - not.

186

You can avoid having this temorarily variable:

if (ParseVarResult)
  return parseNumericVariableUse(*ParseVarResult, IsPseudo, SM);

But actually it seems you also need to check the result of Expected,
not sure why it is assumed there is no error?

427

I'd suggest adding a comment.

llvm/unittests/Support/FileCheckTest.cpp

34

Seems a different variables naming style used here? Ten, Value vs first, set. You need stick to one I think.
(To upper case I guess? LLVM did not do a switch to a new one yet, except LLD project I believe).

[grimar]

llvm/lib/Support/FileCheck.cpp

186

Oh, sorry, I misread the code, you actually check it. Then you still can avoid having the temp var :)

[thopre]

llvm/lib/Support/FileCheck.cpp

184

You mean parseVariable Vs parseNumericVariableUse? Would you rather me have both be reference even though the latter only needs to get the value? Or is there a naming convention to denote a reference as opposed to a parameter passed by value?

llvm/unittests/Support/FileCheckTest.cpp

33

I'm not sure why but when I try with llvm namespace and I switch to unordered_set as suggested below the overloaded operator does not get used and the code fails to compile. However the code compiles and works with llvm namespace if I use a set instead of unordered_set.

I've thus decided to just write a setToString method and output the result of calling it. It does create lots of temporaries though.

[thopre]

Address review comments

[jhenderson]

llvm/unittests/Support/FileCheckTest.cpp

31

static?

Though saying that, this is inside an anonymous namespace, so perhaps the other functions need static removing.

This function returns a StringRef, yet the base type is a local variable. I think that will result in the memory being freed under the reference. Perhaps better would be to make Str a Twine and then use Twine::str() to return a std::string.

37

Perhaps make this ", ", and start and end the full string with '{'/'}' to make it easier to read.

[grimar]

llvm/include/llvm/Support/FileCheck.h

534–535

This comment still mentions Expr and SM.

llvm/lib/Support/FileCheck.cpp

184

There is no special naming conventions, but given that parseVariable method always must set IsPseudo flag,
but another parse* methods can only read the flags they are given, I wonder if it be better/cleaner for parseVariable to return Expected<std::pair<StringRef, bool> for example?

[thopre]

Address review comments

llvm/lib/Support/FileCheck.cpp

184

I'm not a big fan of pairs because its members are accessed without a name (first or second) which I find it less readable. Anyway, done but I've reverted the change to make the call to parseNumericVariableUse a one liner so that it is clear what is being passed to that function (ParseVarResult->first is not very descriptive).

llvm/unittests/Support/FileCheckTest.cpp

31

Ah yes indeed, the StringRef does not own the string it references. I cannot use a Twine for Str because you cannot store a Twine so while I can do Str + S, I cannot store the result in Str (see https://llvm.org/doxygen/classllvm_1_1Twine.html#a28b850ffd65d5997cf730d644e06d89b I've been bitten by this before in this patchset). I guess I'll return a std::string then. I've used a Twine to do the " " concatenation though.

[jhenderson]

llvm/lib/Support/FileCheck.cpp

184

You could use a small struct to name the members, perhaps called VariableProperties:

struct VariableProperties {
  StringRef Name;
  bool IsPseudo;
}

[thopre]

Return a VariableProperties structure in parseVariable

[jhenderson]

LGTM. Thanks for the hard work on this!

llvm/lib/Support/FileCheck.cpp

110

You might be able to just do return {Name, IsPseudo} to avoid the temporary.

[thopre]

llvm/lib/Support/FileCheck.cpp

110

I tried but it didn't work. I think it would if I was returning the structure, but here I'm returning an Expected.

As per 6.6.3 C++11 draft:
"A return statement with a braced-init-list initializes the object or reference to be returned from the function by copy-list-initialization (8.5.4) from the specified initializer list."

So it tries to build an Expected with that initialization list which fails because I think an Expected<foo> only accepts a Foo.

[thopre]

llvm/lib/Support/FileCheck.cpp

110

Yes, that's it:

/home/thomasp/repos/llvm-project/llvm/lib/Support/FileCheck.cpp:107:10: error: no matching constructor for initialization of 'Expected<FileCheckPattern::VariableProperties>'

return {Name, IsPseudo};
       ^~~~~~~~~~~~~~~~

/home/thomasp/repos/llvm-project/llvm/include/llvm/Support/Error.h:474:3: note: candidate template ignored: requirement 'std::is_convertible<StringRef &, VariableProperties>::value' was not satisfied [with OtherT = llvm::StringRef &]

Expected(OtherT &&Val,
^

It tries to convert Name into a VariableProperties and fails. All the other constructors expect an Error or another Expected and fail as well.

[arichardson]

Thanks for your work on this! Looking forward to be able to use these new features the next time I update our fork.

llvm/lib/Support/FileCheck.cpp

110

Maybe return VariableProperties{Name, IsPseudo}would work?

[jhenderson]

llvm/lib/Support/FileCheck.cpp

110

Yes of course. I think you could still do it inline if you want, (not tried, so not 100% certain off the top of my head) and don't mind either way:

return VariableProperties {Name, IsPseudo};

[thopre]

Return structure in parseVariable as a temporary

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>
+.. option:: -D#<NUMVAR>=<VALUE EXPRESSION>
 
-  Sets a filecheck numeric variable ``NUMVAR`` to ``<VALUE>`` that can be used
-  in ``CHECK:`` lines.
+  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>``.
 
 .. option:: -version
 
  Show the version number of this program.
 
 .. option:: -v
 
   Print good directive pattern matches.  However, if ``-input-dump=fail`` or

 For example:
 
 .. code-block:: llvm
 
     ; CHECK: mov r[[#REG:]], 42
 
 would match ``mov r5, 42`` and set ``REG`` to the value ``5``.
 
-The syntax of a numeric substitution is ``[[#<NUMVAR><op><offset>]]`` where:
-
-* ``<NUMVAR>`` is the name of a defined numeric variable.
-
-* ``<op>`` is an optional numeric operation to perform on the value of
-  ``<NUMVAR>``. Currently supported numeric operations are ``+`` and ``-``.
-
-* ``<offset>`` is the immediate value that constitutes the second operand of
-  the numeric operation <op>. It must be present if ``<op>`` is present,
-  absent otherwise.
-
-Spaces are accepted before, after and between any of these elements.
+The syntax of a numeric substitution is ``[[#<expr>]]`` where ``<expr>>`` is a
+numeric expression whose operands are either previously defined numeric
+variables or integer literals.  Currently supported numeric operations are
+``+`` and ``-``. A single numeric variable or integer literal is also accepted.
+Spaces are accepted before, after and between the operands.
 
 For example:
 
 .. code-block:: llvm
 
     ; CHECK: load r[[#REG:]], [r0]
     ; CHECK: load r[[#REG+1]], [r1]
 

llvm/include/llvm/Support/FileCheck.h

struct FileCheckRequest {
   bool Verbose = false;
   bool VerboseVerbose = false;
 };
 
 //===----------------------------------------------------------------------===//
 // Numeric substitution handling code.
 //===----------------------------------------------------------------------===//
 
-/// Class representing a numeric variable with a given value in a numeric
-/// expression. Each definition of a variable gets its own instance of this
-/// class. Variable uses share the same instance as their respective
+/// Base class representing the AST of a given numeric expression.
+class FileCheckNumExprAST {
+public:
+  virtual ~FileCheckNumExprAST() = default;
+
+  /// Evaluates and \returns the value of the expression represented by this
+  /// AST or an error if evaluation fails.
+  virtual Expected<uint64_t> eval() const = 0;
+};
+
+/// Class representing a literal in the AST of a numeric expression.
+class FileCheckNumExprLiteral : public FileCheckNumExprAST {
+private:
+  /// Actual value of the literal.
+  uint64_t Value;
+
+public:
+  /// Constructor for a literal.
+  FileCheckNumExprLiteral(uint64_t Val) : Value(Val) {}
+
+  /// \returns the literal's value.
+  Expected<uint64_t> eval() const { return Value; }
+};
+
+/// Class to represent an undefined variable error which prints that variable's
+/// name between quotes when printed.
+class FileCheckUndefVarError : public ErrorInfo<FileCheckUndefVarError> {
+private:
+  StringRef VarName;
+
+public:
+  static char ID;
+
+  FileCheckUndefVarError(StringRef VarName) : VarName(VarName) {}
+
+  StringRef getVarName() const { return VarName; }
+
+  std::error_code convertToErrorCode() const override {
+    return inconvertibleErrorCode();
+  }
+
+  /// Print name of variable associated with this error.
+  void log(raw_ostream &OS) const override {
+    OS << "\"";
+    OS.write_escaped(VarName) << "\"";
+  }
+};
+
+/// Class representing a numeric variable with a given value in the AST of a
+/// numeric expression. Each definition of a variable gets its own instance of
+/// this class. Variable uses share the same instance as their respective
 /// definition.
-class FileCheckNumericVariable {
+class FileCheckNumericVariable : public FileCheckNumExprAST {
 private:
   /// Name of the numeric variable.
   StringRef Name;
 
   /// Value of numeric variable, if defined, or None otherwise.
   Optional<uint64_t> Value;
 
   /// Line number where this variable is defined. Used to determine whether a
   /// variable is defined on the same line as a given use.
   size_t DefLineNumber;
 
 public:
   /// Constructor for a variable \p Name defined at line \p DefLineNumber.
   FileCheckNumericVariable(size_t DefLineNumber, StringRef Name)
       : Name(Name), DefLineNumber(DefLineNumber) {}
 
   /// Constructor for numeric variable \p Name with a known \p Value at parse
   /// time (e.g. the @LINE numeric variable).
   FileCheckNumericVariable(StringRef Name, uint64_t Value)
       : Name(Name), Value(Value), DefLineNumber(0) {}
 
-  /// \returns name of that numeric variable.
+  /// \returns name of this numeric variable.
   StringRef getName() const { return Name; }
 
-  /// \returns value of this numeric variable.
-  Optional<uint64_t> getValue() const { return Value; }
+  /// \returns this variable's value.
+  Expected<uint64_t> eval() 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.
   size_t getDefLineNumber() { return DefLineNumber; }
 };
 
 /// Type of functions evaluating a given binary operation.
 using binop_eval_t = uint64_t (*)(uint64_t, uint64_t);
 
-/// Class to represent an undefined variable error which prints that variable's
-/// name between quotes when printed.
-class FileCheckUndefVarError : public ErrorInfo<FileCheckUndefVarError> {
-private:
-  StringRef VarName;
-
-public:
-  static char ID;
-
-  FileCheckUndefVarError(StringRef VarName) : VarName(VarName) {}
-
-  StringRef getVarName() const { return VarName; }
-
-  std::error_code convertToErrorCode() const override {
-    return inconvertibleErrorCode();
-  }
-
-  /// Print name of variable associated with this error.
-  void log(raw_ostream &OS) const override {
-    OS << "\"";
-    OS.write_escaped(VarName) << "\"";
-  }
-};
-
-/// Class representing a numeric expression consisting of either a single
-/// numeric variable or a binary operation between a numeric variable and an
-/// immediate.
-class FileCheckNumExpr {
+/// Class representing a single binary operation in the AST of a numeric
+/// expression.
+class FileCheckASTBinop : public FileCheckNumExprAST {
 private:
   /// Left operand.
-  FileCheckNumericVariable *LeftOp;
+  std::shared_ptr<FileCheckNumExprAST> LeftOp;
 
   /// Right operand.
-  uint64_t RightOp;
+  std::shared_ptr<FileCheckNumExprAST> RightOp;
 
   /// Pointer to function that can evaluate this binary operation.
   binop_eval_t EvalBinop;
 
 public:
-  FileCheckNumExpr(binop_eval_t EvalBinop,
-                   FileCheckNumericVariable *OperandLeft, uint64_t OperandRight)
+  FileCheckASTBinop(binop_eval_t EvalBinop,
+                    std::shared_ptr<FileCheckNumExprAST> OperandLeft,
+                    std::shared_ptr<FileCheckNumExprAST> OperandRight)
       : LeftOp(OperandLeft), RightOp(OperandRight), EvalBinop(EvalBinop) {}
 
-  /// Evaluates the value of this numeric expression, using EvalBinop to
-  /// perform the binary operation it consists of. \returns an error if the
-  /// numeric variable used is undefined, or the expression value otherwise.
+  /// Evaluates the value of the binary operation represented by this AST,
+  /// using EvalBinop on the result of recursively evaluating the operands.
+  /// \returns an error if a numeric variable used is undefined, or the
+  /// expression value otherwise.
   Expected<uint64_t> eval() const;
 };
 
 class FileCheckPatternContext;
 
 /// Class representing a substitution to perform in the RegExStr string.
 class FileCheckSubstitution {
 protected:
public:
 
   /// \returns the string to be substituted for something else.
   StringRef getFromString() const { return FromStr; }
 
   /// \returns the index where the substitution is to be performed in RegExStr.
   size_t getIndex() const { return InsertIdx; }
 
   /// \returns a string containing the result of the substitution represented
   /// by this class instance or an error if substitution failed.
   virtual Expected<std::string> getResult() const = 0;
 };
 
 class FileCheckStringSubstitution : public FileCheckSubstitution {
 public:
   FileCheckStringSubstitution(FileCheckPatternContext *Context,
                               StringRef VarName, size_t InsertIdx)
       : FileCheckSubstitution(Context, VarName, InsertIdx) {}
 
   /// \returns the text that the string variable in this substitution matched
   /// when defined, or an error if the variable is undefined.
   Expected<std::string> getResult() const override;
 };
 
 class FileCheckNumericSubstitution : public FileCheckSubstitution {
 private:
   /// Pointer to the class representing the numeric expression whose value is
   /// to be substituted.
-  FileCheckNumExpr *NumExpr;
+  std::shared_ptr<FileCheckNumExprAST> NumExprAST;
 
 public:
   FileCheckNumericSubstitution(FileCheckPatternContext *Context, StringRef Expr,
-                               FileCheckNumExpr *NumExpr, size_t InsertIdx)
-      : FileCheckSubstitution(Context, Expr, InsertIdx), NumExpr(NumExpr) {}
+                               std::shared_ptr<FileCheckNumExprAST> NumExprAST,
+                               size_t InsertIdx)
+      : FileCheckSubstitution(Context, Expr, InsertIdx),
+        NumExprAST(NumExprAST) {}
 
   /// \returns a string containing the result of evaluating the numeric
   /// expression in this substitution, or an error if evaluation failed.
   Expected<std::string> getResult() const override;
 };
 
 //===----------------------------------------------------------------------===//
 // Pattern handling code.
private:
 
   /// When matching a given pattern, this holds the pointers to the classes
   /// representing the last definitions of numeric variables defined in
   /// previous patterns. Earlier definitions of the variables, if any, have
   /// their own class instance not referenced by this table. When matching a
   /// pattern all definitions for that pattern are recorded in the
   /// NumericVariableDefs table in the FileCheckPattern instance of that
   /// pattern.
-  StringMap<FileCheckNumericVariable *> GlobalNumericVariableTable;
-
-  /// Vector holding pointers to all parsed numeric expressions. Used to
-  /// automatically free the numeric expressions once they are guaranteed to no
-  /// longer be used.
-  std::vector<std::unique_ptr<FileCheckNumExpr>> NumExprs;
-
-  /// Vector holding pointers to all parsed numeric variables. Used to
-  /// automatically free them once they are guaranteed to no longer be used.
-  std::vector<std::unique_ptr<FileCheckNumericVariable>> NumericVariables;
+  StringMap<std::shared_ptr<FileCheckNumericVariable>>
+      GlobalNumericVariableTable;
 
   /// Vector holding pointers to all substitutions. Used to automatically free
   /// them once they are guaranteed to no longer be used.
   std::vector<std::unique_ptr<FileCheckSubstitution>> Substitutions;
 
 public:
   /// \returns the value of string variable \p VarName or an error if no such
   /// variable has been defined.
   Expected<StringRef> getPatternVarValue(StringRef VarName);
 
   /// Defines string and numeric variables from definitions given on the
   /// command line, passed as a vector of [#]VAR=VAL strings in
   /// \p CmdlineDefines. \returns an error list containing diagnostics against
   /// \p SM for all definition parsing failures, if any, or Success otherwise.
   Error defineCmdlineVariables(std::vector<std::string> &CmdlineDefines,
                                SourceMgr &SM);
 
   /// Undefines local variables (variables whose name does not start with a '$'
   /// sign), i.e. removes them from GlobalVariableTable and from
   /// GlobalNumericVariableTable and also clears the value of numeric
   /// variables.
   void clearLocalVars();
 
 private:
-  /// Makes a new numeric expression instance and registers it for destruction
-  /// when the context is destroyed.
-  FileCheckNumExpr *makeNumExpr(binop_eval_t EvalBinop,
-                                FileCheckNumericVariable *OperandLeft,
-                                uint64_t OperandRight);
-
-  /// Makes a new numeric variable and registers it for destruction when the
-  /// context is destroyed.
-  template <class... Types>
-  FileCheckNumericVariable *makeNumericVariable(Types... args);
-
   /// Makes a new string substitution and registers it for destruction when the
   /// context is destroyed.
   FileCheckSubstitution *makeStringSubstitution(StringRef VarName,
                                                 size_t InsertIdx);
 
   /// Makes a new numeric substitution and registers it for destruction when
   /// the context is destroyed.
-  FileCheckSubstitution *makeNumericSubstitution(StringRef Expr,
-                                                 FileCheckNumExpr *NumExpr,
-                                                 size_t InsertIdx);
+  FileCheckSubstitution *
+  makeNumericSubstitution(StringRef Expr,
+                          std::shared_ptr<FileCheckNumExprAST> NumExprAST,
 };
 
 /// Class to represent a parsing error, holding a diagnostic with location
 /// information used when printing it.
 class FileCheckParseError : public ErrorInfo<FileCheckParseError> {
 private:
   SMDiagnostic Diagnostic;
 
static Expected<StringRef> parseVariable(StringRef &Str, bool &IsPseudo,
                                            const SourceMgr &SM);
   /// Parses \p Expr for the name of a numeric variable to be defined. \returns
   /// an error holding a diagnostic against \p SM should defining such a
   /// variable be invalid, or Success otherwise. In the latter case, sets
   /// \p Name to the name of the parsed numeric variable name.
   static Error parseNumericVariableDefinition(StringRef &Expr, StringRef &Name,
                                               FileCheckPatternContext *Context,
                                               const SourceMgr &SM);
-  /// Parses \p Expr for a numeric substitution block. \returns the class
-  /// representing the AST of the numeric expression whose value must be
-  /// substituted, or an error holding a diagnostic against \p SM if parsing
-  /// fails. If substitution was successful, sets \p DefinedNumericVariable to
-  /// point to the class representing the numeric variable defined in this
-  /// numeric substitution block, or None if this block does not define any
-  /// variable.
-  Expected<FileCheckNumExpr *> parseNumericSubstitutionBlock(
+  /// Parses \p Expr for a numeric substitution block. Parameter \p Legacy
+  /// indicates whether \p Expr should be a legacy numeric substitution block.
+  /// \returns a pointer to the class instance representing the AST of the
+  /// numeric expression whose value must be substitued, or an error holding a
+  /// diagnostic against \p SM if parsing fails. If substitution was
+  /// successful, sets \p DefinedNumericVariable to point to the class
+  /// representing the numeric variable defined in this numeric substitution
+  /// block, or None if this block does not define any variable.
+  Expected<std::shared_ptr<FileCheckNumExprAST>> parseNumericSubstitutionBlock(
       StringRef Expr,
-      Optional<FileCheckNumericVariable *> &DefinedNumericVariable,
+      Optional<FileCheckNumericVariable *> &DefinedNumericVariable, bool Legacy,
       const SourceMgr &SM) const;
   /// Parses the pattern in \p PatternStr and initializes this FileCheckPattern
   /// instance accordingly.
   ///
   /// \p Prefix provides which prefix is being matched, \p Req describes the
   /// global options that influence the parsing such as whitespace
   /// canonicalization, \p SM provides the SourceMgr used for error reports.
   /// \returns true in case of an error, false otherwise.
   bool parsePattern(StringRef PatternStr, StringRef Prefix, SourceMgr &SM,
                     const FileCheckRequest &Req);
   /// Matches the pattern string against the input buffer \p Buffer
   ///
   /// \returns the position that is matched or npos if there is no match. If
   /// there is a match, updates \p MatchLen with the size of the matched
   /// string.
   ///
   /// The GlobalVariableTable StringMap in the FileCheckPatternContext class
   /// instance provides the current values of FileCheck string variables and
   /// is updated if this match defines new values. Likewise, the
   /// GlobalNumericVariableTable StringMap in the same class provides the
   /// current values of FileCheck numeric variables and is updated if this
   /// match defines new numeric values.
   size_t match(StringRef Buffer, size_t &MatchLen, const SourceMgr &SM) const;
   /// Prints the value of successful substitutions or the name of the undefined
-  /// string or numeric variable preventing a successful substitution.
+  /// string or numeric variables preventing a successful substitution.
   void printSubstitutions(const SourceMgr &SM, StringRef Buffer,
                           SMRange MatchRange = None) const;
   void printFuzzyMatch(const SourceMgr &SM, StringRef Buffer,
                        std::vector<FileCheckDiag> *Diags) const;
 
   bool hasVariable() const {
     return !(Substitutions.empty() && VariableDefs.empty());
   }
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);
 
   /// Parses \p Expr for the use of a numeric variable. \returns the pointer to
   /// the class instance representing that variable if successful, or an error
   /// holding a diagnostic against \p SM otherwise.
-  Expected<FileCheckNumericVariable *>
-  parseNumericVariableUse(StringRef &Expr, const SourceMgr &SM) const;
-  /// Parses \p Expr for a binary operation.
-  /// \returns the class representing the binary operation of the numeric
-  /// expression, or an error holding a diagnostic against \p SM otherwise.
-  Expected<FileCheckNumExpr *> parseBinop(StringRef &Expr,
-                                          const SourceMgr &SM) const;
+  Expected<std::shared_ptr<FileCheckNumericVariable>>
+  parseNumericVariableUse(StringRef Name, bool IsPseudo,
+  enum AllowedOperand { LegacyVar, LegacyLiteral, Any };
+  /// 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 an error holding a diagnostic against \p SM otherwise.
+  Expected<std::shared_ptr<FileCheckNumExprAST>>
+  parseNumericOperand(StringRef &Expr, 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 the
+  /// binary operation in the AST of the numeric expression, or an error
+  /// holding a diagnostic against \p SM otherwise.
+  Expected<std::shared_ptr<FileCheckNumExprAST>>
+  parseBinop(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?

llvm/lib/Support/FileCheck.cpp

 #include <cstdint>
 #include <list>
 #include <map>
 #include <tuple>
 #include <utility>
 
 using namespace llvm;
 
+Expected<uint64_t> FileCheckNumericVariable::eval() const {
+  if (Value)
+    return *Value;
+  return make_error<FileCheckUndefVarError>(Name);
+}
+
 bool FileCheckNumericVariable::setValue(uint64_t NewValue) {
   if (Value)
     return true;
   Value = NewValue;
   return false;
 }
 
 bool FileCheckNumericVariable::clearValue() {
   if (!Value)
     return true;
   Value = None;
   return false;
 }
 
-Expected<uint64_t> FileCheckNumExpr::eval() const {
-  assert(LeftOp && "Evaluating an empty numeric expression");
-  Optional<uint64_t> LeftOpValue = LeftOp->getValue();
-  // Variable is undefined.
-  if (!LeftOpValue)
-    return make_error<FileCheckUndefVarError>(LeftOp->getName());
-  return EvalBinop(*LeftOpValue, RightOp);
+Expected<uint64_t> FileCheckASTBinop::eval() const {
+  Expected<uint64_t> LeftOp = this->LeftOp->eval();
+  Expected<uint64_t> RightOp = this->RightOp->eval();
+
+  // Uses undefined variable(s).
+  if (!LeftOp || !RightOp) {
+    Error Err = Error::success();
+    if (!LeftOp)
+      Err = joinErrors(std::move(Err), LeftOp.takeError());
+    if (!RightOp)
+      Err = joinErrors(std::move(Err), RightOp.takeError());
+    return std::move(Err);
+  }
+
+  return EvalBinop(*LeftOp, *RightOp);
 }
 
 Expected<std::string> FileCheckNumericSubstitution::getResult() const {
-  Expected<uint64_t> EvaluatedValue = NumExpr->eval();
+  Expected<uint64_t> EvaluatedValue = NumExprAST->eval();
   if (!EvaluatedValue)
     return EvaluatedValue.takeError();
   return utostr(*EvaluatedValue);
 }
 
 Expected<std::string> FileCheckStringSubstitution::getResult() const {
   // Look up the value and escape it so that we can put it into the regex.
   Expected<StringRef> VarVal = Context->getPatternVarValue(FromStr);
   if (!VarVal)
     return VarVal.takeError();
   return Regex::escape(*VarVal);
 }
 
 bool FileCheckPattern::isValidVarNameStart(char C) {
   return C == '_' || isalpha(C);
 }
 
 Expected<StringRef> FileCheckPattern::parseVariable(StringRef &Str,
                                                     bool &IsPseudo,
                                                     const SourceMgr &SM) {
   if (Str.empty())
     return FileCheckParseError::get(SM, Str, "empty variable name");
 
for (unsigned E = Str.size(); I != E; ++I) {
     if (Str[I] != '_' && !isalnum(Str[I]))
       break;
     ParsedOneChar = true;
   }
 
   StringRef Name = Str.take_front(I);
   Str = Str.substr(I);
   return Name;
 }
 
 // StringRef holding all characters considered as horizontal whitespaces by
 // FileCheck input canonicalization.
 StringRef SpaceChars = " \t";
 
 // Parsing helper function that strips the first character in S and returns it.
 static char popFront(StringRef &S) {
   char C = S.front();
Error FileCheckPattern::parseNumericVariableDefinition(
   if (Context->DefinedVariableTable.find(Name) !=
       Context->DefinedVariableTable.end())
     return FileCheckParseError::get(
         SM, Name, "string variable with name '" + Name + "' already exists");
 
   return Error::success();
 }
 
-Expected<FileCheckNumericVariable *>
-FileCheckPattern::parseNumericVariableUse(StringRef &Expr,
+Expected<std::shared_ptr<FileCheckNumericVariable>>
+FileCheckPattern::parseNumericVariableUse(StringRef Name, bool IsPseudo,
                                           const SourceMgr &SM) const {
-  bool IsPseudo;
-  Expected<StringRef> ParseVarResult = parseVariable(Expr, IsPseudo, SM);
-  if (!ParseVarResult)
-    return ParseVarResult.takeError();
-  StringRef Name = *ParseVarResult;
-
   if (IsPseudo && !Name.equals("@LINE"))
     return FileCheckParseError::get(
         SM, Name, "invalid pseudo numeric variable '" + Name + "'");
 
   // This method is indirectly called from parsePattern for all numeric
   // variable definitions and uses in the order in which they appear in the
   // CHECK pattern. For each definition, the pointer to the class instance of
   // the corresponding numeric variable definition is stored in
   // GlobalNumericVariableTable. Therefore, the pointer we get below is for the
-  // class instance corresponding to the last definition of this variable use.
+  // AST class instance corresponding to the last definition of this variable
+  // use.
   auto VarTableIter = Context->GlobalNumericVariableTable.find(Name);
   if (VarTableIter == Context->GlobalNumericVariableTable.end())
     return FileCheckParseError::get(
         SM, Name, "using undefined numeric variable '" + Name + "'");
 
-  FileCheckNumericVariable *NumericVariable = VarTableIter->second;
+  std::shared_ptr<FileCheckNumericVariable> NumericVariable =
+      VarTableIter->second;
   if (!IsPseudo && NumericVariable->getDefLineNumber() == LineNumber)
     return FileCheckParseError::get(SM, Name,
                                     "numeric variable '" + Name +
                                         "' defined on the same line as used");
 
   return NumericVariable;
 }
 
+Expected<std::shared_ptr<FileCheckNumExprAST>>
+FileCheckPattern::parseNumericOperand(StringRef &Expr, AllowedOperand AO,
+                                      const SourceMgr &SM) const {
+  if (AO == LegacyVar || AO == Any) {
+    // Try to parse as a numeric variable use.
+    bool IsPseudo;
+    Expected<StringRef> ParseVarResult = parseVariable(Expr, IsPseudo, SM);
+    if (ParseVarResult) {
+      StringRef Name = *ParseVarResult;
+      return parseNumericVariableUse(Name, IsPseudo, SM);
+    }
+    if (AO != LegacyLiteral && AO != Any)
+      return ParseVarResult.takeError();
+    consumeError(ParseVarResult.takeError());
+  }
+
+  if (AO == LegacyLiteral || AO == Any) {
+    // Otherwise, parse it as a literal.
+    uint64_t LiteralValue;
+    if (!Expr.consumeInteger(10 /*Radix*/, LiteralValue))
+      return std::make_shared<FileCheckNumExprLiteral>(LiteralValue);
+  }
+
+  return FileCheckParseError::get(SM, Expr,
+                                  "invalid operand format '" + Expr + "'");
+}
+
 static uint64_t add(uint64_t LeftOp, uint64_t RightOp) {
   return LeftOp + RightOp;
 }
 
 static uint64_t sub(uint64_t LeftOp, uint64_t RightOp) {
   return LeftOp - RightOp;
 }
 
-Expected<FileCheckNumExpr *>
-FileCheckPattern::parseBinop(StringRef &Expr, const SourceMgr &SM) const {
-  Expected<FileCheckNumericVariable *> LeftParseResult =
-      parseNumericVariableUse(Expr, SM);
-  if (!LeftParseResult) {
-    return LeftParseResult.takeError();
-  }
-  FileCheckNumericVariable *LeftOp = *LeftParseResult;
+Expected<std::shared_ptr<FileCheckNumExprAST>>
+FileCheckPattern::parseBinop(StringRef &Expr,
+                             std::shared_ptr<FileCheckNumExprAST> LeftOp,
+                             bool Legacy, const SourceMgr &SM) const {
+  Expr = Expr.ltrim(SpaceChars);
+  if (Expr.empty())
+    return LeftOp;
 
   // Check if this is a supported operation and select a function to perform
   // it.
-  Expr = Expr.ltrim(SpaceChars);
-  if (Expr.empty())
-    return Context->makeNumExpr(add, LeftOp, 0);
   SMLoc OpLoc = SMLoc::getFromPointer(Expr.data());
   char Operator = popFront(Expr);
   binop_eval_t EvalBinop;
   switch (Operator) {
   case '+':
     EvalBinop = add;
     break;
   case '-':
     EvalBinop = sub;
     break;
   default:
     return FileCheckParseError::get(SM, OpLoc,
                                     Twine("unsupported numeric operation '") +
                                         Twine(Operator) + "'");
   }
 
   // Parse right operand.
   Expr = Expr.ltrim(SpaceChars);
   if (Expr.empty())
     return FileCheckParseError::get(SM, Expr,
                                     "missing operand in numeric expression");
-  uint64_t RightOp;
-  if (Expr.consumeInteger(10, RightOp))
-    return FileCheckParseError::get(
-        SM, Expr, "invalid offset in numeric expression '" + Expr + "'");
-  Expr = Expr.ltrim(SpaceChars);
-  if (!Expr.empty())
-    return FileCheckParseError::get(
-        SM, Expr,
-        "unexpected characters at end of numeric expression '" + Expr + "'");
+  // Second operand in legacy numeric expression is a literal.
+  AllowedOperand AO = Legacy ? LegacyLiteral : Any;
+  Expected<std::shared_ptr<FileCheckNumExprAST>> RightOpResult =
+      parseNumericOperand(Expr, AO, SM);
+  if (!RightOpResult)
+    return RightOpResult;
+  std::shared_ptr<FileCheckNumExprAST> RightOp = *RightOpResult;
 
-  return Context->makeNumExpr(EvalBinop, LeftOp, RightOp);
+  Expr = Expr.ltrim(SpaceChars);
+  return std::make_shared<FileCheckASTBinop>(EvalBinop, LeftOp, RightOp);
 }
 
-Expected<FileCheckNumExpr *> FileCheckPattern::parseNumericSubstitutionBlock(
+Expected<std::shared_ptr<FileCheckNumExprAST>>
+FileCheckPattern::parseNumericSubstitutionBlock(
     StringRef Expr,
-    Optional<FileCheckNumericVariable *> &DefinedNumericVariable,
+    Optional<FileCheckNumericVariable *> &DefinedNumericVariable, bool Legacy,
     const SourceMgr &SM) const {
+  std::shared_ptr<FileCheckNumExprAST> NumExprAST = nullptr;
   // Parse the numeric variable definition.
   DefinedNumericVariable = None;
   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;
     Error ParseError =
         parseNumericVariableDefinition(DefExpr, Name, Context, SM);
     if (ParseError)
       return std::move(ParseError);
 
-    DefinedNumericVariable =
-        Context->makeNumericVariable(this->LineNumber, Name);
-
     DefExpr = DefExpr.ltrim(SpaceChars);
     if (!DefExpr.empty())
       return FileCheckParseError::get(SM, DefExpr,
                                       "invalid numeric variable definition");
     UseExpr = UseExpr.ltrim(SpaceChars);
     if (!UseExpr.empty())
       return FileCheckParseError::get(
           SM, UseExpr,
           "unexpected string after variable definition: '" + UseExpr + "'");
-    return Context->makeNumExpr(add, nullptr, 0);
-  }
 
+    DefinedNumericVariable =
+        new FileCheckNumericVariable(this->LineNumber, Name);
+  } else {
     // Parse the numeric expression itself.
     Expr = Expr.ltrim(SpaceChars);
-  return parseBinop(Expr, SM);
+    // First operand in legacy numeric expression is the @LINE pseudo variable.
+    AllowedOperand AO = Legacy ? LegacyVar : Any;
+    Expected<std::shared_ptr<FileCheckNumExprAST>> ParseResult =
+        parseNumericOperand(Expr, AO, SM);
+    while (ParseResult && !Expr.empty()) {
+      ParseResult = parseBinop(Expr, *ParseResult, Legacy, SM);
+      // Legacy numeric expressions only allow 2 operands.
+      if (ParseResult && Legacy && !Expr.empty())
+        return FileCheckParseError::get(
+            SM, Expr,
+            "unexpected characters at end of numeric expression '" + Expr +
+                "'");
+    }
+    if (!ParseResult)
+      return ParseResult;
+    NumExprAST = *ParseResult;
+  }
+
+  return NumExprAST;
 }
 
 bool FileCheckPattern::parsePattern(StringRef PatternStr, StringRef Prefix,
                                     SourceMgr &SM,
                                     const FileCheckRequest &Req) {
   bool MatchFullLinesHere = Req.MatchFullLines && CheckTy != Check::CheckNot;
 
   PatternLoc = SMLoc::getFromPointer(PatternStr.data());
 
   // Create fake @LINE pseudo variable definition.
   StringRef LinePseudo = "@LINE";
   uint64_t LineNumber64 = LineNumber;
-  FileCheckNumericVariable *LinePseudoVar =
-      Context->makeNumericVariable(LinePseudo, LineNumber64);
+  std::shared_ptr<FileCheckNumericVariable> LinePseudoVar =
+      std::make_shared<FileCheckNumericVariable>(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);
 
if (PatternStr.startswith("[[")) {
         return true;
       }
       // Strip the substitution block we are parsing. End points to the start
       // of the "]]" closing the expression so account for it in computing the
       // index of the first unparsed character.
       PatternStr = UnparsedPatternStr.substr(End + 2);
 
       bool IsDefinition = false;
+      bool Legacy = false;
       StringRef DefName;
       StringRef SubstStr;
       StringRef MatchRegexp;
       size_t SubstInsertIdx = RegExStr.size();
 
       // Parse string variable or legacy numeric expression.
       if (!IsNumBlock) {
         size_t VarEndIdx = MatchStr.find(":");
if (PatternStr.startswith("[[")) {
                 "numeric variable with name '" + Name + "' already exists");
             return true;
           }
           DefName = Name;
           MatchRegexp = MatchStr;
         } else {
           if (IsPseudo) {
             MatchStr = OrigMatchStr;
-            IsNumBlock = true;
+            Legacy = IsNumBlock = true;
           } else
             SubstStr = Name;
         }
       }
 
       // Parse numeric substitution block.
-      FileCheckNumExpr *NumExpr;
+      std::shared_ptr<FileCheckNumExprAST> NumExprAST;
       Optional<FileCheckNumericVariable *> DefinedNumericVariable;
       if (IsNumBlock) {
-        Expected<FileCheckNumExpr *> ParseResult =
-            parseNumericSubstitutionBlock(MatchStr, DefinedNumericVariable, SM);
+        Expected<std::shared_ptr<FileCheckNumExprAST>> ParseResult =
+            parseNumericSubstitutionBlock(MatchStr, DefinedNumericVariable,
+                                          Legacy, SM);
         if (!ParseResult) {
           logAllUnhandledErrors(ParseResult.takeError(), errs());
           return true;
         }
-        NumExpr = *ParseResult;
+        NumExprAST = *ParseResult;
         if (DefinedNumericVariable) {
           IsDefinition = true;
           DefName = (*DefinedNumericVariable)->getName();
           MatchRegexp = StringRef("[0-9]+");
         } else
           SubstStr = MatchStr;
       }
 
if (PatternStr.startswith("[[")) {
             return true;
           }
           AddBackrefToRegEx(CaptureParenGroup);
         } else {
           // Handle substitution of string variables ([[<var>]]) defined in
           // previous CHECK patterns, and substitution of numeric expressions.
           FileCheckSubstitution *Substitution =
               IsNumBlock
-                  ? Context->makeNumericSubstitution(SubstStr, NumExpr,
+                  ? Context->makeNumericSubstitution(SubstStr, NumExprAST,
                                                      SubstInsertIdx)
                   : Context->makeStringSubstitution(SubstStr, SubstInsertIdx);
           Substitutions.push_back(Substitution);
         }
         continue;
       }
 
       // Handle variable definitions: [[<def>:(...)]] and
       // [[#(...)<def>:(...)]].
       if (IsNumBlock) {
         FileCheckNumExprMatch NumExprDef = {*DefinedNumericVariable, CurParen};
         NumericVariableDefs[DefName] = NumExprDef;
         // This store is done here rather than in match() to allow
         // parseNumericVariableUse() to get the pointer to the class instance
         // of the right variable definition corresponding to a given numeric
         // variable use.
-        Context->GlobalNumericVariableTable[DefName] = *DefinedNumericVariable;
+        Context->GlobalNumericVariableTable[DefName] =
+            std::shared_ptr<FileCheckNumericVariable>(*DefinedNumericVariable);
       } else {
         VariableDefs[DefName] = CurParen;
         // Mark the string variable as defined to detect collisions between
         // string and numeric variables in parseNumericVariableUse() and
         // DefineCmdlineVariables() when the latter is created later than the
         // former. We cannot reuse GlobalVariableTable for this by populating
         // it with an empty string since we would then lose the ability to
         // detect the use of an undefined variable in match().
void FileCheckPattern::printSubstitutions(const SourceMgr &SM, StringRef Buffer,
   // Print what we know about substitutions.
   if (!Substitutions.empty()) {
     for (const auto &Substitution : Substitutions) {
       SmallString<256> Msg;
       raw_svector_ostream OS(Msg);
       Expected<std::string> MatchedValue = Substitution->getResult();
 
       // Substitution failed or is not known at match time, print the undefined
-      // variable it uses.
+      // variables it uses.
       if (!MatchedValue) {
-        Error UndefVarError = MatchedValue.takeError();
-        assert(UndefVarError.isA<FileCheckUndefVarError>() &&
-               "Unexpected error");
-        logAllUnhandledErrors(std::move(UndefVarError), OS,
-                              "uses undefined variable ");
+        Error UndefVarErrors = MatchedValue.takeError();
+        /*assert(UndefVarErrors.isA<FileCheckUndefVarError>() &&
+               "Unexpected error");*/
+        OS << "uses undefined variable(s):";
+        handleAllErrors(std::move(UndefVarErrors),
+                        [&](const FileCheckUndefVarError &E) {
+                          OS << " ";
+                          E.log(OS);
+                        },
+                        [](const ErrorInfoBase &E) {
+                          assert(false && "Unexpected error");
+                        });
+        OS << "\n";
       } else {
         // Substitution succeeded. Print substituted value.
         OS << "with \"";
         OS.write_escaped(Substitution->getFromString()) << "\" equal to \"";
         OS.write_escaped(*MatchedValue) << "\"";
       }
 
       if (MatchRange.isValid())

 FileCheckPatternContext::getPatternVarValue(StringRef VarName) {
   auto VarIter = GlobalVariableTable.find(VarName);
   if (VarIter == GlobalVariableTable.end())
     return make_error<FileCheckUndefVarError>(VarName);
 
   return VarIter->second;
 }
 
-FileCheckNumExpr *
-FileCheckPatternContext::makeNumExpr(binop_eval_t EvalBinop,
-                                     FileCheckNumericVariable *OperandLeft,
-                                     uint64_t OperandRight) {
-  NumExprs.push_back(llvm::make_unique<FileCheckNumExpr>(EvalBinop, OperandLeft,
-                                                         OperandRight));
-  return NumExprs.back().get();
-}
-
-template <class... Types>
-FileCheckNumericVariable *
-FileCheckPatternContext::makeNumericVariable(Types... args) {
-  NumericVariables.push_back(
-      llvm::make_unique<FileCheckNumericVariable>(args...));
-  return NumericVariables.back().get();
-}
-
 FileCheckSubstitution *
 FileCheckPatternContext::makeStringSubstitution(StringRef VarName,
                                                 size_t InsertIdx) {
   Substitutions.push_back(
       llvm::make_unique<FileCheckStringSubstitution>(this, VarName, InsertIdx));
   return Substitutions.back().get();
 }
 
 FileCheckSubstitution *FileCheckPatternContext::makeNumericSubstitution(
-    StringRef Expr, FileCheckNumExpr *NumExpr, size_t InsertIdx) {
+    StringRef Expr, std::shared_ptr<FileCheckNumExprAST> NumExprAST,
+    size_t InsertIdx) {
   Substitutions.push_back(llvm::make_unique<FileCheckNumericSubstitution>(
-      this, Expr, NumExpr, InsertIdx));
+      this, Expr, NumExprAST, InsertIdx));
   return Substitutions.back().get();
 }
 
 size_t FileCheckPattern::FindRegexVarEnd(StringRef Str, SourceMgr &SM) {
   // Offset keeps track of the current offset within the input Str
   size_t Offset = 0;
   // [...] Nesting depth
   size_t BracketDepth = 0;
if (CmdlineDef[0] == '#') {
       if (CmdlineVal.getAsInteger(10, Val)) {
         Errs = joinErrors(std::move(Errs),
                           FileCheckParseError::get(
                               SM, CmdlineVal,
                               "invalid value in numeric variable definition '" +
                                   CmdlineVal + "'"));
         continue;
       }
-      auto DefinedNumericVariable = makeNumericVariable(0, VarName);
+      auto DefinedNumericVariable =
+          std::make_shared<FileCheckNumericVariable>(0, VarName);
       DefinedNumericVariable->setValue(Val);
 
       // Record this variable definition.
       GlobalNumericVariableTable[DefinedNumericVariable->getName()] =
           DefinedNumericVariable;
     } else {
       // String variable definition.
       std::pair<StringRef, StringRef> CmdlineNameVal = CmdlineDef.split('=');

llvm/test/FileCheck/line-count.txt

 45
 46 BAD8: [[@LIN]]
 47 ERR8: line-count.txt:[[#@LINE-1]]:12: error: invalid pseudo numeric variable '@LIN'
 48
 49 BAD9: [[@LINE*2]]
 50 ERR9: line-count.txt:[[#@LINE-1]]:17: error: unsupported numeric operation '*'
 51
 52 BAD10: [[@LINE-x]]
-53 ERR10: line-count.txt:[[#@LINE-1]]:19: error: invalid offset in numeric expression 'x'
+53 ERR10: line-count.txt:[[#@LINE-1]]:19: error: invalid operand format 'x'
 54
 55 BAD11: [[@LINE-1x]]
 56 ERR11: line-count.txt:[[#@LINE-1]]:20: error: unexpected characters at end of numeric expression 'x'
 57
 58 CHECK: [[#@LINE]] CHECK
 59 CHECK: [[# @LINE]] CHECK
 60 CHECK: [[# @LINE ]] CHECK
 61
 62 CHECK: [[#@LINE-1]]
 63 CHECK: [[# @LINE-1]] CHECK
 64 CHECK: [[# @LINE -1]] CHECK
 65 CHECK: [[# @LINE - 1]] CHECK
 66 CHECK: [[# @LINE - 1 ]] CHECK

llvm/test/FileCheck/var-scope.txt

 local3
 global3
 LOCAL1: [[LOCAL]]3
 LOCAL2: local[[#LOCNUM+2]]
 LOCAL3: [[LOCAL]][[#LOCNUM+2]]
 GLOBAL: [[$GLOBAL]][[#$GLOBNUM+2]]
 
 ERRUNDEF: expected string not found in input
-ERRUNDEFLOCAL: uses undefined variable "LOCAL"
-ERRUNDEFLOCNUM: uses undefined variable "LOCNUM"
+ERRUNDEFLOCAL: uses undefined variable(s): "LOCAL"
+ERRUNDEFLOCNUM: uses undefined variable(s): "LOCNUM"

llvm/unittests/Support/FileCheckTest.cpp

 //===- llvm/unittest/Support/FileCheckTest.cpp - FileCheck tests --===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/FileCheck.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 namespace {
 
 class FileCheckTest : public ::testing::Test {};
 
+TEST_F(FileCheckTest, Literal) {
+  // Eval returns the literal's value.
+  auto Ten = FileCheckNumExprLiteral(10);
+  Expected<uint64_t> Value = Ten.eval();
+  EXPECT_TRUE(static_cast<bool>(Value));
+  EXPECT_EQ(10U, *Value);
+}
+
+static void expectUndefErrors(std::set<std::string> ExpectedUndefVarNames,
+                              Error Err) {
+  handleAllErrors(std::move(Err), [&](const FileCheckUndefVarError &E) {
+    ExpectedUndefVarNames.erase(E.getVarName());
+  });
+  EXPECT_TRUE(ExpectedUndefVarNames.empty());
+}
+
+static void expectUndefError(const Twine &ExpectedUndefVarName, Error Err) {
+  expectUndefErrors({ExpectedUndefVarName.str()}, std::move(Err));
+}
+
 TEST_F(FileCheckTest, NumericVariable) {
-  // Undefined variable: getValue and clearValue fails, setValue works.
+  // Undefined variable: eval and clearValue fails, error returned by eval
+  // holds the name of the undefined variable and setValue works.
   FileCheckNumericVariable FooVar = FileCheckNumericVariable(1, "FOO");
   EXPECT_EQ("FOO", FooVar.getName());
-  llvm::Optional<uint64_t> Value = FooVar.getValue();
+  Expected<uint64_t> Value = FooVar.eval();
   EXPECT_FALSE(Value);
-  EXPECT_TRUE(FooVar.clearValue());
+  expectUndefError("FOO", Value.takeError());
   EXPECT_FALSE(FooVar.setValue(42));
 
-  // Defined variable: getValue returns value set, setValue fails.
-  Value = FooVar.getValue();
-  EXPECT_TRUE(Value);
+  // Defined variable: eval returns value set and setValue fails.
+  Value = FooVar.eval();
+  EXPECT_TRUE(static_cast<bool>(Value));
   EXPECT_EQ(42U, *Value);
   EXPECT_TRUE(FooVar.setValue(43));
-  Value = FooVar.getValue();
-  EXPECT_TRUE(Value);
+  Value = FooVar.eval();
+  EXPECT_TRUE(static_cast<bool>(Value));
   EXPECT_EQ(42U, *Value);
 
-  // Clearing variable: getValue fails, clearValue again fails.
+  // Clearing variable: eval fails and clearValue again fails. Error returned
+  // by eval holds the name of the cleared variable.
   EXPECT_FALSE(FooVar.clearValue());
-  Value = FooVar.getValue();
+  Value = FooVar.eval();
   EXPECT_FALSE(Value);
+  expectUndefError("FOO", Value.takeError());
   EXPECT_TRUE(FooVar.clearValue());
 }
 
 uint64_t doAdd(uint64_t OpL, uint64_t OpR) { return OpL + OpR; }
 
-static void expectUndefError(const Twine &ExpectedStr, Error Err) {
-  handleAllErrors(std::move(Err), [&](const FileCheckUndefVarError &E) {
-    EXPECT_EQ(ExpectedStr.str(), E.getVarName());
-  });
-}
-
-TEST_F(FileCheckTest, NumExpr) {
-  FileCheckNumericVariable FooVar = FileCheckNumericVariable("FOO", 42);
-  FileCheckNumExpr NumExpr = FileCheckNumExpr(doAdd, &FooVar, 18);
+TEST_F(FileCheckTest, Binop) {
+  auto FooVar = std::make_shared<FileCheckNumericVariable>("FOO", 42);
+  auto BarVar = std::make_shared<FileCheckNumericVariable>("BAR", 18);
+  FileCheckASTBinop Binop = FileCheckASTBinop(doAdd, FooVar, BarVar);
 
   // Defined variable: eval returns right value.
-  Expected<uint64_t> Value = NumExpr.eval();
+  Expected<uint64_t> Value = Binop.eval();
   EXPECT_TRUE(static_cast<bool>(Value));
   EXPECT_EQ(60U, *Value);
 
-  // Undefined variable: eval fails, undefined variable returned. We call
-  // getUndefVarName first to check that it can be called without calling
-  // eval() first.
-  FooVar.clearValue();
-  Error EvalError = NumExpr.eval().takeError();
-  EXPECT_TRUE(errorToBool(std::move(EvalError)));
-  expectUndefError("FOO", std::move(EvalError));
+  // 1 undefined variable: eval fails, error contains name of undefined
+  // variable.
+  FooVar->clearValue();
+  Value = Binop.eval();
+  EXPECT_FALSE(Value);
+  expectUndefError("FOO", Value.takeError());
+
+  // 2 undefined variables: eval fails, error contains names of all undefined
+  // variables.
+  BarVar->clearValue();
+  Value = Binop.eval();
+  EXPECT_FALSE(Value);
+  expectUndefErrors({"FOO", "BAR"}, Value.takeError());
 }
 
 TEST_F(FileCheckTest, ValidVarNameStart) {
   EXPECT_TRUE(FileCheckPattern::isValidVarNameStart('a'));
   EXPECT_TRUE(FileCheckPattern::isValidVarNameStart('G'));
   EXPECT_TRUE(FileCheckPattern::isValidVarNameStart('_'));
   EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('2'));
   EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('$'));
bool parseNumVarDefExpect(StringRef Expr) {
     return errorToBool(FileCheckPattern::parseNumericVariableDefinition(
         ExprBufferRef, Name, &Context, SM));
   }
 
   bool parseSubstExpect(StringRef Expr) {
     StringRef ExprBufferRef = bufferize(SM, Expr);
     Optional<FileCheckNumericVariable *> DefinedNumericVariable;
     return errorToBool(P.parseNumericSubstitutionBlock(
-                            ExprBufferRef, DefinedNumericVariable, SM)
+                            ExprBufferRef, DefinedNumericVariable, false, SM)
                            .takeError());
   }
 
   bool parsePatternExpect(StringRef Pattern) {
     StringRef PatBufferRef = bufferize(SM, Pattern);
     return P.parsePattern(PatBufferRef, "CHECK", SM, Req);
   }
 
TEST_F(FileCheckTest, ParseExpr) {
   EXPECT_TRUE(Tester.parseSubstExpect("@LINE+x"));
 
   // Unexpected string at end of numeric expression.
   EXPECT_TRUE(Tester.parseSubstExpect("@LINE+5x"));
 
   // Valid expression.
   EXPECT_FALSE(Tester.parseSubstExpect("@LINE+5"));
   EXPECT_FALSE(Tester.parseSubstExpect("FOO+4"));
+  Tester.initNextPattern();
+  EXPECT_FALSE(Tester.parsePatternExpect("[[#FOO+FOO]]"));
+  EXPECT_FALSE(Tester.parsePatternExpect("[[#FOO+3-FOO]]"));
 }
 
 TEST_F(FileCheckTest, ParsePattern) {
   PatternTester Tester;
 
   // Space in pattern variable expression.
   EXPECT_TRUE(Tester.parsePatternExpect("[[ BAR]]"));
 
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("[[#2+@LINE]]"));
   EXPECT_TRUE(Tester.parsePatternExpect("[[#YUP:@LINE]]"));
 
   // Valid numeric expressions and numeric variable definition.
   EXPECT_FALSE(Tester.parsePatternExpect("[[#FOO]]"));
   EXPECT_FALSE(Tester.parsePatternExpect("[[#@LINE+2]]"));
   EXPECT_FALSE(Tester.parsePatternExpect("[[#NUMVAR:]]"));
 }
 
TEST_F(FileCheckTest, Substitution) {
   FileCheckStringSubstitution StringSubstitution =
       FileCheckStringSubstitution(&Context, "VAR404", 42);
   Expected<std::string> SubstValue = StringSubstitution.getResult();
   EXPECT_FALSE(static_cast<bool>(SubstValue));
   expectUndefError("VAR404", SubstValue.takeError());
 
   // Substitutions of defined pseudo and non-pseudo numeric variables return
   // the right value.
-  FileCheckNumericVariable LineVar = FileCheckNumericVariable("@LINE", 42);
-  FileCheckNumericVariable NVar = FileCheckNumericVariable("N", 10);
-  FileCheckNumExpr NumExprLine = FileCheckNumExpr(doAdd, &LineVar, 0);
-  FileCheckNumExpr NumExprN = FileCheckNumExpr(doAdd, &NVar, 3);
+  auto LineVar = std::make_shared<FileCheckNumericVariable>("@LINE", 42);
+  auto NVar = std::make_shared<FileCheckNumericVariable>("N", 10);
   FileCheckNumericSubstitution SubstitutionLine =
-      FileCheckNumericSubstitution(&Context, "@LINE", &NumExprLine, 12);
+      FileCheckNumericSubstitution(&Context, "@LINE", LineVar, 12);
   FileCheckNumericSubstitution SubstitutionN =
-      FileCheckNumericSubstitution(&Context, "N", &NumExprN, 30);
-  Expected<std::string> Value = SubstitutionLine.getResult();
-  EXPECT_TRUE(static_cast<bool>(Value));
-  EXPECT_EQ("42", *Value);
-  Value = SubstitutionN.getResult();
-  EXPECT_TRUE(static_cast<bool>(Value));
-  EXPECT_EQ("13", *Value);
-
-  // Substitution of an undefined numeric variable fails.
-  LineVar.clearValue();
-  SubstValue = SubstitutionLine.getResult().takeError();
+      FileCheckNumericSubstitution(&Context, "N", NVar, 30);
+  SubstValue = SubstitutionLine.getResult();
+  EXPECT_TRUE(static_cast<bool>(SubstValue));
+  EXPECT_EQ("42", *SubstValue);
+  SubstValue = SubstitutionN.getResult();
+  EXPECT_TRUE(static_cast<bool>(SubstValue));
+  EXPECT_EQ("10", *SubstValue);
+
+  // Substitution of an undefined numeric variable fails, error holds name of
+  // undefined variable.
+  LineVar->clearValue();
+  SubstValue = SubstitutionLine.getResult();
   EXPECT_FALSE(static_cast<bool>(SubstValue));
   expectUndefError("@LINE", SubstValue.takeError());
-  NVar.clearValue();
-  SubstValue = SubstitutionN.getResult().takeError();
+  NVar->clearValue();
+  SubstValue = SubstitutionN.getResult();
   EXPECT_FALSE(static_cast<bool>(SubstValue));
   expectUndefError("N", SubstValue.takeError());
 
   // Substitution of a defined string variable returns the right value.
   FileCheckPattern P = FileCheckPattern(Check::CheckPlain, &Context, 1);
   StringSubstitution = FileCheckStringSubstitution(&Context, "FOO", 42);
-  Value = StringSubstitution.getResult();
-  EXPECT_TRUE(static_cast<bool>(Value));
-  EXPECT_EQ("BAR", *Value);
+  SubstValue = StringSubstitution.getResult();
+  EXPECT_TRUE(static_cast<bool>(SubstValue));
+  EXPECT_EQ("BAR", *SubstValue);
 }
 
 TEST_F(FileCheckTest, FileCheckContext) {
   FileCheckPatternContext Cxt = FileCheckPatternContext();
   std::vector<std::string> GlobalDefines;
   SourceMgr SM;
 
   // Missing equal sign.
TEST_F(FileCheckTest, FileCheckContext) {
   // Check defined variables are present and undefined is absent.
   StringRef LocalVarStr = "LocalVar";
   StringRef LocalNumVarRef = bufferize(SM, "LocalNumVar");
   StringRef EmptyVarStr = "EmptyVar";
   StringRef UnknownVarStr = "UnknownVar";
   Expected<StringRef> LocalVar = Cxt.getPatternVarValue(LocalVarStr);
   FileCheckPattern P = FileCheckPattern(Check::CheckPlain, &Cxt, 1);
   Optional<FileCheckNumericVariable *> DefinedNumericVariable;
-  Expected<FileCheckNumExpr *> NumExpr = P.parseNumericSubstitutionBlock(
-      LocalNumVarRef, DefinedNumericVariable, SM);
-  Expected<StringRef> EmptyVar = Cxt.getPatternVarValue(EmptyVarStr);
-  Expected<StringRef> UnknownVar = Cxt.getPatternVarValue(UnknownVarStr);
+  Expected<std::shared_ptr<FileCheckNumExprAST>> NumExprAST =
+      P.parseNumericSubstitutionBlock(LocalNumVarRef, DefinedNumericVariable,
+                                      false /*Legacy*/, SM);
   EXPECT_TRUE(static_cast<bool>(LocalVar));
   EXPECT_EQ(*LocalVar, "FOO");
-  EXPECT_TRUE(static_cast<bool>(NumExpr));
-  Expected<uint64_t> NumExprVal = (*NumExpr)->eval();
+  Expected<StringRef> EmptyVar = Cxt.getPatternVarValue(EmptyVarStr);
+  Expected<StringRef> UnknownVar = Cxt.getPatternVarValue(UnknownVarStr);
+  EXPECT_TRUE(static_cast<bool>(NumExprAST));
+  Expected<uint64_t> NumExprVal = (*NumExprAST)->eval();
   EXPECT_TRUE(static_cast<bool>(NumExprVal));
   EXPECT_EQ(*NumExprVal, 18U);
   EXPECT_TRUE(static_cast<bool>(EmptyVar));
   EXPECT_EQ(*EmptyVar, "");
   EXPECT_TRUE(errorToBool(UnknownVar.takeError()));
 
   // Clear local variables and check they become absent.
   Cxt.clearLocalVars();
   LocalVar = Cxt.getPatternVarValue(LocalVarStr);
   EXPECT_TRUE(errorToBool(LocalVar.takeError()));
   // Check a numeric expression's evaluation fails if called after clearing of
   // local variables, if it was created before. This is important because local
   // variable clearing due to --enable-var-scope happens after numeric
   // expressions are linked to the numeric variables they use.
-  EXPECT_TRUE(errorToBool((*NumExpr)->eval().takeError()));
+  EXPECT_TRUE(errorToBool((*NumExprAST)->eval().takeError()));
   P = FileCheckPattern(Check::CheckPlain, &Cxt, 2);
-  NumExpr = P.parseNumericSubstitutionBlock(LocalNumVarRef,
-                                            DefinedNumericVariable, SM);
-  EXPECT_TRUE(errorToBool(NumExpr.takeError()));
+  NumExprAST = P.parseNumericSubstitutionBlock(
+      LocalNumVarRef, DefinedNumericVariable, false /*Legacy*/, SM);
+  EXPECT_TRUE(errorToBool(NumExprAST.takeError()));
   EmptyVar = Cxt.getPatternVarValue(EmptyVarStr);
   EXPECT_TRUE(errorToBool(EmptyVar.takeError()));
 
   // Redefine global variables and check variables are defined again.
   GlobalDefines.emplace_back(std::string("$GlobalVar=BAR"));
   GlobalDefines.emplace_back(std::string("#$GlobalNumVar=36"));
   EXPECT_FALSE(errorToBool(Cxt.defineCmdlineVariables(GlobalDefines, SM)));
   StringRef GlobalVarStr = "$GlobalVar";
   StringRef GlobalNumVarRef = bufferize(SM, "$GlobalNumVar");
   Expected<StringRef> GlobalVar = Cxt.getPatternVarValue(GlobalVarStr);
   EXPECT_TRUE(static_cast<bool>(GlobalVar));
   EXPECT_EQ(*GlobalVar, "BAR");
   P = FileCheckPattern(Check::CheckPlain, &Cxt, 3);
-  NumExpr = P.parseNumericSubstitutionBlock(GlobalNumVarRef,
-                                            DefinedNumericVariable, SM);
-  EXPECT_TRUE(static_cast<bool>(NumExpr));
-  NumExprVal = (*NumExpr)->eval();
+  NumExprAST = P.parseNumericSubstitutionBlock(
+      GlobalNumVarRef, DefinedNumericVariable, false /*Legacy*/, SM);
+  EXPECT_TRUE(static_cast<bool>(NumExprAST));
+  NumExprVal = (*NumExprAST)->eval();
   EXPECT_TRUE(static_cast<bool>(NumExprVal));
   EXPECT_EQ(*NumExprVal, 36U);
 
   // Clear local variables and check global variables remain defined.
   Cxt.clearLocalVars();
   EXPECT_FALSE(errorToBool(Cxt.getPatternVarValue(GlobalVarStr).takeError()));
   P = FileCheckPattern(Check::CheckPlain, &Cxt, 4);
-  NumExpr = P.parseNumericSubstitutionBlock(GlobalNumVarRef,
-                                            DefinedNumericVariable, SM);
-  EXPECT_TRUE(static_cast<bool>(NumExpr));
-  NumExprVal = (*NumExpr)->eval();
+  NumExprAST = P.parseNumericSubstitutionBlock(
+      GlobalNumVarRef, DefinedNumericVariable, false /*Legacy*/, SM);
+  EXPECT_TRUE(static_cast<bool>(NumExprAST));
+  NumExprVal = (*NumExprAST)->eval();
   EXPECT_TRUE(static_cast<bool>(NumExprVal));
   EXPECT_EQ(*NumExprVal, 36U);
 }
 } // namespace