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

105

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

llvm/lib/Support/FileCheck.cpp

738

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

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.

103–104

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.

130

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.

152

Could this use a MutableArrayRef?

193–194

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.

209–210

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"

441

What's a "legacy numeric substitution block"?

446

defined -> define

509

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

516

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

llvm/lib/Support/FileCheck.cpp

81

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.

208

Get rid of this blank line.

222

Magic number?

[thopre]

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

[thopre]

llvm/include/llvm/Support/FileCheck.h

43

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.

130

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?

441

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?

509

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–597

<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

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

441

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

509

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

511

I don't see a parameter named AllowedOperandFlag.

519

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

llvm/lib/Support/FileCheck.cpp

216

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

218

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

221

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

278–279

... in a @LINE expression ...

458

IsLineExpr

738

Delete the commented-out assertion.

746

llvm_unreachable instead of assert(false)?

[thopre]

Address all comments but shared_ptr

[thopre]

llvm/include/llvm/Support/FileCheck.h

441

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

221

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.

77

definition -> definitions

103–108

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

130

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

146

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

441

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.

442

substitued -> substituted

509

Perhaps worth making this an enum class.

513

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

llvm/lib/Support/FileCheck.cpp

224

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.

278–279

in a legacy

294–295

Err would be a more common variable name.

[jhenderson]

llvm/lib/Support/FileCheck.cpp

318

in a legacy

736–737

Trailing full stop required.

736–744

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

146

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

513

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

224

/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

65

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.

278–279

is always a literal

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

340

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

736–737

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.

53

getValue not getvalue.

407

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.

541–542

unique_ptr?

[thopre]

Address review comments.

[thopre]

llvm/include/llvm/Support/FileCheck.h

513

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

llvm/lib/Support/FileCheck.cpp

736–737

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

513

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

llvm/lib/Support/FileCheck.cpp

65

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

63

Doesn't look like you need this->.

211

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.

213

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?

458

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

213

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

211

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

504–507

This comment still mentions Expr and SM.

llvm/lib/Support/FileCheck.cpp

211

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

211

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

211

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

124

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

[thopre]

llvm/lib/Support/FileCheck.cpp

124

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

124

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

124

Maybe return VariableProperties{Name, IsPseudo}would work?

[jhenderson]

llvm/lib/Support/FileCheck.cpp

124

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 numeric variables previously
+defined 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.
+  virtual Optional<uint64_t> eval() const = 0;
+
+  /// Appends names of undefined variables used in the expression represented
+  /// by this AST. Must be overriden in any subclass representing an expression
+  /// that can contain a variable.
+  virtual void
+  appendUndefVarNames(std::vector<StringRef> &UndefVarNames) const {}
+};
+
+/// 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 an unsigned literal.
+  FileCheckNumExprLiteral(uint64_t Val) : Value(Val) {}
+
+  /// Evaluates and returns the value of the expression represented by this
+  /// AST. Therefore, \returns the literal's value.
+  Optional<uint64_t> eval() const { return Value; }
+};
+
+/// 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.
   StringRef getName() const { return Name; }
 
-  /// \returns value of this numeric variable.
-  Optional<uint64_t> getValue() const { return Value; }
+  /// Evaluates and returns the value of the expression represented by this
+  /// AST. Therefore, \returns this variable's value.
+  Optional<uint64_t> eval() const { return Value; }
+
+  /// 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.
   size_t getDefLineNumber() { return DefLineNumber; }
 };
 
 /// Type of functions evaluating a given binary operation.
 using binop_eval_t = uint64_t (*)(uint64_t, uint64_t);
 
-/// Class representing a numeric expression consisting of either a single
-/// numeric variable or a binary operation between a numeric variable and an
-/// immediate.
-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 None 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 None if any numeric variable used is undefined, or the
+  /// expression value otherwise.
   Optional<uint64_t> eval() const;
 
