mirror of
https://github.com/RPCS3/llvm-mirror.git
synced 2025-01-31 20:51:52 +01:00
[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
This commit is contained in:
Thomas Preud'homme
parent
59a79d45c8
commit
393fb2cc2e
@ -648,7 +648,7 @@ The ``--enable-var-scope`` option has the same effect on numeric variables as
|
||||
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
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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();
|
||||
|
||||
@ -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: {{^}} ^{{$}}
|
||||
|
||||
@ -58,12 +58,10 @@ static void expectUndefError(const Twine &ExpectedUndefVarName, Error Err) {
|
||||
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]]"));
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user