From b9421952a92ccbf850f332110cfeadfdfce49252 Mon Sep 17 00:00:00 2001
From: Thomas Preud'homme <thomasp@graphcore.ai>
Date: Fri, 5 Jul 2019 12:01:06 +0000
Subject: [PATCH] [FileCheck] Factor some parsing checks out

Summary:
Both callers of parseNumericVariableDefinition() perform the same extra
check that no character is found after the variable name. This patch
factors out this check into parseNumericVariableDefinition().

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

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

Tags: #llvm

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@365191 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Support/FileCheck.cpp                     | 49 ++++++-------------
 .../FileCheck/numeric-defines-diagnostics.txt |  4 +-
 2 files changed, 16 insertions(+), 37 deletions(-)

diff --git a/lib/Support/FileCheck.cpp b/lib/Support/FileCheck.cpp
index 1fb410ac6c3..2e7fad600c1 100644
--- a/lib/Support/FileCheck.cpp
+++ b/lib/Support/FileCheck.cpp
@@ -130,6 +130,11 @@ Error FileCheckPattern::parseNumericVariableDefinition(
     return FileCheckErrorDiagnostic::get(
         SM, Name, "string variable with name '" + Name + "' already exists");
 
+  Expr = Expr.ltrim(SpaceChars);
+  if (!Expr.empty())
+    return FileCheckErrorDiagnostic::get(
+        SM, Expr, "unexpected characters after numeric variable name");
+
   return Error::success();
 }
 
@@ -229,27 +234,21 @@ Expected<FileCheckExpression *> FileCheckPattern::parseNumericSubstitutionBlock(
   size_t DefEnd = Expr.find(':');
   if (DefEnd != StringRef::npos) {
     StringRef DefExpr = Expr.substr(0, DefEnd);
-    StringRef UseExpr = Expr = Expr.substr(DefEnd + 1);
+    StringRef UseExpr = Expr.substr(DefEnd + 1);
 
-    DefExpr = DefExpr.ltrim(SpaceChars);
-    StringRef Name;
-    Error ErrorDiagnostic =
-        parseNumericVariableDefinition(DefExpr, Name, Context, SM);
-    if (ErrorDiagnostic)
-      return std::move(ErrorDiagnostic);
-
-    DefinedNumericVariable =
-        Context->makeNumericVariable(this->LineNumber, Name);
-
-    DefExpr = DefExpr.ltrim(SpaceChars);
-    if (!DefExpr.empty())
-      return FileCheckErrorDiagnostic::get(
-          SM, DefExpr, "invalid numeric variable definition");
     UseExpr = UseExpr.ltrim(SpaceChars);
     if (!UseExpr.empty())
       return FileCheckErrorDiagnostic::get(
           SM, UseExpr,
           "unexpected string after variable definition: '" + UseExpr + "'");
+
+    DefExpr = DefExpr.ltrim(SpaceChars);
+    StringRef Name;
+    Error Err = parseNumericVariableDefinition(DefExpr, Name, Context, SM);
+    if (Err)
+      return std::move(Err);
+    DefinedNumericVariable = Context->makeNumericVariable(LineNumber, Name);
+
     return Context->makeExpression(add, nullptr, 0);
   }
 
@@ -1735,32 +1734,12 @@ Error FileCheckPatternContext::defineCmdlineVariables(
     if (CmdlineDef[0] == '#') {
       StringRef CmdlineName = CmdlineDef.substr(1, EqIdx - 1);
       StringRef VarName;
-      SMLoc CmdlineNameLoc = SMLoc::getFromPointer(CmdlineName.data());
       Error ErrorDiagnostic = FileCheckPattern::parseNumericVariableDefinition(
           CmdlineName, VarName, this, SM);
       if (ErrorDiagnostic) {
         Errs = joinErrors(std::move(Errs), std::move(ErrorDiagnostic));
         continue;
       }
-      // Check that CmdlineName is only composed of the parsed numeric
-      // variable. This catches cases like "FOO+2" in a "FOO+2=10" definition.
-      if (!CmdlineName.empty()) {
-        Errs = joinErrors(std::move(Errs),
-                          FileCheckErrorDiagnostic::get(
-                              SM, CmdlineNameLoc, "invalid variable name"));
-        continue;
-      }
-
-      // Detect collisions between string and numeric variables when the latter
-      // is created later than the former.
-      if (DefinedVariableTable.find(VarName) != DefinedVariableTable.end()) {
-        Errs = joinErrors(
-            std::move(Errs),
-            FileCheckErrorDiagnostic::get(SM, VarName,
-                                          "string variable with name '" +
-                                              VarName + "' already exists"));
-        continue;
-      }
 
       StringRef CmdlineVal = CmdlineDef.substr(EqIdx + 1);
       uint64_t Val;
diff --git a/test/FileCheck/numeric-defines-diagnostics.txt b/test/FileCheck/numeric-defines-diagnostics.txt
index a5b3e2bc843..bee0b402699 100644
--- a/test/FileCheck/numeric-defines-diagnostics.txt
+++ b/test/FileCheck/numeric-defines-diagnostics.txt
@@ -20,9 +20,9 @@ NUMERRCLIPSEUDO-NEXT: {{^                   \^$}}
 RUN: not FileCheck -D#VALUE+2=10 --input-file %s %s 2>&1 \
 RUN:   | FileCheck %s --strict-whitespace --check-prefix NUMERRCLITRAIL
 
-NUMERRCLITRAIL: Global defines:1:20: error: invalid variable name
+NUMERRCLITRAIL: Global defines:1:25: error: unexpected characters after numeric variable name
 NUMERRCLITRAIL-NEXT: Global define #1: #VALUE+2=10
-NUMERRCLITRAIL-NEXT: {{^                   \^$}}
+NUMERRCLITRAIL-NEXT: {{^                        \^$}}
 
 ; Invalid value: numeric expression instead of literal.
 RUN: not FileCheck -D#VALUE1=3 -D#VALUE2='VALUE1 + 2' --input-file %s %s 2>&1 \
-- 
2.50.1