-  /// \returns the name of the undefined variable used in this expression if
-  /// any or an empty string otherwise.
-  StringRef getUndefVarName() const;
+  /// Appends names of undefined variables used in any of the operands of this
+  /// binary operation.
+  void appendUndefVarNames(std::vector<StringRef> &UndefVarNames) const;
 };
 
 class FileCheckPatternContext;
 
 /// Class representing a substitution to perform in the RegExStr string.
 class FileCheckSubstitution {
 protected:
   /// Pointer to a class instance holding, among other things, the table with
public:
 
   /// \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 None if substitution failed.
   virtual Optional<std::string> getResult() const = 0;
 
-  /// \returns the name of the variable used in this substitution if undefined,
-  /// or an empty string otherwise.
-  virtual StringRef getUndefVarName() const = 0;
+  /// Records in \p UndefVarNames the name of the variables used in this
+  /// substitution, if undefined.
+  virtual void
+  getUndefVarNames(std::vector<StringRef> &UndefVarNames) 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 None if the variable is undefined.
   Optional<std::string> getResult() const override;
 
-  /// \returns the name of the string variable used in this substitution if
-  /// undefined, or an empty string otherwise.
-  StringRef getUndefVarName() const override;
+  /// Records in \p UndefVarNames the name of the string variables used in this
+  /// substitution, if undefined.
+  void getUndefVarNames(std::vector<StringRef> &UndefVarNames) 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 None if evaluation failed.
   Optional<std::string> getResult() const override;
 
-  /// \returns the name of the numeric variable used in this substitution if
-  /// undefined, or an empty string otherwise.
-  StringRef getUndefVarName() const override;
+  /// Records in \p UndefVarNames the name of the numeric variables used in
+  /// this substitution, if undefined.
+  void getUndefVarNames(std::vector<StringRef> &UndefVarNames) const override;
 };
 
 //===----------------------------------------------------------------------===//
 // Pattern handling code.
 //===----------------------------------------------------------------------===//
 
 namespace Check {
 
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 None if no such
   /// variable has been defined.
   Optional<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. Reports any error to \p SM and \returns whether an
   /// error occured.
   bool 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 FileCheckPattern {
   SMLoc PatternLoc;
 
   /// A fixed string to match as the pattern or empty if this pattern requires
   /// a regex match.
   StringRef FixedStr;
public:
   /// Parses \p Expr for the definition of a numeric variable, returning an
   /// error if \p Context already holds a string variable with the same name.
   /// \returns whether parsing fails, in which case errors are reported on
   /// \p SM. Otherwise, sets \p Name to the name of the parsed numeric
   /// variable.
   static bool parseNumericVariableDefinition(StringRef &Expr, StringRef &Name,
                                              FileCheckPatternContext *Context,
                                              const SourceMgr &SM);
-  /// Parses \p Expr for a numeric substitution block. \returns the class
-  /// representing the AST of the numeric expression whose value must be
-  /// substituted, or nullptr if parsing fails, in which case errors are
-  /// reported on \p SM. Sets \p DefinedNumericVariable to point to the class
-  /// representing the numeric variable defined in this numeric substitution
-  /// block, or nullptr if this block does not define any variable.
-  FileCheckNumExpr *parseNumericSubstitutionBlock(
-      StringRef Expr, FileCheckNumericVariable *&DefinedNumericVariable,
+  /// Parses \p Expr for a numeric substitution block. Parameter \p Legacy
+  /// indicates whether Expr should be a legacy numeric substitution block.
+  /// Sets \p NumExprAST to point to the class instance representing the
+  /// numeric expression whose value must be substituted. Also sets
+  /// \p DefinedNumericVariable to point to the class representing the numeric
+  /// variable defined in this numeric substitution block, or nullptr if this
+  /// block does not defined any variable. \returns whether parsing fails, in
+  /// which case errors are reported on \p SM.
+  bool parseNumericSubstitutionBlock(
+      StringRef Expr, std::shared_ptr<FileCheckNumExprAST> &NumExprAST,
+      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:
   /// Finds the closing sequence of a regex variable usage or definition.
   ///
   /// \p Str has to point in the beginning of the definition (right after the
   /// opening sequence). \p SM holds the SourceMgr used for error repporting.
   ///  \returns the offset of the closing sequence within Str, or npos if it
   /// was not found.
   size_t FindRegexVarEnd(StringRef Str, SourceMgr &SM);
 
   /// Parses \p Expr for the use of a numeric variable. \returns the pointer to
   /// the class instance representing that variable if successful, or nullptr
   /// otherwise, in which case errors are reported on \p SM.
-  FileCheckNumericVariable *parseNumericVariableUse(StringRef &Expr,
-                                                    const SourceMgr &SM) const;
-  /// Parses \p Expr for a binary operation.
-  /// \returns the class representing the binary operation of the numeric
-  /// expression, or nullptr if parsing fails, in which case errors are
-  /// reported on \p SM.
-  FileCheckNumExpr *parseBinop(StringRef &Expr, const SourceMgr &SM) const;
+  std::shared_ptr<FileCheckNumericVariable>
+  parseNumericVariableUse(StringRef &Expr, const SourceMgr &SM) const;
+  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 nullptr if parsing fails in which case errors are reported
+  /// on \p SM.
+  std::shared_ptr<FileCheckNumExprAST>
+  parseNumericOperand(StringRef &Expr, enum AllowedOperand AO,
+  /// 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 nullptr if
+  /// parsing fails, in which case errors are reported on \p SM.
+  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;
 
+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 = None;
   return false;
 }
 
-Optional<uint64_t> FileCheckNumExpr::eval() const {
-  assert(LeftOp && "Evaluating an empty numeric expression");
-  Optional<uint64_t> LeftOpValue = LeftOp->getValue();
-  // Variable is undefined.
-  if (!LeftOpValue)
+Optional<uint64_t> FileCheckASTBinop::eval() const {
+  Optional<uint64_t> LeftOp = this->LeftOp->eval();
+  Optional<uint64_t> RightOp = this->RightOp->eval();
+
+  // Uses undefined variable.
+  if (!LeftOp || !RightOp)
     return None;
-  return EvalBinop(*LeftOpValue, RightOp);
+
+  return EvalBinop(*LeftOp, *RightOp);
 }
 
-StringRef FileCheckNumExpr::getUndefVarName() const {
-  if (!LeftOp->getValue())
-    return LeftOp->getName();
-  return StringRef();
+void FileCheckASTBinop::appendUndefVarNames(
+    std::vector<StringRef> &UndefVarNames) const {
+  LeftOp->appendUndefVarNames(UndefVarNames);
+  RightOp->appendUndefVarNames(UndefVarNames);
 }
 
 Optional<std::string> FileCheckNumericSubstitution::getResult() const {
-  Optional<uint64_t> EvaluatedValue = NumExpr->eval();
+  Optional<uint64_t> EvaluatedValue = NumExprAST->eval();
   if (!EvaluatedValue)
     return None;
   return utostr(*EvaluatedValue);
 }
 
 Optional<std::string> FileCheckStringSubstitution::getResult() const {
   // Look up the value and escape it so that we can put it into the regex.
   Optional<StringRef> VarVal = Context->getPatternVarValue(FromStr);
   if (!VarVal)
     return None;
   return Regex::escape(*VarVal);
 }
 
-StringRef FileCheckNumericSubstitution::getUndefVarName() const {
+void FileCheckNumericSubstitution::getUndefVarNames(
+    std::vector<StringRef> &UndefVarNames) const {
+  UndefVarNames.clear();
   // Although a use of an undefined numeric variable is detected at parse
-  // time, a numeric variable can be undefined later by ClearLocalVariables.
-  return NumExpr->getUndefVarName();
+  // time, a numeric variable can get undefined later by ClearLocalVariables.
+  NumExprAST->appendUndefVarNames(UndefVarNames);
 }
 
-StringRef FileCheckStringSubstitution::getUndefVarName() const {
+void FileCheckStringSubstitution::getUndefVarNames(
+    std::vector<StringRef> &UndefVarNames) const {
+  UndefVarNames.clear();
   if (!Context->getPatternVarValue(FromStr))
-    return FromStr;
-
-  return StringRef();
+    UndefVarNames.emplace_back(FromStr);
 }
 
 bool FileCheckPattern::isValidVarNameStart(char C) {
   return C == '_' || isalpha(C);
 }
 
 bool FileCheckPattern::parseVariable(StringRef &Str, StringRef &Name,
                                      bool &IsPseudo) {
for (unsigned E = Str.size(); I != E; ++I) {
     if (Str[I] != '_' && !isalnum(Str[I]))
       break;
     ParsedOneChar = true;
   }
 
   Name = Str.take_front(I);
   Str = Str.substr(I);
   return false;
 }
 
 // StringRef holding all characters considered as horizontal whitespaces by
 // FileCheck input canonicalization.
 StringRef SpaceChars = " \t";
 
 // Parsing helper function that strips the first character in S and returns it.
 static char popFront(StringRef &S) {
   char C = S.front();
if (Context->DefinedVariableTable.find(Name) !=
     SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
                     "string variable with name '" + Name + "' already exists");
     return true;
   }
 
   return false;
 }
 
-FileCheckNumericVariable *
+std::shared_ptr<FileCheckNumericVariable>
 FileCheckPattern::parseNumericVariableUse(StringRef &Expr,
                                           const SourceMgr &SM) const {
   bool IsPseudo;
   StringRef Name;
-  if (parseVariable(Expr, Name, IsPseudo)) {
-    SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
-                    "invalid variable name");
+  if (parseVariable(Expr, Name, IsPseudo))
     return nullptr;
-  }
 
   if (IsPseudo && !Name.equals("@LINE")) {
     SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
                     "invalid pseudo numeric variable '" + Name + "'");
     return nullptr;
   }
 
   // This method is indirectly called from parsePattern for all numeric
   // 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()) {
     SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
                     "using undefined numeric variable '" + Name + "'");
     return nullptr;
   }
 
-  FileCheckNumericVariable *NumericVariable = VarTableIter->second;
+  std::shared_ptr<FileCheckNumericVariable> NumericVariable =
+      VarTableIter->second;
   if (!IsPseudo && NumericVariable->getDefLineNumber() == LineNumber) {
     SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
                     "numeric variable '" + Name +
                         "' defined on the same line as used");
     return nullptr;
   }
 
   return NumericVariable;
 }
 
+std::shared_ptr<FileCheckNumExprAST>
+FileCheckPattern::parseNumericOperand(StringRef &Expr, enum AllowedOperand AO,
+                                      const SourceMgr &SM) const {
+
+  // Try to parse as a numeric variable use.
+  if (AO == LegacyVar || AO == Any) {
+    // Error reporting done in ParseNumericVariableUse().
+    std::shared_ptr<FileCheckNumericVariable> NumericVariable =
+        parseNumericVariableUse(Expr, SM);
+    if (NumericVariable)
+      return NumericVariable;
+  }
+
+  // Otherwise, parse it as a literal.
+  if (AO != LegacyLiteral && AO != Any)
+    return nullptr;
+  uint64_t LiteralValue;
+  if (Expr.consumeInteger(10, LiteralValue))
+    return nullptr;
+  return std::make_shared<FileCheckNumExprLiteral>(LiteralValue);
+}
+
 static uint64_t add(uint64_t LeftOp, uint64_t RightOp) {
   return LeftOp + RightOp;
 }
 
 static uint64_t sub(uint64_t LeftOp, uint64_t RightOp) {
   return LeftOp - RightOp;
 }
 
-FileCheckNumExpr *FileCheckPattern::parseBinop(StringRef &Expr,
-                                               const SourceMgr &SM) const {
-  FileCheckNumericVariable *LeftOp = parseNumericVariableUse(Expr, SM);
-  if (!LeftOp) {
-    // Error reporting done in parseNumericVariableUse().
-    return nullptr;
-  }
+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:
     SM.PrintMessage(OpLoc, SourceMgr::DK_Error,
                     Twine("unsupported numeric operation '") + Twine(Operator) +
                         "'");
     return nullptr;
   }
 
   // Parse right operand.
   Expr = Expr.ltrim(SpaceChars);
   if (Expr.empty()) {
     SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
                     "missing operand in numeric expression");
     return nullptr;
   }
-  uint64_t RightOp;
-  if (Expr.consumeInteger(10, RightOp)) {
+  // Second operand in legacy numeric expression is a literal.
+  enum AllowedOperand AO = Legacy ? LegacyLiteral : Any;
+  std::shared_ptr<FileCheckNumExprAST> RightOp =
+      parseNumericOperand(Expr, AO, SM);
+  if (RightOp == nullptr) {
     SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
-                    "invalid offset in numeric expression '" + Expr + "'");
+                    "invalid operand format '" + Expr + "'");
     return nullptr;
   }
   Expr = Expr.ltrim(SpaceChars);
