[FileCheck] Introduce substitution subclasses

[thopre]

Summary

With now a clear distinction between string and numeric substitutions,
this patch introduces separate classes to represent them with a parent
class implementing the common interface. Diagnostics in
printSubstitutions() are also adapted to not require knowing which
substitution is being looked at since it does not hinder clarity and
makes the implementation simpler.

[jhenderson]

LGTM, with two small comments. Might be best getting @probinson to give a final thumbs up though, since he suggested it.

llvm/include/llvm/Support/FileCheck.h

141–143 ↗

(On Diff #200691)

The explanations of the different substitutions performed (i.e. the second and third sentences of this comment) don't belong in this class here. I'd just remove them.

177–178 ↗

(On Diff #200691)

Maybe better to rephrase this as "\returns a string containing the result...

[thopre]

Address review comments

[probinson]

Apart from missing 'override' keywords it looks pretty straightforward. LGTM.

llvm/include/llvm/Support/FileCheck.h

157 ↗

(On Diff #200729)

This is virtual so should be marked 'override'.

161 ↗

(On Diff #200729)

'override'

177 ↗

(On Diff #200729)

override

181 ↗

(On Diff #200729)

override

[thopre]

Address review comments

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

protected:
   /// Pointer to a class instance holding, among other things, the table with
   /// the values of live string variables at the start of any given CHECK line.
   /// Used for substituting string variables with the text they were defined
   /// as. Numeric expressions are linked to the numeric variables they use at
   /// parse time and directly access the value of the numeric variable to
   /// evaluate their value.
   FileCheckPatternContext *Context;
 
-  /// Whether this represents a numeric expression substitution.
-  bool IsNumSubst;
-
   /// The string that needs to be substituted for something else. For a
   /// string variable this is its name, otherwise this is the whole numeric
   /// expression.
   StringRef FromStr;
 
-  /// If this is a numeric expression substitution, this is the pointer to the
-  /// class representing the numeric expression whose value is to be
-  /// substituted.
-  FileCheckNumExpr *NumExpr = nullptr;
-
   // Index in RegExStr of where to do the substitution.
   size_t InsertIdx;
 
 public:
-  /// Constructor for a pattern variable substitution.
-  FileCheckSubstitution(FileCheckPatternContext *Context,
-                        StringRef VarName, size_t InsertIdx)
-      : Context(Context), IsNumSubst(false), FromStr(VarName),
-        InsertIdx(InsertIdx) {}
+  FileCheckSubstitution(FileCheckPatternContext *Context, StringRef VarName,
+                        size_t InsertIdx)
+      : Context(Context), FromStr(VarName), InsertIdx(InsertIdx) {}
 
-  /// Constructor for a numeric expression substitution.
-  FileCheckSubstitution(FileCheckPatternContext *Context, StringRef Expr,
-                        FileCheckNumExpr *NumExpr, size_t InsertIdx)
-      : Context(Context), IsNumSubst(true), FromStr(Expr), NumExpr(NumExpr),
-        InsertIdx(InsertIdx) {}
-
-  /// \returns whether this is a numeric expression substitution.
-  bool isNumSubst() const { return IsNumSubst; }
+  virtual ~FileCheckSubstitution() = default;
 
   /// \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 the result of the substitution represented by this class
-  /// instance or None if substitution failed. Numeric expressions are
-  /// substituted by their values. String variables are simply replaced by the
-  /// text their definition matched.
-  llvm::Optional<std::string> getResult() const;
+  /// \returns a string containing the result of the substitution represented
+  /// by this class instance or None if substitution failed.
+  virtual llvm::Optional<std::string> getResult() const = 0;
 
-  /// \returns the name of the undefined variable used in this substitution, if
-  /// any, or an empty string otherwise.
-  StringRef getUndefVarName() const;
+  /// \returns the name of the variable used in this substitution if undefined,
+  /// or an empty string otherwise.
+  virtual StringRef getUndefVarName() 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.
+  llvm::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;
+};
+
+class FileCheckNumericSubstitution : public FileCheckSubstitution {
+private:
+  /// Pointer to the class representing the numeric expression whose value is
+  /// to be substituted.
+  FileCheckNumExpr *NumExpr;
+
+public:
+  FileCheckNumericSubstitution(FileCheckPatternContext *Context, StringRef Expr,
+                               FileCheckNumExpr *NumExpr, size_t InsertIdx)
+      : FileCheckSubstitution(Context, Expr, InsertIdx), NumExpr(NumExpr) {}
+
+  /// \returns a string containing the result of evaluating the numeric
+  /// expression in this substitution, or None if evaluation failed.
+  llvm::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;
 };
 
 //===----------------------------------------------------------------------===//
 // Pattern handling code.
 //===----------------------------------------------------------------------===//
 
 namespace Check {
 
private:
   /// 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;
 
+  /// 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.
   llvm::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
private:
   /// when the context is destroyed.
   FileCheckNumExpr *makeNumExpr(binop_eval_t EvalBinop,
                                 FileCheckNumericVariable *OperandLeft,
                                 uint64_t OperandRight);
 
   /// Makes a new numeric variable and registers it for destruction when the
   /// context is destroyed.
   FileCheckNumericVariable *makeNumericVariable(StringRef Name, uint64_t Value);
+
+  /// 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);
 };
 
 class FileCheckPattern {
   SMLoc PatternLoc;
 
   /// A fixed string to match as the pattern or empty if this pattern requires
   /// a regex match.
   StringRef FixedStr;
 
   /// A regex string to match as the pattern or empty if this pattern requires
   /// a fixed string to match.
   std::string RegExStr;
 
   /// Entries in this vector represent a substitution of a string variable or a
   /// numeric expression in the RegExStr regex at match time. For example, in
   /// the case of a CHECK directive with the pattern "foo[[bar]]baz[[#N+1]]",
   /// RegExStr will contain "foobaz" and we'll get two entries in this vector
   /// that tells us to insert the value of string variable "bar" at offset 3
   /// and the value of numeric expression "N+1" at offset 6.
-  std::vector<FileCheckSubstitution> Substitutions;
+  std::vector<FileCheckSubstitution *> Substitutions;
 
   /// Maps names of string variables defined in a pattern to the parenthesized
   /// capture numbers of their last definition.
   ///
   /// E.g. for the pattern "foo[[bar:.*]]baz[[bar]]quux[[bar:.*]]",
   /// VariableDefs will map "bar" to 2 corresponding to the second definition
   /// of "bar".
   ///

llvm/trunk/lib/Support/FileCheck.cpp

 }
 
 StringRef FileCheckNumExpr::getUndefVarName() const {
   if (!LeftOp->getValue())
     return LeftOp->getName();
   return StringRef();
 }
 
-llvm::Optional<std::string> FileCheckSubstitution::getResult() const {
-  if (IsNumSubst) {
-    llvm::Optional<uint64_t> EvaluatedValue = NumExpr->eval();
-    if (!EvaluatedValue)
-      return llvm::None;
-    return utostr(*EvaluatedValue);
-  }
+llvm::Optional<std::string> FileCheckNumericSubstitution::getResult() const {
 
+llvm::Optional<std::string> FileCheckStringSubstitution::getResult() const {
   // Look up the value and escape it so that we can put it into the regex.
   llvm::Optional<StringRef> VarVal = Context->getPatternVarValue(FromStr);
   if (!VarVal)
     return llvm::None;
   return Regex::escape(*VarVal);
 }
 
-StringRef FileCheckSubstitution::getUndefVarName() const {
-  if (IsNumSubst)
-    // 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();
+StringRef FileCheckNumericSubstitution::getUndefVarName() const {
+}
 
+StringRef FileCheckStringSubstitution::getUndefVarName() const {
   if (!Context->getPatternVarValue(FromStr))
     return FromStr;
 
   return StringRef();
 }
 
 bool FileCheckPattern::isValidVarNameStart(char C) {
   return C == '_' || isalpha(C);
if (PatternStr.startswith("[[")) {
                             SourceMgr::DK_Error,
                             "Can't back-reference more than 9 variables");
             return true;
           }
           AddBackrefToRegEx(CaptureParen);
         } else {
           // Handle substitution of string variables ([[<var>]]) defined in
           // previous CHECK patterns, and substitution of numeric expressions.
-          FileCheckSubstitution Substitution =
-              IsNumBlock ? FileCheckSubstitution(Context, MatchStr, NumExpr,
+          FileCheckSubstitution *Substitution =
+              IsNumBlock
+                  ? Context->makeNumericSubstitution(MatchStr, NumExpr,
                                                      SubstInsertIdx)
-                         : FileCheckSubstitution(Context, MatchStr,
-                                                 SubstInsertIdx);
+                  : Context->makeStringSubstitution(MatchStr, SubstInsertIdx);
           Substitutions.push_back(Substitution);
         }
         continue;
       }
 
       // Handle variable definitions: [[foo:.*]].
       VariableDefs[Name] = CurParen;
       RegExStr += '(';
if (!Substitutions.empty()) {
     TmpStr = RegExStr;
 
     size_t InsertOffset = 0;
     // Substitute all string variables and numeric expressions whose values are
     // only now known. Use of string variables defined on the same line are
     // handled by back-references.
     for (const auto &Substitution : Substitutions) {
       // Substitute and check for failure (e.g. use of undefined variable).
-      llvm::Optional<std::string> Value = Substitution.getResult();
+      llvm::Optional<std::string> Value = Substitution->getResult();
       if (!Value)
         return StringRef::npos;
 
       // Plop it into the regex at the adjusted offset.
-      TmpStr.insert(TmpStr.begin() + Substitution.getIndex() + InsertOffset,
+      TmpStr.insert(TmpStr.begin() + Substitution->getIndex() + InsertOffset,
                     Value->begin(), Value->end());
       InsertOffset += Value->size();
     }
 
     // Match the newly constructed regex.
     RegExToMatch = TmpStr;
   }
 

 
 void FileCheckPattern::printSubstitutions(const SourceMgr &SM, StringRef Buffer,
                                           SMRange MatchRange) const {
   // Print what we know about substitutions.
   if (!Substitutions.empty()) {
     for (const auto &Substitution : Substitutions) {
       SmallString<256> Msg;
       raw_svector_ostream OS(Msg);
-      bool IsNumSubst = Substitution.isNumSubst();
-      llvm::Optional<std::string> MatchedValue = Substitution.getResult();
+      llvm::Optional<std::string> MatchedValue = Substitution->getResult();
 
       // Substitution failed or is not known at match time, print the undefined
       // variable it uses.
       if (!MatchedValue) {
-        StringRef UndefVarName = Substitution.getUndefVarName();
+        StringRef UndefVarName = Substitution->getUndefVarName();
         if (UndefVarName.empty())
           continue;
         OS << "uses undefined variable \"";
         OS.write_escaped(UndefVarName) << "\"";
       } else {
         // Substitution succeeded. Print substituted value.
-        if (IsNumSubst)
-          OS << "with numeric expression \"";
-        else
-          OS << "with string variable \"";
-        OS.write_escaped(Substitution.getFromString()) << "\" equal to \"";
+        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});
       else
         SM.PrintMessage(SMLoc::getFromPointer(Buffer.data()),

 
 FileCheckNumericVariable *
 FileCheckPatternContext::makeNumericVariable(StringRef Name, uint64_t Value) {
   NumericVariables.push_back(
       llvm::make_unique<FileCheckNumericVariable>(Name, Value));
   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) {
+  Substitutions.push_back(llvm::make_unique<FileCheckNumericSubstitution>(
+      this, Expr, NumExpr, 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;
 
   while (!Str.empty()) {
     if (Str.startswith("]]") && BracketDepth == 0)

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

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

llvm/trunk/test/FileCheck/string-defines.txt

 RUN: FileCheck -DVALUE=20 --check-prefix NOT --input-file %s %s
 
 Value = 10
 CHECK: Value = [[VALUE]]
 NOT-NOT: Value = [[VALUE]]
 
 ERRMSG: defines.txt:[[@LINE-3]]:8: error: CHECK: expected string not found in input
 ERRMSG: defines.txt:1:1: note: scanning from here
-ERRMSG: defines.txt:1:1: note: with string variable "VALUE" equal to "20"
+ERRMSG: defines.txt:1:1: note: with "VALUE" equal to "20"
 ERRMSG: defines.txt:[[@LINE-7]]:1: note: possible intended match here
 
 NOT-ERRMSG: defines.txt:[[@LINE-7]]:10: error: {{NOT}}-NOT: excluded string found in input
 NOT-ERRMSG: defines.txt:[[@LINE-10]]:1: note: found here
-NOT-ERRMSG: defines.txt:[[@LINE-11]]:1: note: with string variable "VALUE" equal to "10"
+NOT-ERRMSG: defines.txt:[[@LINE-11]]:1: note: with "VALUE" equal to "10"
 
 ; Definition of string variable to an empty string.
 RUN: FileCheck -DVALUE= --check-prefix EMPTY --input-file %s %s 2>&1
 
 Empty value = @@
 EMPTY: Empty value = @[[VALUE]]@

llvm/trunk/test/FileCheck/verbose.txt

 CHECK-NEXT: NUMVAR - 1:[[#NUMVAR - 1]]
 
 V:      verbose.txt:[[#@LINE-4]]:8: remark: {{C}}HECK: expected string found in input
 V-NEXT: {{C}}HECK: {{NUMVAR:[[][[]#NUMVAR[]][]]$}}
 V-NEXT: {{^       \^$}}
 V-NEXT: verbose.txt:[[#@LINE-9]]:1: note: found here
 V-NEXT: {{^}}NUMVAR:42{{$}}
 V-NEXT: {{^}}^~~~~~~~~{{$}}
-V-NEXT: verbose.txt:[[#@LINE-12]]:1: note: with numeric expression "NUMVAR" equal to "42"
+V-NEXT: verbose.txt:[[#@LINE-12]]:1: note: with "NUMVAR" equal to "42"
 V-NEXT: {{^}}NUMVAR:42{{$}}
 V-NEXT: {{^}}^~~~~~~~~{{$}}
 
 V-NEXT: verbose.txt:[[#@LINE-12]]:13: remark: {{C}}HECK-NEXT: expected string found in input
 V-NEXT: {{C}}HECK-NEXT: {{NUMVAR - 1:[[][[]#NUMVAR - 1[]][]]$}}
 V-NEXT: {{^            \^$}}
 V-NEXT: verbose.txt:[[#@LINE-18]]:1: note: found here
 V-NEXT: {{^}}NUMVAR - 1:41{{$}}
 V-NEXT: {{^}}^~~~~~~~~~~~~{{$}}
-V-NEXT: verbose.txt:[[#@LINE-21]]:1: note: with numeric expression "NUMVAR - 1" equal to "41"
+V-NEXT: verbose.txt:[[#@LINE-21]]:1: note: with "NUMVAR - 1" equal to "41"
 V-NEXT: {{^}}NUMVAR - 1:41{{$}}
 V-NEXT: {{^}}^~~~~~~~~~~~~{{$}}
 
 VV-NEXT: verbose.txt:[[#@LINE-23]]:12: remark: {{C}}HECK-NOT: excluded string not found in input
 VV-NEXT: {{C}}HECK-NOT: {{[[][[]#NUMVAR [+] 1[]][]]$}}
 VV-NEXT: {{^           \^$}}
 VV-NEXT: verbose.txt:[[#@LINE-28]]:1: note: scanning from here
 VV-NEXT: {{^}}NUMVAR - 1:41{{$}}

llvm/trunk/unittests/Support/FileCheckTest.cpp

 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.
-  FileCheckSubstitution Substitution =
-      FileCheckSubstitution(&Context, "VAR404", 42);
-  EXPECT_FALSE(Substitution.getResult());
+  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);
-  FileCheckSubstitution SubstitutionLine =
-      FileCheckSubstitution(&Context, "@LINE", &NumExprLine, 12);
-  FileCheckSubstitution SubstitutionN =
-      FileCheckSubstitution(&Context, "N", &NumExprN, 30);
+  FileCheckNumericSubstitution SubstitutionLine =
+      FileCheckNumericSubstitution(&Context, "@LINE", &NumExprLine, 12);
+  FileCheckNumericSubstitution SubstitutionN =
+      FileCheckNumericSubstitution(&Context, "N", &NumExprN, 30);
   llvm::Optional<std::string> Value = SubstitutionLine.getResult();
   EXPECT_TRUE(Value);
   EXPECT_EQ("42", *Value);
   Value = SubstitutionN.getResult();
   EXPECT_TRUE(Value);
   EXPECT_EQ("13", *Value);
 
   // Substitution of an undefined numeric variable fails.
   LineVar.clearValue();
   EXPECT_FALSE(SubstitutionLine.getResult());
   NVar.clearValue();
   EXPECT_FALSE(SubstitutionN.getResult());
 
   // Substitution of a defined string variable returns the right value.
   FileCheckPattern P = FileCheckPattern(Check::CheckPlain, &Context);
-  Substitution = FileCheckSubstitution(&Context, "FOO", 42);
-  Value = Substitution.getResult();
+  StringSubstitution = FileCheckStringSubstitution(&Context, "FOO", 42);
+  Value = StringSubstitution.getResult();
   EXPECT_TRUE(Value);
   EXPECT_EQ("BAR", *Value);
 }
 
 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.
-  FileCheckSubstitution Substitution =
-      FileCheckSubstitution(&Context, "VAR404", 42);
-  StringRef UndefVar = Substitution.getUndefVarName();
+  FileCheckStringSubstitution StringSubstitution =
+      FileCheckStringSubstitution(&Context, "VAR404", 42);
+  StringRef UndefVar = StringSubstitution.getUndefVarName();
   EXPECT_EQ("VAR404", UndefVar);
 
   // getUndefVarName() on a string substitution with a defined variable returns
   // an empty string.
-  Substitution = FileCheckSubstitution(&Context, "FOO", 42);
-  UndefVar = Substitution.getUndefVarName();
+  StringSubstitution = FileCheckStringSubstitution(&Context, "FOO", 42);
+  UndefVar = StringSubstitution.getUndefVarName();
   EXPECT_EQ("", UndefVar);
 
   // getUndefVarName() on a numeric substitution with a defined variable
   // returns an empty string.
   FileCheckNumericVariable LineVar = FileCheckNumericVariable("@LINE", 42);
   FileCheckNumExpr NumExpr = FileCheckNumExpr(doAdd, &LineVar, 0);
-  Substitution = FileCheckSubstitution(&Context, "@LINE", &NumExpr, 12);
-  UndefVar = Substitution.getUndefVarName();
+  FileCheckNumericSubstitution NumericSubstitution =
+      FileCheckNumericSubstitution(&Context, "@LINE", &NumExpr, 12);
+  UndefVar = NumericSubstitution.getUndefVarName();
   EXPECT_EQ("", UndefVar);
 
   // getUndefVarName() on a numeric substitution with an undefined variable
   // returns that variable.
   LineVar.clearValue();
-  UndefVar = Substitution.getUndefVarName();
+  UndefVar = NumericSubstitution.getUndefVarName();
   EXPECT_EQ("@LINE", UndefVar);
 }
 
 TEST_F(FileCheckTest, FileCheckContext) {
   FileCheckPatternContext Cxt = FileCheckPatternContext();
   std::vector<std::string> GlobalDefines;
   SourceMgr SM;