[FileCheck] Forbid using var defined on same line

Summary:
Commit r366897 introduced the possibility to set a variable from an
expression, such as [[#VAR2:VAR1+3]]. While introducing this feature, it
introduced extra logic to allow using such a variable on the same line
later on. Unfortunately that extra logic is flawed as it relies on a
mapping from variable to expression defining it when the mapping is from
variable definition to expression. This flaw causes among other issues
PR42896.

This commit avoids the problem by forbidding all use of a variable
defined on the same line, and removes the now useless logic. Redesign
will be done in a later commit because it will require some amount of
refactoring first for the solution to be clean. One example is the need
for some sort of transaction mechanism to set a variable temporarily and
from an expression and rollback if the CHECK pattern does not match so
that diagnostics show the right variable values.

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

llvm-svn: 370663

1 files changed, 6 insertions, 36 deletions

diff --git a/llvm/lib/Support/FileCheck.cpp b/llvm/lib/Support/FileCheck.cpp
index 9310a79..5c05af4 100644
--- a/llvm/lib/Support/FileCheck.cpp
+++ b/llvm/lib/Support/FileCheck.cpp
@@ -24,38 +24,12 @@
 
 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;
 
-  FileCheckExpressionAST *ExpressionAST = NumericVariable->getExpressionAST();
-  if (!ExpressionAST)
-    return make_error<FileCheckUndefVarError>(Name);
-
-  return ExpressionAST->eval();
+  return make_error<FileCheckUndefVarError>(Name);
 }
 
 Expected<uint64_t> FileCheckASTBinop::eval() const {
@@ -141,8 +115,7 @@ char FileCheckNotFoundError::ID = 0;
 Expected<FileCheckNumericVariable *>
 FileCheckPattern::parseNumericVariableDefinition(
     StringRef &Expr, FileCheckPatternContext *Context,
-    Optional<size_t> LineNumber, FileCheckExpressionAST *ExpressionAST,
-    const SourceMgr &SM) {
+    Optional<size_t> LineNumber, const SourceMgr &SM) {
   Expected<VariableProperties> ParseVarResult = parseVariable(Expr, SM);
   if (!ParseVarResult)
     return ParseVarResult.takeError();
@@ -169,8 +142,7 @@ FileCheckPattern::parseNumericVariableDefinition(
   if (VarTableIter != Context->GlobalNumericVariableTable.end())
     DefinedNumericVariable = VarTableIter->second;
   else
-    DefinedNumericVariable =
-        Context->makeNumericVariable(Name, LineNumber, ExpressionAST);
+    DefinedNumericVariable = Context->makeNumericVariable(Name, LineNumber);
 
   return DefinedNumericVariable;
 }
@@ -202,12 +174,11 @@ FileCheckPattern::parseNumericVariableUse(StringRef Name, bool IsPseudo,
   }
 
   Optional<size_t> DefLineNumber = NumericVariable->getDefLineNumber();
-  if (DefLineNumber && LineNumber && *DefLineNumber == *LineNumber &&
-      !NumericVariable->isValueKnownAtMatchTime())
+  if (DefLineNumber && LineNumber && *DefLineNumber == *LineNumber)
     return FileCheckErrorDiagnostic::get(
         SM, Name,
         "numeric variable '" + Name +
-            "' defined from input on the same line as used");
+            "' defined earlier in the same CHECK directive");
 
   return std::make_unique<FileCheckNumericVariableUse>(Name, NumericVariable);
 }
@@ -334,8 +305,7 @@ FileCheckPattern::parseNumericSubstitutionBlock(
   if (DefEnd != StringRef::npos) {
     DefExpr = DefExpr.ltrim(SpaceChars);
     Expected<FileCheckNumericVariable *> ParseResult =
-        parseNumericVariableDefinition(DefExpr, Context, LineNumber,
-                                       ExpressionAST.get(), SM);
+        parseNumericVariableDefinition(DefExpr, Context, LineNumber, SM);
 
     if (!ParseResult)
       return ParseResult.takeError();