-  if (!Expr.empty()) {
-    SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
-                    "unexpected characters at end of numeric expression '" +
-                        Expr + "'");
-    return nullptr;
-  }
+  return std::make_shared<FileCheckASTBinop>(EvalBinop, LeftOp, RightOp);
 
-  return Context->makeNumExpr(EvalBinop, LeftOp, RightOp);
-}
-
-FileCheckNumExpr *FileCheckPattern::parseNumericSubstitutionBlock(
-    StringRef Expr, FileCheckNumericVariable *&DefinedNumericVariable,
+bool FileCheckPattern::parseNumericSubstitutionBlock(
+    StringRef Expr, std::shared_ptr<FileCheckNumExprAST> &NumExprAST,
+    FileCheckNumericVariable *&DefinedNumericVariable, bool Legacy,
     const SourceMgr &SM) const {
   // Parse the numeric variable definition.
   DefinedNumericVariable = nullptr;
   size_t DefEnd = Expr.find(':');
   if (DefEnd != StringRef::npos) {
     StringRef DefExpr = Expr.substr(0, DefEnd);
     StringRef UseExpr = Expr = Expr.substr(DefEnd + 1);
 
     DefExpr = DefExpr.ltrim(SpaceChars);
     StringRef Name;
     if (parseNumericVariableDefinition(DefExpr, Name, Context, SM)) {
       // Invalid variable definition. Error reporting done in parsing function.
-      return nullptr;
+      return true;
     }
 
-    DefinedNumericVariable =
-        Context->makeNumericVariable(this->LineNumber, Name);
-
     DefExpr = DefExpr.ltrim(SpaceChars);
     if (!DefExpr.empty()) {
       SM.PrintMessage(SMLoc::getFromPointer(DefExpr.data()),
                       SourceMgr::DK_Error,
                       "invalid numeric variable definition");
-      return nullptr;
+      return true;
     }
     UseExpr = UseExpr.ltrim(SpaceChars);
     if (!UseExpr.empty()) {
       SM.PrintMessage(
           SMLoc::getFromPointer(UseExpr.data()), SourceMgr::DK_Error,
           "unexpected string after variable definition: '" + UseExpr + "'");
-      return nullptr;
-    }
-    return Context->makeNumExpr(add, nullptr, 0);
+      return true;
     }
 
+    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.
+    enum AllowedOperand AO = Legacy ? LegacyVar : Any;
+    NumExprAST = parseNumericOperand(Expr, AO, SM);
+    while (NumExprAST != nullptr && !Expr.empty()) {
+      NumExprAST = parseBinop(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 true;
+      }
+    }
+    if (NumExprAST == nullptr) {
+      SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
+                      "invalid operand format '" + Expr + "'");
+      return true;
+    }
+  }
+
+  return false;
 }
 
 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;
       FileCheckNumericVariable *DefinedNumericVariable;
       if (IsNumBlock) {
-        NumExpr =
-            parseNumericSubstitutionBlock(MatchStr, DefinedNumericVariable, SM);
-        if (NumExpr == nullptr)
+        if (parseNumericSubstitutionBlock(MatchStr, NumExprAST,
+                                          DefinedNumericVariable, Legacy, SM))
           return true;
         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);
       Optional<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) {
-        StringRef UndefVarName = Substitution->getUndefVarName();
-        if (UndefVarName.empty())
+        std::vector<StringRef> UndefVarNames;
+        Substitution->getUndefVarNames(UndefVarNames);
+        if (UndefVarNames.empty())
           continue;
+        for (auto UndefVarName : UndefVarNames) {
           OS << "uses undefined variable \"";
           OS.write_escaped(UndefVarName) << "\"";
+        }
       } else {
         // Substitution succeeded. Print substituted value.
         OS << "with \"";
         OS.write_escaped(Substitution->getFromString()) << "\" equal to \"";
         OS.write_escaped(*MatchedValue) << "\"";
       }
 
       if (MatchRange.isValid())
         SM.PrintMessage(MatchRange.Start, SourceMgr::DK_Note, OS.str(),
                         {MatchRange});

 FileCheckPatternContext::getPatternVarValue(StringRef VarName) {
   auto VarIter = GlobalVariableTable.find(VarName);
   if (VarIter == GlobalVariableTable.end())
     return None;
 
   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)) {
         SM.PrintMessage(SMLoc::getFromPointer(CmdlineVal.data()),
                         SourceMgr::DK_Error,
                         "invalid value in numeric variable definition '" +
                             CmdlineVal + "'");
         ErrorFound = true;
         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/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);
