[llvm] r370663 - [FileCheck] Forbid using var defined on same line
Thomas Preud'homme via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 2 07:04:01 PDT 2019
Author: thopre
Date: Mon Sep 2 07:04:00 2019
New Revision: 370663
URL: http://llvm.org/viewvc/llvm-project?rev=370663&view=rev
Log:
[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
Modified:
llvm/trunk/docs/CommandGuide/FileCheck.rst
llvm/trunk/include/llvm/Support/FileCheck.h
llvm/trunk/lib/Support/FileCheck.cpp
llvm/trunk/test/FileCheck/numeric-expression.txt
llvm/trunk/unittests/Support/FileCheckTest.cpp
Modified: llvm/trunk/docs/CommandGuide/FileCheck.rst
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/CommandGuide/FileCheck.rst?rev=370663&r1=370662&r2=370663&view=diff
==============================================================================
--- llvm/trunk/docs/CommandGuide/FileCheck.rst (original)
+++ llvm/trunk/docs/CommandGuide/FileCheck.rst Mon Sep 2 07:04:00 2019
@@ -648,7 +648,7 @@ The ``--enable-var-scope`` option has th
on string variables.
Important note: In its current implementation, an expression cannot use a
-numeric variable with a non-empty expression defined on the same line.
+numeric variable defined earlier in the same CHECK directive.
FileCheck Pseudo Numeric Variables
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Modified: llvm/trunk/include/llvm/Support/FileCheck.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FileCheck.h?rev=370663&r1=370662&r2=370663&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/FileCheck.h (original)
+++ llvm/trunk/include/llvm/Support/FileCheck.h Mon Sep 2 07:04:00 2019
@@ -98,11 +98,6 @@ private:
/// Name of the numeric variable.
StringRef Name;
- /// Pointer to expression defining this numeric variable. Null for pseudo
- /// variable whose value is known at parse time (e.g. @LINE pseudo variable)
- /// or cleared local variable.
- FileCheckExpressionAST *ExpressionAST;
-
/// Value of numeric variable, if defined, or None otherwise.
Optional<uint64_t> Value;
@@ -113,14 +108,10 @@ private:
public:
/// Constructor for a variable \p Name defined at line \p DefLineNumber or
- /// defined before input is parsed if \p DefLineNumber is None. If not null,
- /// the value set with setValue must match the result of evaluating
- /// \p ExpressionAST.
+ /// defined before input is parsed if \p DefLineNumber is None.
FileCheckNumericVariable(StringRef Name,
- Optional<size_t> DefLineNumber = None,
- FileCheckExpressionAST *ExpressionAST = nullptr)
- : Name(Name), ExpressionAST(ExpressionAST), DefLineNumber(DefLineNumber) {
- }
+ Optional<size_t> DefLineNumber = None)
+ : Name(Name), DefLineNumber(DefLineNumber) {}
/// \returns name of this numeric variable.
StringRef getName() const { return Name; }
@@ -128,25 +119,12 @@ public:
/// \returns this variable's value.
Optional<uint64_t> getValue() const { return Value; }
- /// \returns the pointer to the expression defining this numeric variable, if
- /// any, or null otherwise.
- FileCheckExpressionAST *getExpressionAST() const { return ExpressionAST; }
-
- /// \returns whether this variable's value is known when performing the
- /// substitutions of the line where it is defined.
- bool isValueKnownAtMatchTime() const;
-
- /// Sets value of this numeric variable to \p NewValue. Triggers an assertion
- /// failure if the variable is defined by an expression and the expression
- /// cannot be evaluated to be equal to \p NewValue.
- void setValue(uint64_t NewValue);
+ /// Sets value of this numeric variable to \p NewValue.
+ void setValue(uint64_t NewValue) { Value = NewValue; }
/// Clears value of this numeric variable, regardless of whether it is
/// currently defined or not.
- void clearValue() {
- Value = None;
- ExpressionAST = nullptr;
- }
+ void clearValue() { Value = None; }
/// \returns the line number where this variable is defined, if any, or None
/// if defined before input is parsed.
@@ -609,8 +587,7 @@ private:
/// should defining such a variable be invalid.
static Expected<FileCheckNumericVariable *> parseNumericVariableDefinition(
StringRef &Expr, FileCheckPatternContext *Context,
- Optional<size_t> LineNumber, FileCheckExpressionAST *ExpressionAST,
- const SourceMgr &SM);
+ Optional<size_t> LineNumber, const SourceMgr &SM);
/// Parses \p Name as a (pseudo if \p IsPseudo is true) numeric variable use
/// at line \p LineNumber, or before input is parsed if \p LineNumber is
/// None. Parameter \p Context points to the class instance holding the live
Modified: llvm/trunk/lib/Support/FileCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/FileCheck.cpp?rev=370663&r1=370662&r2=370663&view=diff
==============================================================================
--- llvm/trunk/lib/Support/FileCheck.cpp (original)
+++ llvm/trunk/lib/Support/FileCheck.cpp Mon Sep 2 07:04:00 2019
@@ -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::parseNumericVariableDe
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::parseNumericVariableUs
}
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::parseNumericSubstituti
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();
Modified: llvm/trunk/test/FileCheck/numeric-expression.txt
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/FileCheck/numeric-expression.txt?rev=370663&r1=370662&r2=370663&view=diff
==============================================================================
--- llvm/trunk/test/FileCheck/numeric-expression.txt (original)
+++ llvm/trunk/test/FileCheck/numeric-expression.txt Mon Sep 2 07:04:00 2019
@@ -85,10 +85,10 @@ CHECK-NEXT: [[#VAR1+VAR2]]
; Numeric expression using a variable defined from a numeric expression.
DEF EXPR GOOD MATCH
42
-41 43
+41
; CHECK-LABEL: DEF EXPR GOOD MATCH
; CHECK-NEXT: [[# VAR42:VAR1+31]]
-; CHECK-NEXT: [[# VAR41: VAR42-1]] [[# VAR41 + 2]]
+; CHECK-NEXT: [[# VAR42-1]]
; Empty numeric expression.
EMPTY NUM EXPR
@@ -189,3 +189,27 @@ DEF-EXPR-FAIL-NEXT: [[# VAR42: VAR20+22]
DEF-EXPR-FAIL-MSG: numeric-expression.txt:[[#@LINE-1]]:21: error: {{D}}EF-EXPR-FAIL-NEXT: is not on the line after the previous match
DEF-EXPR-FAIL-MSG-NEXT: {{D}}EF-EXPR-FAIL-NEXT: {{\[\[# VAR42: VAR20\+22\]\]}}
DEF-EXPR-FAIL-MSG-NEXT: {{^}} ^{{$}}
+
+; Verify that using a numeric variable defined on the same line (whether from
+; input or from an expression) is rejected.
+RUN: not FileCheck --check-prefix SAME-LINE-USE1 --input-file %s %s 2>&1 \
+RUN: | FileCheck --strict-whitespace --check-prefix SAME-LINE-USE-MSG1 %s
+RUN: not FileCheck --check-prefix SAME-LINE-USE2 --input-file %s %s 2>&1 \
+RUN: | FileCheck --strict-whitespace --check-prefix SAME-LINE-USE-MSG2 %s
+
+SAME LINE USE
+3
+4 5
+SAME-LINE-USE1-LABEL: SAME LINE USE
+SAME-LINE-USE1-NEXT: [[#]]
+SAME-LINE-USE1-NEXT: [[#VAR1:]] [[#VAR1+1]]
+SAME-LINE-USE-MSG1: numeric-expression.txt:[[#@LINE-1]]:36: error: numeric variable 'VAR1' defined earlier in the same CHECK directive
+SAME-LINE-USE-MSG1-NEXT: {{S}}AME-LINE-USE1-NEXT: {{\[\[#VAR1:\]\] \[\[#VAR1\+1\]\]}}
+SAME-LINE-USE-MSG1-NEXT: {{^}} ^{{$}}
+
+SAME-LINE-USE2-LABEL: SAME LINE USE
+SAME-LINE-USE2-NEXT: [[#VAR1:]]
+SAME-LINE-USE2-NEXT: [[#VAR2:VAR1+1]] [[#VAR2+1]]
+SAME-LINE-USE-MSG2: numeric-expression.txt:[[#@LINE-1]]:42: error: numeric variable 'VAR2' defined earlier in the same CHECK directive
+SAME-LINE-USE-MSG2-NEXT: {{S}}AME-LINE-USE2-NEXT: {{\[\[#VAR2:VAR1\+1\]\] \[\[#VAR2\+1\]\]}}
+SAME-LINE-USE-MSG2-NEXT: {{^}} ^{{$}}
Modified: llvm/trunk/unittests/Support/FileCheckTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/FileCheckTest.cpp?rev=370663&r1=370662&r2=370663&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/FileCheckTest.cpp (original)
+++ llvm/trunk/unittests/Support/FileCheckTest.cpp Mon Sep 2 07:04:00 2019
@@ -58,12 +58,10 @@ static void expectUndefError(const Twine
uint64_t doAdd(uint64_t OpL, uint64_t OpR) { return OpL + OpR; }
TEST_F(FileCheckTest, NumericVariable) {
- // Undefined variable: isValueKnownAtMatchTime returns false, getValue and
- // eval fail, error returned by eval holds the name of the undefined
- // variable.
+ // Undefined variable: getValue and eval fail, error returned by eval holds
+ // the name of the undefined variable.
FileCheckNumericVariable FooVar = FileCheckNumericVariable("FOO", 1);
EXPECT_EQ("FOO", FooVar.getName());
- EXPECT_FALSE(FooVar.isValueKnownAtMatchTime());
FileCheckNumericVariableUse FooVarUse =
FileCheckNumericVariableUse("FOO", &FooVar);
EXPECT_FALSE(FooVar.getValue());
@@ -73,9 +71,7 @@ TEST_F(FileCheckTest, NumericVariable) {
FooVar.setValue(42);
- // Defined variable: isValueKnownAtMatchTime returns true, getValue and eval
- // return value set.
- EXPECT_TRUE(FooVar.isValueKnownAtMatchTime());
+ // Defined variable: getValue and eval return value set.
Optional<uint64_t> Value = FooVar.getValue();
ASSERT_TRUE(bool(Value));
EXPECT_EQ(42U, *Value);
@@ -83,43 +79,13 @@ TEST_F(FileCheckTest, NumericVariable) {
ASSERT_TRUE(bool(EvalResult));
EXPECT_EQ(42U, *EvalResult);
- // Variable defined by numeric expression: isValueKnownAtMatchTime
- // returns true, getValue and eval return value of expression, setValue
- // clears expression.
- std::unique_ptr<FileCheckNumericVariableUse> FooVarUsePtr =
- std::make_unique<FileCheckNumericVariableUse>("FOO", &FooVar);
- std::unique_ptr<FileCheckExpressionLiteral> One =
- std::make_unique<FileCheckExpressionLiteral>(1);
- FileCheckASTBinop Binop =
- FileCheckASTBinop(doAdd, std::move(FooVarUsePtr), std::move(One));
- FileCheckNumericVariable FoobarExprVar =
- FileCheckNumericVariable("FOOBAR", 2, &Binop);
- EXPECT_TRUE(FoobarExprVar.isValueKnownAtMatchTime());
- ASSERT_FALSE(FoobarExprVar.getValue());
- FileCheckNumericVariableUse FoobarExprVarUse =
- FileCheckNumericVariableUse("FOOBAR", &FoobarExprVar);
- EvalResult = FoobarExprVarUse.eval();
- ASSERT_TRUE(bool(EvalResult));
- EXPECT_EQ(43U, *EvalResult);
- EXPECT_TRUE(FoobarExprVar.getExpressionAST());
- FoobarExprVar.setValue(43);
- EXPECT_FALSE(FoobarExprVar.getExpressionAST());
- FoobarExprVar = FileCheckNumericVariable("FOOBAR", 2, &Binop);
- EXPECT_TRUE(FoobarExprVar.getExpressionAST());
-
// Clearing variable: getValue and eval fail. Error returned by eval holds
// the name of the cleared variable.
FooVar.clearValue();
- FoobarExprVar.clearValue();
- EXPECT_FALSE(FoobarExprVar.getExpressionAST());
EXPECT_FALSE(FooVar.getValue());
- EXPECT_FALSE(FoobarExprVar.getValue());
EvalResult = FooVarUse.eval();
ASSERT_FALSE(EvalResult);
expectUndefError("FOO", EvalResult.takeError());
- EvalResult = FoobarExprVarUse.eval();
- ASSERT_FALSE(EvalResult);
- expectUndefError("FOOBAR", EvalResult.takeError());
}
TEST_F(FileCheckTest, Binop) {
@@ -332,12 +298,12 @@ TEST_F(FileCheckTest, ParseExpr) {
// Valid empty expression.
EXPECT_FALSE(Tester.parseSubstExpect(""));
- // Valid use of variable defined on the same line from expression. Note that
- // the same pattern object is used for the parsePatternExpect and
+ // Invalid use of variable defined on the same line from expression. Note
+ // that the same pattern object is used for the parsePatternExpect and
// parseSubstExpect since no initNextPattern is called, thus appearing as
// being on the same line from the pattern's point of view.
ASSERT_FALSE(Tester.parsePatternExpect("[[#LINE1VAR:FOO+1]]"));
- EXPECT_FALSE(Tester.parseSubstExpect("LINE1VAR"));
+ EXPECT_TRUE(Tester.parseSubstExpect("LINE1VAR"));
// Invalid use of variable defined on same line from input. As above, the
// absence of a call to initNextPattern makes it appear to be on the same
@@ -354,8 +320,9 @@ TEST_F(FileCheckTest, ParseExpr) {
// Valid expression.
EXPECT_FALSE(Tester.parseSubstExpect("@LINE+5"));
EXPECT_FALSE(Tester.parseSubstExpect("FOO+4"));
- EXPECT_FALSE(Tester.parseSubstExpect("FOOBAR"));
Tester.initNextPattern();
+ EXPECT_FALSE(Tester.parseSubstExpect("FOOBAR"));
+ EXPECT_FALSE(Tester.parseSubstExpect("LINE1VAR"));
EXPECT_FALSE(Tester.parsePatternExpect("[[#FOO+FOO]]"));
EXPECT_FALSE(Tester.parsePatternExpect("[[#FOO+3-FOO]]"));
}
More information about the llvm-commits
mailing list