FileCheck [2/12]: Stricter parsing of -D option

Summary:
This patch is part of a patch series to add support for FileCheck
numeric expressions. This specific patch gives earlier and better
diagnostics for the -D option.

Prior to this change, parsing of -D option was very loose: it assumed
that there is an equal sign (which to be fair is now checked by the
FileCheck executable) and that the part on the left of the equal sign
was a valid variable name. This commit adds logic to ensure that this
is the case and gives diagnostic when it is not, making it clear that
the issue came from a command-line option error. This is achieved by
sharing the variable parsing code into a new function ParseVariable.

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/D60382

llvm-svn: 359447

1 files changed, 105 insertions, 4 deletions

diff --git a/llvm/unittests/Support/FileCheckTest.cpp b/llvm/unittests/Support/FileCheckTest.cpp
index bf2339c9db6..497c8086431 100644
--- a/llvm/unittests/Support/FileCheckTest.cpp
+++ b/llvm/unittests/Support/FileCheckTest.cpp
@@ -14,31 +14,132 @@ namespace {
 
 class FileCheckTest : public ::testing::Test {};
 
+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('$'));
+  EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('@'));
+  EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('+'));
+  EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('-'));
+  EXPECT_FALSE(FileCheckPattern::isValidVarNameStart(':'));
+}
+
+TEST_F(FileCheckTest, ParseVar) {
+  StringRef VarName = "GoodVar42";
+  bool IsPseudo = true;
+  unsigned TrailIdx = 0;
+  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_FALSE(IsPseudo);
+  EXPECT_EQ(TrailIdx, VarName.size());
+
+  VarName = "$GoodGlobalVar";
+  IsPseudo = true;
+  TrailIdx = 0;
+  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_FALSE(IsPseudo);
+  EXPECT_EQ(TrailIdx, VarName.size());
+
+  VarName = "@GoodPseudoVar";
+  IsPseudo = true;
+  TrailIdx = 0;
+  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_TRUE(IsPseudo);
+  EXPECT_EQ(TrailIdx, VarName.size());
+
+  VarName = "42BadVar";
+  EXPECT_TRUE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+
+  VarName = "$@";
+  EXPECT_TRUE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+
+  VarName = "B@dVar";
+  IsPseudo = true;
+  TrailIdx = 0;
+  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_FALSE(IsPseudo);
+  EXPECT_EQ(TrailIdx, 1U);
+
+  VarName = "B$dVar";
+  IsPseudo = true;
+  TrailIdx = 0;
+  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_FALSE(IsPseudo);
+  EXPECT_EQ(TrailIdx, 1U);
+
+  VarName = "BadVar+";
+  IsPseudo = true;
+  TrailIdx = 0;
+  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_FALSE(IsPseudo);
+  EXPECT_EQ(TrailIdx, VarName.size() - 1);
+
+  VarName = "BadVar-";
+  IsPseudo = true;
+  TrailIdx = 0;
+  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_FALSE(IsPseudo);
+  EXPECT_EQ(TrailIdx, VarName.size() - 1);
+
+  VarName = "BadVar:";
+  IsPseudo = true;
+  TrailIdx = 0;
+  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx));
+  EXPECT_FALSE(IsPseudo);
+  EXPECT_EQ(TrailIdx, VarName.size() - 1);
+}
+
 TEST_F(FileCheckTest, FileCheckContext) {
-  FileCheckPatternContext Cxt;
+  FileCheckPatternContext Cxt = FileCheckPatternContext();
   std::vector<std::string> GlobalDefines;
+  SourceMgr SM;
+
+  // Missing equal sign
+  GlobalDefines.emplace_back(std::string("LocalVar"));
+  EXPECT_TRUE(Cxt.defineCmdlineVariables(GlobalDefines, SM));
+
+  // Empty variable
+  GlobalDefines.clear();
+  GlobalDefines.emplace_back(std::string("=18"));
+  EXPECT_TRUE(Cxt.defineCmdlineVariables(GlobalDefines, SM));
+
+  // Invalid variable
+  GlobalDefines.clear();
+  GlobalDefines.emplace_back(std::string("18LocalVar=18"));
+  EXPECT_TRUE(Cxt.defineCmdlineVariables(GlobalDefines, SM));
 
-  // Define local and global variables from command-line.
+  // Define local variables from command-line.
+  GlobalDefines.clear();
   GlobalDefines.emplace_back(std::string("LocalVar=FOO"));
-  Cxt.defineCmdlineVariables(GlobalDefines);
+  GlobalDefines.emplace_back(std::string("EmptyVar="));
+  bool GotError = Cxt.defineCmdlineVariables(GlobalDefines, SM);
+  EXPECT_FALSE(GotError);
 
   // Check defined variables are present and undefined is absent.
   StringRef LocalVarStr = "LocalVar";
+  StringRef EmptyVarStr = "EmptyVar";
   StringRef UnknownVarStr = "UnknownVar";
   llvm::Optional<StringRef> LocalVar = Cxt.getVarValue(LocalVarStr);
+  llvm::Optional<StringRef> EmptyVar = Cxt.getVarValue(EmptyVarStr);
   llvm::Optional<StringRef> UnknownVar = Cxt.getVarValue(UnknownVarStr);
   EXPECT_TRUE(LocalVar);
   EXPECT_EQ(*LocalVar, "FOO");
+  EXPECT_TRUE(EmptyVar);
+  EXPECT_EQ(*EmptyVar, "");
   EXPECT_FALSE(UnknownVar);
 
   // Clear local variables and check they become absent.
   Cxt.clearLocalVars();
   LocalVar = Cxt.getVarValue(LocalVarStr);
   EXPECT_FALSE(LocalVar);
+  EmptyVar = Cxt.getVarValue(EmptyVarStr);
+  EXPECT_FALSE(EmptyVar);
 
   // Redefine global variables and check variables are defined again.
   GlobalDefines.emplace_back(std::string("$GlobalVar=BAR"));
-  Cxt.defineCmdlineVariables(GlobalDefines);
+  GotError = Cxt.defineCmdlineVariables(GlobalDefines, SM);
+  EXPECT_FALSE(GotError);
   StringRef GlobalVarStr = "$GlobalVar";
   llvm::Optional<StringRef> GlobalVar = Cxt.getVarValue(GlobalVarStr);
   EXPECT_TRUE(GlobalVar);