+  llvm::Optional<uint64_t> Value = Ten.eval();
+  EXPECT_TRUE(Value);
+  EXPECT_EQ(10U, *Value);
+
+  // Literal has no undefined variable.
+  std::vector<StringRef> UndefVarNames;
+  EXPECT_TRUE(UndefVarNames.empty());
+  Ten.appendUndefVarNames(UndefVarNames);
+  EXPECT_TRUE(UndefVarNames.empty());
+}
+
 TEST_F(FileCheckTest, NumericVariable) {
-  // Undefined variable: getValue and clearValue fails, setValue works.
+  // Undefined variable: eval and clearValue fails, appendUndefVarNames returns
+  // the variable and setValue works.
   FileCheckNumericVariable FooVar = FileCheckNumericVariable(1, "FOO");
   EXPECT_EQ("FOO", FooVar.getName());
-  llvm::Optional<uint64_t> Value = FooVar.getValue();
+  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: getValue returns value set, setValue fails.
-  Value = FooVar.getValue();
+  // 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.getValue();
+  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: getValue fails, clearValue again fails.
+  // Clearing variable: eval fails and clearValue again fails.
+  // appendUndefVarNames returns the variable again.
   EXPECT_FALSE(FooVar.clearValue());
-  Value = FooVar.getValue();
+  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, 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, no undefined variable
   // returned.
-  llvm::Optional<uint64_t> Value = NumExpr.eval();
+  llvm::Optional<uint64_t> Value = Binop.eval();
   EXPECT_TRUE(Value);
   EXPECT_EQ(60U, *Value);
-  StringRef UndefVar = NumExpr.getUndefVarName();
-  EXPECT_EQ("", UndefVar);
+  std::vector<StringRef> UndefVarNames;
+  Binop.appendUndefVarNames(UndefVarNames);
+  EXPECT_TRUE(UndefVarNames.empty());
 
-  // Undefined variable: eval fails, undefined variable returned. We call
+  // 1 undefined variable: eval fails, undefined variable returned. We call
   // getUndefVarName first to check that it can be called without calling
   // eval() first.
-  FooVar.clearValue();
-  UndefVar = NumExpr.getUndefVarName();
-  EXPECT_EQ("FOO", UndefVar);
-  Value = NumExpr.eval();
+  FooVar->clearValue();
+  Binop.appendUndefVarNames(UndefVarNames);
+  EXPECT_EQ(1U, UndefVarNames.size());
+  EXPECT_EQ("FOO", UndefVarNames[0]);
+  Value = Binop.eval();
   EXPECT_FALSE(Value);
+
+  // 2 undefined variables: eval fails, undefined variables returned.
+  BarVar->clearValue();
+  Value = Binop.eval();
+  EXPECT_FALSE(Value);
+  UndefVarNames.clear();
+  Binop.appendUndefVarNames(UndefVarNames);
+  EXPECT_EQ(2U, UndefVarNames.size());
+  EXPECT_EQ("FOO", UndefVarNames[0]);
+  EXPECT_EQ("BAR", UndefVarNames[1]);
 }
 
 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) {
     StringRef ExprBufferRef = bufferize(SM, Expr);
     StringRef Name;
     return FileCheckPattern::parseNumericVariableDefinition(ExprBufferRef, Name,
                                                             &Context, SM);
   }
 
   bool parseSubstExpect(StringRef Expr) {
     StringRef ExprBufferRef = bufferize(SM, Expr);
+    std::shared_ptr<FileCheckNumExprAST> NumExprAST;
     FileCheckNumericVariable *DefinedNumericVariable;
-    return P.parseNumericSubstitutionBlock(
-               ExprBufferRef, DefinedNumericVariable, SM) == nullptr;
+    return P.parseNumericSubstitutionBlock(ExprBufferRef, NumExprAST,
+                                           DefinedNumericVariable, false, SM);
   }
 
   bool parsePatternExpect(StringRef Pattern) {
     StringRef PatBufferRef = bufferize(SM, Pattern);
     return P.parsePattern(PatBufferRef, "CHECK", SM, Req);
   }
 
   bool matchExpect(StringRef Buffer) {
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) {
   SourceMgr SM;
   FileCheckPatternContext Context;
   std::vector<std::string> GlobalDefines;
   GlobalDefines.emplace_back(std::string("FOO=BAR"));
   Context.defineCmdlineVariables(GlobalDefines, SM);
 
-  // Substitution of an undefined string variable fails.
+  // Substitution of an undefined strin variable fails, getUndefVarNames
+  // returns that variable's name.
   FileCheckStringSubstitution StringSubstitution =
       FileCheckStringSubstitution(&Context, "VAR404", 42);
   EXPECT_FALSE(StringSubstitution.getResult());
-
-  // Substitutions of defined pseudo and non-pseudo numeric variables return
-  // the right value.
-  FileCheckNumericVariable LineVar = FileCheckNumericVariable("@LINE", 42);
-  FileCheckNumericVariable NVar = FileCheckNumericVariable("N", 10);
-  FileCheckNumExpr NumExprLine = FileCheckNumExpr(doAdd, &LineVar, 0);
-  FileCheckNumExpr NumExprN = FileCheckNumExpr(doAdd, &NVar, 3);
+  std::vector<StringRef> UndefVarNames;
+  EXPECT_TRUE(UndefVarNames.empty());
+  StringSubstitution.getUndefVarNames(UndefVarNames);
+  EXPECT_EQ(1U, UndefVarNames.size());
+  EXPECT_EQ("VAR404", UndefVarNames[0]);
+
+  // Substitution of defined pseudo and non-pseudo numeric variable returns the
+  // right value, getUndefVarNames does not return any variable.
+  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);
+      FileCheckNumericSubstitution(&Context, "N", NVar, 30);
   llvm::Optional<std::string> Value = SubstitutionLine.getResult();
   EXPECT_TRUE(Value);
   EXPECT_EQ("42", *Value);
