From 53ea3b074cd9fb01df22e1fa0bed50219ac7638a Mon Sep 17 00:00:00 2001 From: Thomas Preud'homme Date: Fri, 5 Jul 2019 16:25:46 +0000 Subject: [PATCH] [FileCheck] Share variable instance among uses Summary: This patch changes expression support to use one instance of FileCheckNumericVariable per numeric variable rather than one per variable and per definition. The current system was only necessary for the last patch of the numeric expression support patch series in order to handle a line using a variable defined earlier on the same line from the input text. However this can be dealt more efficiently. 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/D64229 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@365220 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Support/FileCheck.h | 34 ++++++++------- lib/Support/FileCheck.cpp | 67 +++++++++++++++++------------ unittests/Support/FileCheckTest.cpp | 5 ++- 3 files changed, 61 insertions(+), 45 deletions(-) diff --git a/include/llvm/Support/FileCheck.h b/include/llvm/Support/FileCheck.h index 284ea30902c..dea68ba482c 100644 --- a/include/llvm/Support/FileCheck.h +++ b/include/llvm/Support/FileCheck.h @@ -40,10 +40,7 @@ struct FileCheckRequest { // Numeric substitution handling code. //===----------------------------------------------------------------------===// -/// Class representing a numeric variable with a given value in a numeric -/// expression. Each definition of a variable gets its own instance of this -/// class. Variable uses share the same instance as their respective -/// definition. +/// Class representing a numeric variable and its associated current value. class FileCheckNumericVariable { private: /// Name of the numeric variable. @@ -271,14 +268,16 @@ private: StringMap DefinedVariableTable; /// When matching a given pattern, this holds the pointers to the classes - /// representing the last definitions of numeric variables defined in - /// previous patterns. Earlier definitions of the variables, if any, have - /// their own class instance not referenced by this table. When matching a - /// pattern all definitions for that pattern are recorded in the + /// representing the numeric variables defined in previous patterns. When + /// matching a pattern all definitions for that pattern are recorded in the /// NumericVariableDefs table in the FileCheckPattern instance of that /// pattern. StringMap GlobalNumericVariableTable; + /// Pointer to the class instance representing the @LINE pseudo variable for + /// easily updating its value. + FileCheckNumericVariable *LineVariable = nullptr; + /// Vector holding pointers to all parsed expressions. Used to automatically /// free the expressions once they are guaranteed to no longer be used. std::vector> Expressions; @@ -303,6 +302,10 @@ public: Error defineCmdlineVariables(std::vector &CmdlineDefines, SourceMgr &SM); + /// Create @LINE pseudo variable. Value is set when pattern are being + /// matched. + void createLineVariable(); + /// Undefines local variables (variables whose name does not start with a '$' /// sign), i.e. removes them from GlobalVariableTable and from /// GlobalNumericVariableTable and also clears the value of numeric @@ -462,13 +465,14 @@ public: /// name. static Expected parseVariable(StringRef &Str, bool &IsPseudo, const SourceMgr &SM); - /// Parses \p Expr for the name of a numeric variable to be defined. \returns - /// an error holding a diagnostic against \p SM should defining such a - /// variable be invalid, or Success otherwise. In the latter case, sets - /// \p Name to the name of the parsed numeric variable name. - static Error parseNumericVariableDefinition(StringRef &Expr, StringRef &Name, - FileCheckPatternContext *Context, - const SourceMgr &SM); + /// Parses \p Expr for the name of a numeric variable to be defined at line + /// \p LineNumber. \returns a pointer to the class instance representing that + /// variable, creating it if needed, or an error holding a diagnostic against + /// \p SM should defining such a variable be invalid. + static Expected + parseNumericVariableDefinition(StringRef &Expr, + FileCheckPatternContext *Context, + size_t LineNumber, const SourceMgr &SM); /// Parses \p Expr for a numeric substitution block. \returns the class /// representing the AST of the expression whose value must be substituted, /// or an error holding a diagnostic against \p SM if parsing fails. If diff --git a/lib/Support/FileCheck.cpp b/lib/Support/FileCheck.cpp index f93dde26c32..03e892af274 100644 --- a/lib/Support/FileCheck.cpp +++ b/lib/Support/FileCheck.cpp @@ -110,14 +110,15 @@ char FileCheckUndefVarError::ID = 0; char FileCheckErrorDiagnostic::ID = 0; char FileCheckNotFoundError::ID = 0; -Error FileCheckPattern::parseNumericVariableDefinition( - StringRef &Expr, StringRef &Name, FileCheckPatternContext *Context, +Expected +FileCheckPattern::parseNumericVariableDefinition( + StringRef &Expr, FileCheckPatternContext *Context, size_t LineNumber, const SourceMgr &SM) { bool IsPseudo; Expected ParseVarResult = parseVariable(Expr, IsPseudo, SM); if (!ParseVarResult) return ParseVarResult.takeError(); - Name = *ParseVarResult; + StringRef Name = *ParseVarResult; if (IsPseudo) return FileCheckErrorDiagnostic::get( @@ -135,7 +136,14 @@ Error FileCheckPattern::parseNumericVariableDefinition( return FileCheckErrorDiagnostic::get( SM, Expr, "unexpected characters after numeric variable name"); - return Error::success(); + FileCheckNumericVariable *DefinedNumericVariable; + auto VarTableIter = Context->GlobalNumericVariableTable.find(Name); + if (VarTableIter != Context->GlobalNumericVariableTable.end()) + DefinedNumericVariable = VarTableIter->second; + else + DefinedNumericVariable = Context->makeNumericVariable(LineNumber, Name); + + return DefinedNumericVariable; } Expected @@ -154,12 +162,11 @@ FileCheckPattern::parseNumericVariableUse(StringRef &Expr, // Numeric variable definitions and uses are parsed in the order in which // they appear in the CHECK patterns. For each definition, the pointer to the // class instance of the corresponding numeric variable definition is stored - // in GlobalNumericVariableTable in parsePattern. Therefore, the pointer we - // get below is for the class instance corresponding to the last definition - // of this variable use. If we don't find a variable definition we create a - // dummy one so that parsing can continue. All uses of undefined variables, - // whether string or numeric, are then diagnosed in printSubstitutions() - // after failing to match. + // in GlobalNumericVariableTable in parsePattern. Therefore, if the pointer + // we get below is null, it means no such variable was defined before. When + // that happens, we create a dummy variable so that parsing can continue. All + // uses of undefined variables, whether string or numeric, are then diagnosed + // in printSubstitutions() after failing to match. auto VarTableIter = Context->GlobalNumericVariableTable.find(Name); FileCheckNumericVariable *NumericVariable; if (VarTableIter != Context->GlobalNumericVariableTable.end()) @@ -249,11 +256,11 @@ Expected FileCheckPattern::parseNumericSubstitutionBlock( "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); + Expected ParseResult = + parseNumericVariableDefinition(DefExpr, Context, LineNumber, SM); + if (!ParseResult) + return ParseResult.takeError(); + DefinedNumericVariable = *ParseResult; return Context->makeExpression(add, nullptr, 0); } @@ -270,13 +277,6 @@ bool FileCheckPattern::parsePattern(StringRef PatternStr, StringRef Prefix, PatternLoc = SMLoc::getFromPointer(PatternStr.data()); - // Create fake @LINE pseudo variable definition. - StringRef LinePseudo = "@LINE"; - uint64_t LineNumber64 = LineNumber; - FileCheckNumericVariable *LinePseudoVar = - Context->makeNumericVariable(LinePseudo, LineNumber64); - Context->GlobalNumericVariableTable[LinePseudo] = LinePseudoVar; - if (!(Req.NoCanonicalizeWhiteSpace && Req.MatchFullLines)) // Ignore trailing whitespace. while (!PatternStr.empty() && @@ -571,6 +571,7 @@ Expected FileCheckPattern::match(StringRef Buffer, size_t &MatchLen, std::string TmpStr; if (!Substitutions.empty()) { TmpStr = RegExStr; + Context->LineVariable->setValue(LineNumber); size_t InsertOffset = 0; // Substitute all string variables and expressions whose values are only @@ -590,6 +591,7 @@ Expected FileCheckPattern::match(StringRef Buffer, size_t &MatchLen, // Match the newly constructed regex. RegExToMatch = TmpStr; + Context->LineVariable->clearValue(); } SmallVector MatchInfo; @@ -1058,6 +1060,13 @@ FindFirstMatchingPrefix(Regex &PrefixRE, StringRef &Buffer, return {StringRef(), StringRef()}; } +void FileCheckPatternContext::createLineVariable() { + assert(!LineVariable && "@LINE pseudo numeric variable already created"); + StringRef LineName = "@LINE"; + LineVariable = makeNumericVariable(0, LineName); + GlobalNumericVariableTable[LineName] = LineVariable; +} + bool FileCheck::ReadCheckFile(SourceMgr &SM, StringRef Buffer, Regex &PrefixRE, std::vector &CheckStrings) { Error DefineError = @@ -1067,6 +1076,8 @@ bool FileCheck::ReadCheckFile(SourceMgr &SM, StringRef Buffer, Regex &PrefixRE, return true; } + PatternContext.createLineVariable(); + std::vector ImplicitNegativeChecks; for (const auto &PatternString : Req.ImplicitCheckNot) { // Create a buffer with fake command line content in order to display the @@ -1739,11 +1750,11 @@ Error FileCheckPatternContext::defineCmdlineVariables( // Numeric variable definition. if (CmdlineDef[0] == '#') { StringRef CmdlineName = CmdlineDef.substr(1, EqIdx - 1); - StringRef VarName; - Error ErrorDiagnostic = FileCheckPattern::parseNumericVariableDefinition( - CmdlineName, VarName, this, SM); - if (ErrorDiagnostic) { - Errs = joinErrors(std::move(Errs), std::move(ErrorDiagnostic)); + Expected ParseResult = + FileCheckPattern::parseNumericVariableDefinition(CmdlineName, this, 0, + SM); + if (!ParseResult) { + Errs = joinErrors(std::move(Errs), ParseResult.takeError()); continue; } @@ -1757,7 +1768,7 @@ Error FileCheckPatternContext::defineCmdlineVariables( CmdlineVal + "'")); continue; } - auto DefinedNumericVariable = makeNumericVariable(0, VarName); + FileCheckNumericVariable *DefinedNumericVariable = *ParseResult; DefinedNumericVariable->setValue(Val); // Record this variable definition. diff --git a/unittests/Support/FileCheckTest.cpp b/unittests/Support/FileCheckTest.cpp index b9970ffc1f7..7fcd5bba608 100644 --- a/unittests/Support/FileCheckTest.cpp +++ b/unittests/Support/FileCheckTest.cpp @@ -178,6 +178,7 @@ public: GlobalDefines.emplace_back(std::string("BAR=BAZ")); EXPECT_FALSE( errorToBool(Context.defineCmdlineVariables(GlobalDefines, SM))); + Context.createLineVariable(); // Call parsePattern to have @LINE defined. P.parsePattern("N/A", "CHECK", SM, Req); // parsePattern does not expect to be called twice for the same line and @@ -192,9 +193,9 @@ public: bool parseNumVarDefExpect(StringRef Expr) { StringRef ExprBufferRef = bufferize(SM, Expr); - StringRef Name; return errorToBool(FileCheckPattern::parseNumericVariableDefinition( - ExprBufferRef, Name, &Context, SM)); + ExprBufferRef, &Context, LineNumber, SM) + .takeError()); } bool parseSubstExpect(StringRef Expr) { -- 2.40.0