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

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

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

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

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

Copyright:
    - Linaro (changes up to diff 183612 of revision D55940)
    - GraphCore (changes in later versions of revision D55940 and
                 in new revision created off D55940)

Reviewers: jhenderson, chandlerc, jdenny, probinson, grimar, arichardson, rnk

Subscribers: hiraditya, llvm-commits, probinson, dblaikie, grimar, arichardson, tra, rnk, kristina, hfinkel, rogfer01, JonChesterfield

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D60388

> llvm-svn: 366860

llvm-svn: 366897

1 files changed, 191 insertions, 118 deletions

diff --git a/llvm/lib/Support/FileCheck.cpp b/llvm/lib/Support/FileCheck.cpp
index 44e394f..db02315 100644
--- a/llvm/lib/Support/FileCheck.cpp
+++ b/llvm/lib/Support/FileCheck.cpp
@@ -24,11 +24,38 @@
 
 using namespace llvm;
 
+bool FileCheckNumericVariable::isValueKnownAtMatchTime() const {
+  if (Value)
+    return true;
+
+  return ExpressionAST != nullptr;
+}
+
+void FileCheckNumericVariable::setValue(uint64_t NewValue) {
+  if (ExpressionAST != nullptr) {
+    // Caller is expected to call setValue only if substitution was successful.
+    assert(NewValue == cantFail(ExpressionAST->eval(),
+                                "Failed to evaluate associated expression when "
+                                "sanity checking value") &&
+           "Value being set to different from variable evaluation");
+  }
+  Value = NewValue;
+  // Clear pointer to AST to ensure it is not used after the numeric
+  // substitution defining this variable is processed since it's the
+  // substitution that owns the pointer.
+  ExpressionAST = nullptr;
+}
+
 Expected<uint64_t> FileCheckNumericVariableUse::eval() const {
   Optional<uint64_t> Value = NumericVariable->getValue();
   if (Value)
     return *Value;
-  return make_error<FileCheckUndefVarError>(Name);
+
+  FileCheckExpressionAST *ExpressionAST = NumericVariable->getExpressionAST();
+  if (!ExpressionAST)
+    return make_error<FileCheckUndefVarError>(Name);
+
+  return ExpressionAST->eval();
 }
 
 Expected<uint64_t> FileCheckASTBinop::eval() const {
@@ -114,7 +141,8 @@ char FileCheckNotFoundError::ID = 0;
 Expected<FileCheckNumericVariable *>
 FileCheckPattern::parseNumericVariableDefinition(
     StringRef &Expr, FileCheckPatternContext *Context,
-    Optional<size_t> LineNumber, const SourceMgr &SM) {
+    Optional<size_t> LineNumber, FileCheckExpressionAST *ExpressionAST,
+    const SourceMgr &SM) {
   Expected<VariableProperties> ParseVarResult = parseVariable(Expr, SM);
   if (!ParseVarResult)
     return ParseVarResult.takeError();
@@ -141,14 +169,17 @@ FileCheckPattern::parseNumericVariableDefinition(
   if (VarTableIter != Context->GlobalNumericVariableTable.end())
     DefinedNumericVariable = VarTableIter->second;
   else
-    DefinedNumericVariable = Context->makeNumericVariable(Name, LineNumber);
+    DefinedNumericVariable =
+        Context->makeNumericVariable(Name, LineNumber, ExpressionAST);
 
   return DefinedNumericVariable;
 }
 
 Expected<std::unique_ptr<FileCheckNumericVariableUse>>
 FileCheckPattern::parseNumericVariableUse(StringRef Name, bool IsPseudo,
-                                          const SourceMgr &SM) const {
+                                          Optional<size_t> LineNumber,
+                                          FileCheckPatternContext *Context,
+                                          const SourceMgr &SM) {
   if (IsPseudo && !Name.equals("@LINE"))
     return FileCheckErrorDiagnostic::get(
         SM, Name, "invalid pseudo numeric variable '" + Name + "'");
@@ -171,24 +202,29 @@ FileCheckPattern::parseNumericVariableUse(StringRef Name, bool IsPseudo,
   }
 
   Optional<size_t> DefLineNumber = NumericVariable->getDefLineNumber();
-  if (DefLineNumber && LineNumber && *DefLineNumber == *LineNumber)
+  if (DefLineNumber && LineNumber && *DefLineNumber == *LineNumber &&
+      !NumericVariable->isValueKnownAtMatchTime())
     return FileCheckErrorDiagnostic::get(
         SM, Name,
-        "numeric variable '" + Name + "' defined on the same line as used");
+        "numeric variable '" + Name +
+            "' defined from input on the same line as used");
 
   return llvm::make_unique<FileCheckNumericVariableUse>(Name, NumericVariable);
 }
 
 Expected<std::unique_ptr<FileCheckExpressionAST>>
 FileCheckPattern::parseNumericOperand(StringRef &Expr, AllowedOperand AO,
-                                      const SourceMgr &SM) const {
+                                      Optional<size_t> LineNumber,
+                                      FileCheckPatternContext *Context,
+                                      const SourceMgr &SM) {
   if (AO == AllowedOperand::LineVar || AO == AllowedOperand::Any) {
     // Try to parse as a numeric variable use.
     Expected<FileCheckPattern::VariableProperties> ParseVarResult =
         parseVariable(Expr, SM);
     if (ParseVarResult)
       return parseNumericVariableUse(ParseVarResult->Name,
-                                     ParseVarResult->IsPseudo, SM);
+                                     ParseVarResult->IsPseudo, LineNumber,
+                                     Context, SM);
     if (AO == AllowedOperand::LineVar)
       return ParseVarResult.takeError();
     // Ignore the error and retry parsing as a literal.
@@ -212,10 +248,10 @@ static uint64_t sub(uint64_t LeftOp, uint64_t RightOp) {
   return LeftOp - RightOp;
 }
 
-Expected<std::unique_ptr<FileCheckExpressionAST>>
-FileCheckPattern::parseBinop(StringRef &Expr,
-                             std::unique_ptr<FileCheckExpressionAST> LeftOp,
-                             bool IsLegacyLineExpr, const SourceMgr &SM) const {
+Expected<std::unique_ptr<FileCheckExpressionAST>> FileCheckPattern::parseBinop(
+    StringRef &Expr, std::unique_ptr<FileCheckExpressionAST> LeftOp,
+    bool IsLegacyLineExpr, Optional<size_t> LineNumber,
+    FileCheckPatternContext *Context, const SourceMgr &SM) {
   Expr = Expr.ltrim(SpaceChars);
   if (Expr.empty())
     return std::move(LeftOp);
@@ -246,7 +282,7 @@ FileCheckPattern::parseBinop(StringRef &Expr,
   AllowedOperand AO =
       IsLegacyLineExpr ? AllowedOperand::Literal : AllowedOperand::Any;
   Expected<std::unique_ptr<FileCheckExpressionAST>> RightOpResult =
-      parseNumericOperand(Expr, AO, SM);
+      parseNumericOperand(Expr, AO, LineNumber, Context, SM);
   if (!RightOpResult)
     return RightOpResult;
 
@@ -259,50 +295,54 @@ Expected<std::unique_ptr<FileCheckExpressionAST>>
 FileCheckPattern::parseNumericSubstitutionBlock(
     StringRef Expr,
     Optional<FileCheckNumericVariable *> &DefinedNumericVariable,
-    bool IsLegacyLineExpr, const SourceMgr &SM) const {
-  // Parse the numeric variable definition.
+    bool IsLegacyLineExpr, Optional<size_t> LineNumber,
+    FileCheckPatternContext *Context, const SourceMgr &SM) {
+  std::unique_ptr<FileCheckExpressionAST> ExpressionAST = nullptr;
+  StringRef DefExpr = StringRef();
   DefinedNumericVariable = None;
+  // Save variable definition expression if any.
   size_t DefEnd = Expr.find(':');
   if (DefEnd != StringRef::npos) {
-    StringRef DefExpr = Expr.substr(0, DefEnd);
-    StringRef UseExpr = Expr.substr(DefEnd + 1);
+    DefExpr = Expr.substr(0, DefEnd);
+    Expr = Expr.substr(DefEnd + 1);
+  }
 
-    UseExpr = UseExpr.ltrim(SpaceChars);
-    if (!UseExpr.empty())
-      return FileCheckErrorDiagnostic::get(
-          SM, UseExpr,
-          "unexpected string after variable definition: '" + UseExpr + "'");
+  // Parse the expression itself.
+  Expr = Expr.ltrim(SpaceChars);
+  if (!Expr.empty()) {
+    // The first operand in a legacy @LINE expression is always the @LINE
+    // pseudo variable.
+    AllowedOperand AO =
+        IsLegacyLineExpr ? AllowedOperand::LineVar : AllowedOperand::Any;
+    Expected<std::unique_ptr<FileCheckExpressionAST>> ParseResult =
+        parseNumericOperand(Expr, AO, LineNumber, Context, SM);
+    while (ParseResult && !Expr.empty()) {
+      ParseResult = parseBinop(Expr, std::move(*ParseResult), IsLegacyLineExpr,
+                               LineNumber, Context, SM);
+      // Legacy @LINE expressions only allow 2 operands.
+      if (ParseResult && IsLegacyLineExpr && !Expr.empty())
+        return FileCheckErrorDiagnostic::get(
+            SM, Expr,
+            "unexpected characters at end of expression '" + Expr + "'");
+    }
+    if (!ParseResult)
+      return ParseResult;
+    ExpressionAST = std::move(*ParseResult);
+  }
 
+  // Parse the numeric variable definition.
+  if (DefEnd != StringRef::npos) {
     DefExpr = DefExpr.ltrim(SpaceChars);
     Expected<FileCheckNumericVariable *> ParseResult =
-        parseNumericVariableDefinition(DefExpr, Context, LineNumber, SM);
+        parseNumericVariableDefinition(DefExpr, Context, LineNumber,
+                                       ExpressionAST.get(), SM);
+
     if (!ParseResult)
       return ParseResult.takeError();
     DefinedNumericVariable = *ParseResult;
-
-    return nullptr;
   }
 
-  // Parse the expression itself.
-  Expr = Expr.ltrim(SpaceChars);
-  // The first operand in a legacy @LINE expression is always the @LINE pseudo
-  // variable.
-  AllowedOperand AO =
-      IsLegacyLineExpr ? AllowedOperand::LineVar : AllowedOperand::Any;
-  Expected<std::unique_ptr<FileCheckExpressionAST>> ParseResult =
-      parseNumericOperand(Expr, AO, SM);
-  while (ParseResult && !Expr.empty()) {
-    ParseResult =
-        parseBinop(Expr, std::move(*ParseResult), IsLegacyLineExpr, SM);
-    // Legacy @LINE expressions only allow 2 operands.
-    if (ParseResult && IsLegacyLineExpr && !Expr.empty())
-      return FileCheckErrorDiagnostic::get(
-          SM, Expr,
-          "unexpected characters at end of expression '" + Expr + "'");
-  }
-  if (!ParseResult)
-    return ParseResult;
-  return std::move(*ParseResult);
+  return std::move(ExpressionAST);
 }
 
 bool FileCheckPattern::parsePattern(StringRef PatternStr, StringRef Prefix,
@@ -385,14 +425,15 @@ bool FileCheckPattern::parsePattern(StringRef PatternStr, StringRef Prefix,
       continue;
     }
 
-    // String and numeric substitution blocks. String substitution blocks come
+    // String and numeric substitution blocks. Pattern substitution blocks come
     // in two forms: [[foo:.*]] and [[foo]]. The former matches .* (or some
     // other regex) and assigns it to the string variable 'foo'. The latter
-    // substitutes foo's value. Numeric substitution blocks work the same way
-    // as string ones, but start with a '#' sign after the double brackets.
-    // Both string and numeric variable names must satisfy the regular
-    // expression "[a-zA-Z_][0-9a-zA-Z_]*" to be valid, as this helps catch
-    // some common errors.
+    // substitutes foo's value. Numeric substitution blocks recognize the same
+    // form as string ones, but start with a '#' sign after the double
+    // brackets. They also accept a combined form which sets a numeric variable
+    // to the evaluation of an expression. Both string and numeric variable
+    // names must satisfy the regular expression "[a-zA-Z_][0-9a-zA-Z_]*" to be
+    // valid, as this helps catch some common errors.
     if (PatternStr.startswith("[[")) {
       StringRef UnparsedPatternStr = PatternStr.substr(2);
       // Find the closing bracket pair ending the match.  End is going to be an
@@ -413,6 +454,7 @@ bool FileCheckPattern::parsePattern(StringRef PatternStr, StringRef Prefix,
       PatternStr = UnparsedPatternStr.substr(End + 2);
 
       bool IsDefinition = false;
+      bool SubstNeeded = false;
       // Whether the substitution block is a legacy use of @LINE with string
       // substitution block syntax.
       bool IsLegacyLineExpr = false;
@@ -443,6 +485,7 @@ bool FileCheckPattern::parsePattern(StringRef PatternStr, StringRef Prefix,
         bool IsPseudo = ParseVarResult->IsPseudo;
 
         IsDefinition = (VarEndIdx != StringRef::npos);
+        SubstNeeded = !IsDefinition;
         if (IsDefinition) {
           if ((IsPseudo || !MatchStr.consume_front(":"))) {
             SM.PrintMessage(SMLoc::getFromPointer(Name.data()),
@@ -477,22 +520,61 @@ bool FileCheckPattern::parsePattern(StringRef PatternStr, StringRef Prefix,
       if (IsNumBlock) {
         Expected<std::unique_ptr<FileCheckExpressionAST>> ParseResult =
             parseNumericSubstitutionBlock(MatchStr, DefinedNumericVariable,
-                                          IsLegacyLineExpr, SM);
+                                          IsLegacyLineExpr, LineNumber, Context,
+                                          SM);
         if (!ParseResult) {
           logAllUnhandledErrors(ParseResult.takeError(), errs());
           return true;
         }
         ExpressionAST = std::move(*ParseResult);
+        SubstNeeded = ExpressionAST != nullptr;
         if (DefinedNumericVariable) {
           IsDefinition = true;
           DefName = (*DefinedNumericVariable)->getName();
-          MatchRegexp = StringRef("[0-9]+");
-        } else
+        }
+        if (SubstNeeded)
           SubstStr = MatchStr;
+        else
+          MatchRegexp = "[0-9]+";
       }
 
+      // Handle variable definition: [[<def>:(...)]] and [[#(...)<def>:(...)]].
+      if (IsDefinition) {
+        RegExStr += '(';
+        ++SubstInsertIdx;
+
+        if (IsNumBlock) {
+          FileCheckNumericVariableMatch NumericVariableDefinition = {
+              *DefinedNumericVariable, CurParen};
+          NumericVariableDefs[DefName] = NumericVariableDefinition;
+          // 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;
+        } else {
+          VariableDefs[DefName] = CurParen;
+          // Mark 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().
+          Context->DefinedVariableTable[DefName] = true;
+        }
+
+        ++CurParen;
+      }
+
+      if (!MatchRegexp.empty() && AddRegExToRegEx(MatchRegexp, CurParen, SM))
+        return true;
+
+      if (IsDefinition)
+        RegExStr += ')';
+
       // Handle substitutions: [[foo]] and [[#<foo expr>]].
-      if (!IsDefinition) {
+      if (SubstNeeded) {
         // Handle substitution of string variables that were defined earlier on
         // the same line by emitting a backreference. Expressions do not
         // support substituting a numeric variable defined on the same line.
@@ -515,37 +597,7 @@ bool FileCheckPattern::parsePattern(StringRef PatternStr, StringRef Prefix,
                   : Context->makeStringSubstitution(SubstStr, SubstInsertIdx);
           Substitutions.push_back(Substitution);
         }
-        continue;
-      }
-
-      // Handle variable definitions: [[<def>:(...)]] and
-      // [[#(...)<def>:(...)]].
-      if (IsNumBlock) {
-        FileCheckNumericVariableMatch NumericVariableDefinition = {
-            *DefinedNumericVariable, CurParen};
-        NumericVariableDefs[DefName] = NumericVariableDefinition;
-        // 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;
-      } 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().
-        Context->DefinedVariableTable[DefName] = true;
       }
-      RegExStr += '(';
-      ++CurParen;
-
-      if (AddRegExToRegEx(MatchRegexp, CurParen, SM))
-        return true;
-
-      RegExStr += ')';
     }
 
     // Handle fixed string matches.
@@ -1745,11 +1797,32 @@ Error FileCheckPatternContext::defineCmdlineVariables(
   unsigned I = 0;
   Error Errs = Error::success();
   std::string CmdlineDefsDiag;
-  StringRef Prefix1 = "Global define #";
-  StringRef Prefix2 = ": ";
-  for (StringRef CmdlineDef : CmdlineDefines)
-    CmdlineDefsDiag +=
-        (Prefix1 + Twine(++I) + Prefix2 + CmdlineDef + "\n").str();
+  SmallVector<std::pair<size_t, size_t>, 4> CmdlineDefsIndices;
+  for (StringRef CmdlineDef : CmdlineDefines) {
+    std::string DefPrefix = ("Global define #" + Twine(++I) + ": ").str();
+    size_t EqIdx = CmdlineDef.find('=');
+    if (EqIdx == StringRef::npos) {
+      CmdlineDefsIndices.push_back(std::make_pair(CmdlineDefsDiag.size(), 0));
+      continue;
+    }
+    // Numeric variable definition.
+    if (CmdlineDef[0] == '#') {
+      // Append a copy of the command-line definition adapted to use the same
+      // format as in the input file to be able to reuse
+      // parseNumericSubstitutionBlock.
+      CmdlineDefsDiag += (DefPrefix + CmdlineDef + " (parsed as: [[").str();
+      std::string SubstitutionStr = CmdlineDef;
+      SubstitutionStr[EqIdx] = ':';
+      CmdlineDefsIndices.push_back(
+          std::make_pair(CmdlineDefsDiag.size(), SubstitutionStr.size()));
+      CmdlineDefsDiag += (SubstitutionStr + Twine("]])\n")).str();
+    } else {
+      CmdlineDefsDiag += DefPrefix;
+      CmdlineDefsIndices.push_back(
+          std::make_pair(CmdlineDefsDiag.size(), CmdlineDef.size()));
+      CmdlineDefsDiag += (CmdlineDef + "\n").str();
+    }
+  }
 
   // Create a buffer with fake command line content in order to display
   // parsing diagnostic with location information and point to the
@@ -1759,14 +1832,10 @@ Error FileCheckPatternContext::defineCmdlineVariables(
   StringRef CmdlineDefsDiagRef = CmdLineDefsDiagBuffer->getBuffer();
   SM.AddNewSourceBuffer(std::move(CmdLineDefsDiagBuffer), SMLoc());
 
-  SmallVector<StringRef, 4> CmdlineDefsDiagVec;
-  CmdlineDefsDiagRef.split(CmdlineDefsDiagVec, '\n', -1 /*MaxSplit*/,
-                           false /*KeepEmpty*/);
-  for (StringRef CmdlineDefDiag : CmdlineDefsDiagVec) {
-    unsigned DefStart = CmdlineDefDiag.find(Prefix2) + Prefix2.size();
-    StringRef CmdlineDef = CmdlineDefDiag.substr(DefStart);
-    size_t EqIdx = CmdlineDef.find('=');
-    if (EqIdx == StringRef::npos) {
+  for (std::pair<size_t, size_t> CmdlineDefIndices : CmdlineDefsIndices) {
+    StringRef CmdlineDef = CmdlineDefsDiagRef.substr(CmdlineDefIndices.first,
+                                                     CmdlineDefIndices.second);
+    if (CmdlineDef.empty()) {
       Errs = joinErrors(
           std::move(Errs),
           FileCheckErrorDiagnostic::get(
@@ -1776,31 +1845,35 @@ Error FileCheckPatternContext::defineCmdlineVariables(
 
     // Numeric variable definition.
     if (CmdlineDef[0] == '#') {
-      StringRef CmdlineName = CmdlineDef.substr(1, EqIdx - 1);
-      Expected<FileCheckNumericVariable *> ParseResult =
-          FileCheckPattern::parseNumericVariableDefinition(CmdlineName, this,
-                                                           None, SM);
-      if (!ParseResult) {
-        Errs = joinErrors(std::move(Errs), ParseResult.takeError());
+      // Now parse the definition both to check that the syntax is correct and
+      // to create the necessary class instance.
+      StringRef CmdlineDefExpr = CmdlineDef.substr(1);
+      Optional<FileCheckNumericVariable *> DefinedNumericVariable;
+      Expected<std::unique_ptr<FileCheckExpressionAST>> ExpressionASTResult =
+          FileCheckPattern::parseNumericSubstitutionBlock(
+              CmdlineDefExpr, DefinedNumericVariable, false, None, this, SM);
+      if (!ExpressionASTResult) {
+        Errs = joinErrors(std::move(Errs), ExpressionASTResult.takeError());
         continue;
       }
-
-      StringRef CmdlineVal = CmdlineDef.substr(EqIdx + 1);
-      uint64_t Val;
-      if (CmdlineVal.getAsInteger(10, Val)) {
-        Errs = joinErrors(std::move(Errs),
-                          FileCheckErrorDiagnostic::get(
-                              SM, CmdlineVal,
-                              "invalid value in numeric variable definition '" +
-                                  CmdlineVal + "'"));
+      std::unique_ptr<FileCheckExpressionAST> ExpressionAST =
+          std::move(*ExpressionASTResult);
+      // Now evaluate the expression whose value this variable should be set
+      // to, since the expression of a command-line variable definition should
+      // only use variables defined earlier on the command-line. If not, this
+      // is an error and we report it.
+      Expected<uint64_t> Value = ExpressionAST->eval();
+      if (!Value) {
+        Errs = joinErrors(std::move(Errs), Value.takeError());
         continue;
       }
-      FileCheckNumericVariable *DefinedNumericVariable = *ParseResult;
-      DefinedNumericVariable->setValue(Val);
+
+      assert(DefinedNumericVariable && "No variable defined");
+      (*DefinedNumericVariable)->setValue(*Value);
 
       // Record this variable definition.
-      GlobalNumericVariableTable[DefinedNumericVariable->getName()] =
-          DefinedNumericVariable;
+      GlobalNumericVariableTable[(*DefinedNumericVariable)->getName()] =
+          *DefinedNumericVariable;
     } else {
       // String variable definition.
       std::pair<StringRef, StringRef> CmdlineNameVal = CmdlineDef.split('=');
@@ -1837,7 +1910,7 @@ Error FileCheckPatternContext::defineCmdlineVariables(
       }
       GlobalVariableTable.insert(CmdlineNameVal);
       // Mark the string variable as defined to detect collisions between
-      // string and numeric variables in DefineCmdlineVariables when the latter
+      // string and numeric variables in 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