+  EXPECT_FALSE(UndefVarNames.empty());
+  SubstitutionLine.getUndefVarNames(UndefVarNames);
+  EXPECT_TRUE(UndefVarNames.empty());
   Value = SubstitutionN.getResult();
   EXPECT_TRUE(Value);
-  EXPECT_EQ("13", *Value);
-
-  // Substitution of an undefined numeric variable fails.
-  LineVar.clearValue();
+  EXPECT_EQ("10", *Value);
+  UndefVarNames.push_back("ToClear");
+  EXPECT_FALSE(UndefVarNames.empty());
+  SubstitutionN.getUndefVarNames(UndefVarNames);
+  EXPECT_TRUE(UndefVarNames.empty());
+
+  // Substitution of an undefined numeric variable fails, getUndefVarNames
+  // returns that variable's name.
+  LineVar->clearValue();
   EXPECT_FALSE(SubstitutionLine.getResult());
-  NVar.clearValue();
+  EXPECT_TRUE(UndefVarNames.empty());
+  SubstitutionLine.getUndefVarNames(UndefVarNames);
+  EXPECT_EQ(1U, UndefVarNames.size());
+  EXPECT_EQ("@LINE", UndefVarNames[0]);
+  NVar->clearValue();
   EXPECT_FALSE(SubstitutionN.getResult());
+  UndefVarNames.clear();
+  EXPECT_TRUE(UndefVarNames.empty());
+  SubstitutionN.getUndefVarNames(UndefVarNames);
+  EXPECT_EQ(1U, UndefVarNames.size());
+  EXPECT_EQ("N", UndefVarNames[0]);
 
-  // Substitution of a defined string variable returns the right value.
+  // Substitution of a defined string variable returns the right value,
+  // getUndefVarNames does not return any variable.
   FileCheckPattern P = FileCheckPattern(Check::CheckPlain, &Context, 1);
   StringSubstitution = FileCheckStringSubstitution(&Context, "FOO", 42);
   Value = StringSubstitution.getResult();
   EXPECT_TRUE(Value);
   EXPECT_EQ("BAR", *Value);
+  EXPECT_FALSE(UndefVarNames.empty());
+  StringSubstitution.getUndefVarNames(UndefVarNames);
+  EXPECT_TRUE(UndefVarNames.empty());
 }
 
 TEST_F(FileCheckTest, UndefVars) {
   SourceMgr SM;
   FileCheckPatternContext Context;
   std::vector<std::string> GlobalDefines;
   GlobalDefines.emplace_back(std::string("FOO=BAR"));
   Context.defineCmdlineVariables(GlobalDefines, SM);
 
-  // getUndefVarName() on a string substitution with an undefined variable
-  // returns that variable.
+  // getUndefVarName() on a string variable substitution with an undefined
+  // variable returns that variable.
   FileCheckStringSubstitution StringSubstitution =
       FileCheckStringSubstitution(&Context, "VAR404", 42);
-  StringRef UndefVar = StringSubstitution.getUndefVarName();
-  EXPECT_EQ("VAR404", UndefVar);
+  std::vector<StringRef> UndefVarNames;
+  StringSubstitution.getUndefVarNames(UndefVarNames);
+  EXPECT_EQ(1U, UndefVarNames.size());
+  EXPECT_EQ("VAR404", UndefVarNames[0]);
 
-  // getUndefVarName() on a string substitution with a defined variable returns
-  // an empty string.
+  // getUndefVarName() on a string variable substitution with a defined
+  // variable returns an empty string.
   StringSubstitution = FileCheckStringSubstitution(&Context, "FOO", 42);
-  UndefVar = StringSubstitution.getUndefVarName();
-  EXPECT_EQ("", UndefVar);
+  UndefVarNames.clear();
+  StringSubstitution.getUndefVarNames(UndefVarNames);
+  EXPECT_TRUE(UndefVarNames.empty());
 
   // getUndefVarName() on a numeric substitution with a defined variable
   // returns an empty string.
-  FileCheckNumericVariable LineVar = FileCheckNumericVariable("@LINE", 42);
-  FileCheckNumExpr NumExpr = FileCheckNumExpr(doAdd, &LineVar, 0);
+  auto LineVar = std::make_shared<FileCheckNumericVariable>("@LINE", 42);
+  auto Zero = std::make_shared<FileCheckNumExprLiteral>(0);
+  auto Binop = std::make_shared<FileCheckASTBinop>(doAdd, LineVar, Zero);
   FileCheckNumericSubstitution NumericSubstitution =
-      FileCheckNumericSubstitution(&Context, "@LINE", &NumExpr, 12);
-  UndefVar = NumericSubstitution.getUndefVarName();
-  EXPECT_EQ("", UndefVar);
+      FileCheckNumericSubstitution(&Context, "@LINE", Binop, 12);
+  UndefVarNames.clear();
+  NumericSubstitution.getUndefVarNames(UndefVarNames);
+  EXPECT_TRUE(UndefVarNames.empty());
 
   // getUndefVarName() on a numeric substitution with an undefined variable
   // returns that variable.
-  LineVar.clearValue();
-  UndefVar = NumericSubstitution.getUndefVarName();
-  EXPECT_EQ("@LINE", UndefVar);
+  LineVar->clearValue();
+  UndefVarNames.clear();
+  NumericSubstitution.getUndefVarNames(UndefVarNames);
+  EXPECT_EQ(1U, UndefVarNames.size());
+  EXPECT_EQ("@LINE", UndefVarNames[0]);
 }
 
 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";
   llvm::Optional<StringRef> LocalVar = Cxt.getPatternVarValue(LocalVarStr);
   FileCheckPattern P = FileCheckPattern(Check::CheckPlain, &Cxt, 1);
+  std::shared_ptr<FileCheckNumExprAST> NumExprAST;
   FileCheckNumericVariable *DefinedNumericVariable;
-  FileCheckNumExpr *NumExpr = P.parseNumericSubstitutionBlock(
-      LocalNumVarRef, DefinedNumericVariable, SM);
-  llvm::Optional<StringRef> EmptyVar = Cxt.getPatternVarValue(EmptyVarStr);
-  llvm::Optional<StringRef> UnknownVar = Cxt.getPatternVarValue(UnknownVarStr);
+  EXPECT_FALSE(P.parseNumericSubstitutionBlock(LocalNumVarRef, NumExprAST,
+                                               DefinedNumericVariable,
+                                               false /*Legacy*/, SM));
   EXPECT_TRUE(LocalVar);
   EXPECT_EQ(*LocalVar, "FOO");
-  EXPECT_TRUE(NumExpr);
-  llvm::Optional<uint64_t> NumExprVal = NumExpr->eval();
+  llvm::Optional<uint64_t> NumExprVal = NumExprAST->eval();
   EXPECT_TRUE(NumExprVal);
   EXPECT_EQ(*NumExprVal, 18U);
+  llvm::Optional<StringRef> EmptyVar = Cxt.getPatternVarValue(EmptyVarStr);
+  llvm::Optional<StringRef> UnknownVar = Cxt.getPatternVarValue(UnknownVarStr);
   EXPECT_TRUE(EmptyVar);
   EXPECT_EQ(*EmptyVar, "");
   EXPECT_FALSE(UnknownVar);
 
   // Clear local variables and check they become absent.
   Cxt.clearLocalVars();
   LocalVar = Cxt.getPatternVarValue(LocalVarStr);
   EXPECT_FALSE(LocalVar);
   // Check a numeric expression's evaluation fails if called after clearing of
   // local variables, if it was created before. This is important because local
   // variable clearing due to --enable-var-scope happens after numeric
   // expressions are linked to the numeric variables they use.
-  EXPECT_FALSE(NumExpr->eval());
+  EXPECT_FALSE(NumExprAST->eval());
   P = FileCheckPattern(Check::CheckPlain, &Cxt, 2);
-  NumExpr = P.parseNumericSubstitutionBlock(LocalNumVarRef,
-                                            DefinedNumericVariable, SM);
-  EXPECT_FALSE(NumExpr);
+  EXPECT_TRUE(P.parseNumericSubstitutionBlock(LocalNumVarRef, NumExprAST,
+                                              DefinedNumericVariable,
+                                              false /*Legacy*/, SM));
   EmptyVar = Cxt.getPatternVarValue(EmptyVarStr);
   EXPECT_FALSE(EmptyVar);
 
   // Redefine global variables and check variables are defined again.
   GlobalDefines.emplace_back(std::string("$GlobalVar=BAR"));
   GlobalDefines.emplace_back(std::string("#$GlobalNumVar=36"));
   GotError = Cxt.defineCmdlineVariables(GlobalDefines, SM);
   EXPECT_FALSE(GotError);
   StringRef GlobalVarStr = "$GlobalVar";
   StringRef GlobalNumVarRef = bufferize(SM, "$GlobalNumVar");
   llvm::Optional<StringRef> GlobalVar = Cxt.getPatternVarValue(GlobalVarStr);
   EXPECT_TRUE(GlobalVar);
   EXPECT_EQ(*GlobalVar, "BAR");
   P = FileCheckPattern(Check::CheckPlain, &Cxt, 3);
-  NumExpr = P.parseNumericSubstitutionBlock(GlobalNumVarRef,
-                                            DefinedNumericVariable, SM);
-  EXPECT_TRUE(NumExpr);
-  NumExprVal = NumExpr->eval();
+  EXPECT_FALSE(P.parseNumericSubstitutionBlock(GlobalNumVarRef, NumExprAST,
+                                               DefinedNumericVariable,
+                                               false /*Legacy*/, SM));
+  NumExprVal = NumExprAST->eval();
   EXPECT_TRUE(NumExprVal);
   EXPECT_EQ(*NumExprVal, 36U);
 
   // Clear local variables and check global variables remain defined.
   Cxt.clearLocalVars();
   GlobalVar = Cxt.getPatternVarValue(GlobalVarStr);
   EXPECT_TRUE(GlobalVar);
   P = FileCheckPattern(Check::CheckPlain, &Cxt, 4);
-  NumExpr = P.parseNumericSubstitutionBlock(GlobalNumVarRef,
-                                            DefinedNumericVariable, SM);
-  EXPECT_TRUE(NumExpr);
-  NumExprVal = NumExpr->eval();
+  EXPECT_FALSE(P.parseNumericSubstitutionBlock(GlobalNumVarRef, NumExprAST,
+                                               DefinedNumericVariable,
+                                               false /*Legacy*/, SM));
+  NumExprVal = NumExprAST->eval();
   EXPECT_TRUE(NumExprVal);
   EXPECT_EQ(*NumExprVal, 36U);
 }
 } // namespace