FileCheck [1/12]: Move variable table in new object

[thopre]

Summary

This patch is part of a patch series to add support for FileCheck
numeric expressions. This specific patch adds a new class to hold
pattern matching global state.

The table holding the values of FileCheck variable constitutes some sort
of global state for the matching phase, yet is passed as parameters of
all functions using it. This commit create a new FileCheckPatternContext
class pointed at from FileCheckPattern. While it increases the line
count, it separates local data from global state. Later commits build
on that to add numeric expression global state to that class.

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)

[thopre]

Use camel casing for new functions

[thopre]

Fix camel casing of modified functions as well

[jhenderson]

llvm/include/llvm/Support/FileCheck.h

84

This forward declaration should probably be above your new class, so that your class sits next to the FileCheckPattern class.

100

Is there going to be a getNumericVarValue or similar function in the future? If not, I think this should be simply getVarValue or getVariableValue.

102

As numeric variables don't exist at this point, your comments should probably not refer to them. You can add the reference once they start becoming relevant here.

107

Nit: ie -> i.e.

108

Nit: you don't need the explicit void parameter.

138

Just to be clear, this is shared between all patterns?

llvm/lib/Support/FileCheck.cpp

271–272

I'd mention that the GlobalVariableTable is in the Context.

305–306

I don't think you need to have Opt in the name, as it is fairly obviously an Optional from context. Maybe better would be VarValue.

308

It is more typical to do if (!OptValue) for Optionals.

312

More common is to do *OptValue.

390

See comments above about the name and the usage immediately below.

478

nothing -> None (to be explicit).

482

"git" is a meaningless acronym to me. Perhaps simply "VarIter" or something like that would be better.

760

unique_ptr or shared_ptr?

1387

Is this necessary? Under what circumstances do we read the file twice?

1391

Could you make this a StringRef here? That would simplify the code below.

1396

Nit: ie -> i.e.

1399

Don't use auto here. I don't know what the type is immediately.

[thopre]

• Make context a shared_ptr
• Don't use auto for parameter of foreach loop if collection is not a local variable
• Fix usage of Optional objects
• Remove void function parameter
• Remove references to pattern variable, be it in comment or identifiers
• Fix ie -> i.e. typo

[thopre]

Thanks for such a quick review James! I addressed all your comments, except perhaps the one about guarding against DefineCmdlineVariables being called twice. Let me know if my fix seems sensible to you.

llvm/include/llvm/Support/FileCheck.h

100

There is no getNumericVarValue in the future but this function is only for Pattern variable. Numeric variable use are represented by a pointer to the structure that holds the value so there is no need for a function. That said the term of pattern variable is introduced later so I could rename it later.

138

Yes, updated the comment to make it clear.

llvm/lib/Support/FileCheck.cpp

1387

I wanted to avoid this function being called twice by mistake and thus overriding variable values read from the input file by the value on the command-line. An assert is fine but it'll still need that static variable. Is that ok?

[jhenderson]

llvm/lib/Support/FileCheck.cpp

1387

I'd actually look at it a different way. Clear the variables list and only add the new values. That way this class is more flexible. The problem with a static local is that it is just global state with a local visibility. This would make it impossible to, for example, write unit tests that use this code. Speaking of which, how practical would adding unit tests be for the new code?

1411–1412

In this case you can just use the raw pointer referenced by the shared_ptr, as you don't need to own the pointer here.

[thopre]

• Prevent variable definition from command-line switches if some variables are already defined
• Use raw FileCheckPatternContext pointer to clear local variables

[jhenderson]

Have you looked at unit-testing the new code? It might be nice if you can.

llvm/lib/Support/FileCheck.cpp

1385–1386

I'm okay with this, but an alternative method based on a different bit of code in the DWARFDebug* parser classes, might be to explicitly call GlobalVariableTable.clear() at the start of this function.

No need to change unless you want to.

1438

Under what circumstances can Context be nullptr?

[thopre]

llvm/lib/Support/FileCheck.cpp

1385–1386

Command-line defines are called "GlobalDefines" in the FileCheckRequest API so my understanding is variables defined from processing the input file override the global defines rather than the reverse so no command-line definition should be processed once any variable has been defined from the input text.

Note that the API associate one FileCheckRequest to a given FileCheck class so 1 request = 1 input file and 1 directive file. There is thus no need to support the case of several input file sharing the same set of global defines. Since each input file gets its own FileCheckPatternContext, they will all get their own GlobalVariableTable which should be empty before the input text starts to be processed. This assert therefore does not get triggered in normal usage (global defined processed first) and will fire if global defines would be overriding variable defined from the input text.

I thus think this is a better approach.

1438

When CheckStrings is empty but I've just realized thanks to you that readCheckFile guarantees it is not empty.

[arsenm]

llvm/include/llvm/Support/FileCheck.h

138

Why is this shared_ptr? For a context, there should be clear ownership. Each pattern doesn't need to have a potentially owning reference. It can live somewhere else

[thopre]

• Remove useless null check
• Add unit tests

[thopre]

llvm/include/llvm/Support/FileCheck.h

138

Hi Matt, thanks for the review!

The object is created in readCheckFile function and cease to be used in checkInput without a pointer being passed around explicitely. I used to delete the pointer taken from one of the pattern but James suggested to use a shared_ptr. It is overkill but allow the code to still work in case it is changed slightly. If the patterns hold a regular pointer I don't know in what shared_ptr to store the ownership and would need to revert to the initial approach.

Is there any other solution I've missed?

[thopre]

llvm/include/llvm/Support/FileCheck.h

138

How about creating a pointer to the context in the FileCheck class? I'm tempted to make it a regular pointer since it is clear it can be destroyed once checkInput is finished but let me know if I should be using a shared_ptr instead.

[arsenm]

llvm/include/llvm/Support/FileCheck.h

138

Yes, that's what I was thinking. Each instance needs access to the context, but it shouldn't potentially own it.

[thopre]

Make Context a member of FileCheck class and stop using shared_ptr

[jhenderson]

Basically looks good to me. Just a few small comments remaining.

llvm/lib/Support/FileCheck.cpp

1381–1382

I might be wrong, but I'm not sure you need (identical) doxygen comments in both source and header, so I think this and similar ones can be removed?

1393

Nit: no need for the void parameter.

llvm/unittests/Support/FileCheckTest.cpp

44

I would add one last check to this test: re-add a local variable and show that you can fetch it again.

[thopre]

• Remove comments for new functions from cpp file
• remove void from parameter list

[thopre]

Fix comment to refer to None rather than nothing

[probinson]

llvm/lib/Support/FileCheck.cpp

1381–1382

FTR, I'm quite sure doxygen comments go in the header if there is one. FileCheck was not tidied up when it moved to be a library, I think; would be worth a separate cleanup patch after the functional stuff is done, to get that all sorted.

[jhenderson]

llvm/unittests/Support/FileCheckTest.cpp

44

Okay, just waiting for this point to be addressed before I LGTM this.

[thopre]

llvm/lib/Support/FileCheck.cpp

1381–1382

It is a maintenance nightmare too. However it is useful in the header to document the API and in the code as a comment. It seems to be also the case for existing functions. It does seem that in the rest of LLVM the comments are in the header usually, with occasional comment in the source as well. I have thus followed your suggestion and removed the comment in the cpp file, except when already present.

llvm/unittests/Support/FileCheckTest.cpp

44

The API does not provide a function to add a variable so I'm not sure how I would do that. FileCheckPattern is friend of FileCheckPatternContext so can add it directly so as to avoid creating a any user of the class to be able to add variables.

[jhenderson]

llvm/unittests/Support/FileCheckTest.cpp

44

Oh, right, yes of course. I forgot you decided to go the assert route in defineCmdlineVariables. Perhaps in another test you could run defineCmdlineVariables with just locals, clear them, then run it again?

[thopre]

llvm/lib/Support/FileCheck.cpp

1381–1382

Which is what I did in this patch and am similarly removing comments from the cpp files in later patches.

llvm/unittests/Support/FileCheckTest.cpp

44

See my answer above. Do you want me to create a function to set a variable and make it public?

[thopre]

Add redefinition unit testing

[thopre]

llvm/unittests/Support/FileCheckTest.cpp

44

Oops, just saw your comment about local vars after I wrote the above message. Will fix that immediately

[jhenderson]

LGTM, with one nit.

llvm/unittests/Support/FileCheckTest.cpp

48

Either variables -> variable or remains -> remain (but not both).

[thopre]

Fix grammar in unit test comment

llvm/include/llvm/Support/FileCheck.h

public:
 
   operator FileCheckKind() const { return Kind; }
 
   int getCount() const { return Count; }
   FileCheckType &setCount(int C);
 
   std::string getDescription(StringRef Prefix) const;
 };
-}
+} // namespace Check
 
 struct FileCheckDiag;
 
+/// Class holding the FileCheckPattern global state, shared by all patterns:
+/// tables holding values of variables and whether they are defined or not at
+/// any given time in the matching process.
+class FileCheckPatternContext {
+  friend class FileCheckPattern;
+
+private:
+  /// When matching a given pattern, this holds the value of all the FileCheck
+  /// variables defined in previous patterns. In a pattern only the last
+  /// definition for a given variable is recorded in this table, back-references
+  /// are used for uses after any the other definition.
+  StringMap<StringRef> GlobalVariableTable;
+
+public:
+  /// Return the value of variable \p VarName or nothing if no such variable has
+  /// been defined.
+  llvm::Optional<StringRef> getVarValue(StringRef VarName);
+
+  /// Define variables from definitions given on the command line passed as a
+  /// vector of VAR=VAL strings in \p CmdlineDefines.
+  void defineCmdlineVariables(std::vector<std::string> &CmdlineDefines);
+
+  /// Undefine local variables (variables whose name does not start with a '$'
+  /// sign), i.e. remove them from GlobalVariableTable.
+  void clearLocalVars();
+};
+
 class FileCheckPattern {
   SMLoc PatternLoc;
 
   /// A fixed string to match as the pattern or empty if this pattern requires
   /// a regex match.
   StringRef FixedStr;
 
   /// A regex string to match as the pattern or empty if this pattern requires
   /// a fixed string to match.
   std::string RegExStr;
 
   /// Entries in this vector map to uses of a variable in the pattern, e.g.
   /// "foo[[bar]]baz".  In this case, the RegExStr will contain "foobaz" and
   /// we'll get an entry in this vector that tells us to insert the value of
   /// bar at offset 3.
   std::vector<std::pair<StringRef, unsigned>> VariableUses;
 
   /// Maps definitions of variables to their parenthesized capture numbers.
   ///
   /// E.g. for the pattern "foo[[bar:.*]]baz", VariableDefs will map "bar" to
   /// 1.
   std::map<StringRef, unsigned> VariableDefs;
 
+  /// Pointer to the class instance shared by all patterns holding a table with
+  /// the values of live variables at the start of any given CHECK line.
+  FileCheckPatternContext *Context;
+
   Check::FileCheckType CheckTy;
 
   /// Contains the number of line this pattern is in.
   unsigned LineNumber;
 
 public:
-  explicit FileCheckPattern(Check::FileCheckType Ty)
-      : CheckTy(Ty) {}
+  explicit FileCheckPattern(Check::FileCheckType Ty,
+                            FileCheckPatternContext *Context)
+      : Context(Context), CheckTy(Ty) {}
 
   /// Returns the location in source code.
   SMLoc getLoc() const { return PatternLoc; }
 
+  /// Returns the pointer to the global state for all patterns in this
+  /// FileCheck instance.
+  FileCheckPatternContext *getContext() const { return Context; }
   bool ParsePattern(StringRef PatternStr, StringRef Prefix, SourceMgr &SM,
                     unsigned LineNumber, const FileCheckRequest &Req);
-  size_t Match(StringRef Buffer, size_t &MatchLen,
-               StringMap<StringRef> &VariableTable) const;
-  void PrintVariableUses(const SourceMgr &SM, StringRef Buffer,
-                         const StringMap<StringRef> &VariableTable,
+  size_t match(StringRef Buffer, size_t &MatchLen) const;
+  void printVariableUses(const SourceMgr &SM, StringRef Buffer,
                          SMRange MatchRange = None) const;
-  void PrintFuzzyMatch(const SourceMgr &SM, StringRef Buffer,
-                       const StringMap<StringRef> &VariableTable,
+  void printFuzzyMatch(const SourceMgr &SM, StringRef Buffer,
                        std::vector<FileCheckDiag> *Diags) const;
 
   bool hasVariable() const {
     return !(VariableUses.empty() && VariableDefs.empty());
   }
 
   Check::FileCheckType getCheckTy() const { return CheckTy; }
 
   int getCount() const { return CheckTy.getCount(); }
 
 private:
   bool AddRegExToRegEx(StringRef RS, unsigned &CurParen, SourceMgr &SM);
   void AddBackrefToRegEx(unsigned BackrefNum);
-  unsigned
-  ComputeMatchDistance(StringRef Buffer,
-                       const StringMap<StringRef> &VariableTable) const;
+  unsigned computeMatchDistance(StringRef Buffer) const;
   bool EvaluateExpression(StringRef Expr, std::string &Value) const;
   size_t FindRegexVarEnd(StringRef Str, SourceMgr &SM);
 };
 
 //===----------------------------------------------------------------------===//
 /// Summary of a FileCheck diagnostic.
 //===----------------------------------------------------------------------===//
 
struct FileCheckString {
   /// All of the strings that are disallowed from occurring between this match
   /// string and the previous one (or start of file).
   std::vector<FileCheckPattern> DagNotStrings;
 
   FileCheckString(const FileCheckPattern &P, StringRef S, SMLoc L)
       : Pat(P), Prefix(S), Loc(L) {}
 
   size_t Check(const SourceMgr &SM, StringRef Buffer, bool IsLabelScanMode,
-               size_t &MatchLen, StringMap<StringRef> &VariableTable,
-               FileCheckRequest &Req, std::vector<FileCheckDiag> *Diags) const;
+               size_t &MatchLen, FileCheckRequest &Req,
+               std::vector<FileCheckDiag> *Diags) const;
 
   bool CheckNext(const SourceMgr &SM, StringRef Buffer) const;
   bool CheckSame(const SourceMgr &SM, StringRef Buffer) const;
   bool CheckNot(const SourceMgr &SM, StringRef Buffer,
                 const std::vector<const FileCheckPattern *> &NotStrings,
-                StringMap<StringRef> &VariableTable,
                 const FileCheckRequest &Req,
                 std::vector<FileCheckDiag> *Diags) const;
   size_t CheckDag(const SourceMgr &SM, StringRef Buffer,
                   std::vector<const FileCheckPattern *> &NotStrings,
-                  StringMap<StringRef> &VariableTable,
                   const FileCheckRequest &Req,
                   std::vector<FileCheckDiag> *Diags) const;
 };
 
 /// FileCheck class takes the request and exposes various methods that
 /// use information from the request.
 class FileCheck {
   FileCheckRequest Req;
+  FileCheckPatternContext PatternContext;
 
 public:
   FileCheck(FileCheckRequest Req) : Req(Req) {}
 
   // Combines the check prefixes into a single regex so that we can efficiently
   // scan for any of the set.
   //
   // The semantics are that the longest-match wins which matches our regex

llvm/lib/Support/FileCheck.cpp

bool FileCheckPattern::EvaluateExpression(StringRef Expr, std::string &Value) const {
   return true;
 }
 
 /// Matches the pattern string against the input buffer \p Buffer
 ///
 /// This returns the position that is matched or npos if there is no match. If
 /// there is a match, the size of the matched string is returned in \p
 /// MatchLen.
 ///
-/// The \p VariableTable StringMap provides the current values of filecheck
-/// variables and is updated if this match defines new values.
-size_t FileCheckPattern::Match(StringRef Buffer, size_t &MatchLen,
-                      StringMap<StringRef> &VariableTable) const {
+/// The GlobalVariableTable StringMap in the FileCheckPatternContext class
+/// instance provides the current values of FileCheck variables and is updated
+/// if this match defines new values.
+size_t FileCheckPattern::match(StringRef Buffer, size_t &MatchLen) const {
   // If this is the EOF pattern, match it immediately.
   if (CheckTy == Check::CheckEOF) {
     MatchLen = 0;
     return Buffer.size();
   }
 
   // If this is a fixed string pattern, just match it now.
   if (!FixedStr.empty()) {
if (!VariableUses.empty()) {
     unsigned InsertOffset = 0;
     for (const auto &VariableUse : VariableUses) {
       std::string Value;
 
       if (VariableUse.first[0] == '@') {
         if (!EvaluateExpression(VariableUse.first, Value))
           return StringRef::npos;
       } else {
-        StringMap<StringRef>::iterator it =
-            VariableTable.find(VariableUse.first);
+        llvm::Optional<StringRef> ValueRef =
+            Context->getVarValue(VariableUse.first);
         // If the variable is undefined, return an error.
-        if (it == VariableTable.end())
+        if (!ValueRef)
           return StringRef::npos;
 
         // Look up the value and escape it so that we can put it into the regex.
-        Value += Regex::escape(it->second);
+        Value += Regex::escape(*ValueRef);
       }
 
       // Plop it into the regex at the adjusted offset.
       TmpStr.insert(TmpStr.begin() + VariableUse.second + InsertOffset,
                     Value.begin(), Value.end());
       InsertOffset += Value.size();
     }
 
     // Match the newly constructed regex.
     RegExToMatch = TmpStr;
   }
 
   SmallVector<StringRef, 4> MatchInfo;
   if (!Regex(RegExToMatch, Regex::Newline).match(Buffer, &MatchInfo))
     return StringRef::npos;
 
   // Successful regex match.
   assert(!MatchInfo.empty() && "Didn't get any match");
   StringRef FullMatch = MatchInfo[0];
 
   // If this defines any variables, remember their values.
   for (const auto &VariableDef : VariableDefs) {
     assert(VariableDef.second < MatchInfo.size() && "Internal paren error");
-    VariableTable[VariableDef.first] = MatchInfo[VariableDef.second];
+    Context->GlobalVariableTable[VariableDef.first] =
+        MatchInfo[VariableDef.second];
   }
 
   // Like CHECK-NEXT, CHECK-EMPTY's match range is considered to start after
   // the required preceding newline, which is consumed by the pattern in the
   // case of CHECK-EMPTY but not CHECK-NEXT.
   size_t MatchStartSkip = CheckTy == Check::CheckEmpty;
   MatchLen = FullMatch.size() - MatchStartSkip;
   return FullMatch.data() - Buffer.data() + MatchStartSkip;
 }
 
-
 /// Computes an arbitrary estimate for the quality of matching this pattern at
 /// the start of \p Buffer; a distance of zero should correspond to a perfect
 /// match.
-unsigned
-FileCheckPattern::ComputeMatchDistance(StringRef Buffer,
-                              const StringMap<StringRef> &VariableTable) const {
+unsigned FileCheckPattern::computeMatchDistance(StringRef Buffer) const {
   // Just compute the number of matching characters. For regular expressions, we
   // just compare against the regex itself and hope for the best.
   //
   // FIXME: One easy improvement here is have the regex lib generate a single
   // example regular expression which matches, and use that as the example
   // string.
   StringRef ExampleString(FixedStr);
   if (ExampleString.empty())
     ExampleString = RegExStr;
 
   // Only compare up to the first line in the buffer, or the string size.
   StringRef BufferPrefix = Buffer.substr(0, ExampleString.size());
   BufferPrefix = BufferPrefix.split('\n').first;
   return BufferPrefix.edit_distance(ExampleString);
 }
 
-void FileCheckPattern::PrintVariableUses(const SourceMgr &SM, StringRef Buffer,
-                                const StringMap<StringRef> &VariableTable,
+/// Print value of successful substitutions or name of undefined pattern
+/// variables preventing such a successful substitution.
+void FileCheckPattern::printVariableUses(const SourceMgr &SM, StringRef Buffer,
                                          SMRange MatchRange) const {
   // If this was a regular expression using variables, print the current
   // variable values.
   if (!VariableUses.empty()) {
     for (const auto &VariableUse : VariableUses) {
       SmallString<256> Msg;
       raw_svector_ostream OS(Msg);
       StringRef Var = VariableUse.first;
       if (Var[0] == '@') {
         std::string Value;
         if (EvaluateExpression(Var, Value)) {
           OS << "with expression \"";
           OS.write_escaped(Var) << "\" equal to \"";
           OS.write_escaped(Value) << "\"";
         } else {
           OS << "uses incorrect expression \"";
           OS.write_escaped(Var) << "\"";
         }
       } else {
-        StringMap<StringRef>::const_iterator it = VariableTable.find(Var);
+        llvm::Optional<StringRef> VarValue = Context->getVarValue(Var);
 
         // Check for undefined variable references.
-        if (it == VariableTable.end()) {
+        if (!VarValue) {
           OS << "uses undefined variable \"";
           OS.write_escaped(Var) << "\"";
         } else {
           OS << "with variable \"";
           OS.write_escaped(Var) << "\" equal to \"";
-          OS.write_escaped(it->second) << "\"";
+          OS.write_escaped(*VarValue) << "\"";
         }
       }
 
       if (MatchRange.isValid())
         SM.PrintMessage(MatchRange.Start, SourceMgr::DK_Note, OS.str(),
                         {MatchRange});
       else
         SM.PrintMessage(SMLoc::getFromPointer(Buffer.data()),
if (Diags) {
     if (AdjustPrevDiag)
       Diags->rbegin()->MatchTy = MatchTy;
     else
       Diags->emplace_back(SM, CheckTy, Loc, MatchTy, Range);
   }
   return Range;
 }
 
-void FileCheckPattern::PrintFuzzyMatch(
+void FileCheckPattern::printFuzzyMatch(
     const SourceMgr &SM, StringRef Buffer,
-    const StringMap<StringRef> &VariableTable,
     std::vector<FileCheckDiag> *Diags) const {
   // Attempt to find the closest/best fuzzy match.  Usually an error happens
   // because some string in the output didn't exactly match. In these cases, we
   // would like to show the user a best guess at what "should have" matched, to
   // save them having to actually check the input manually.
   size_t NumLinesForward = 0;
   size_t Best = StringRef::npos;
   double BestQuality = 0;
 
   // Use an arbitrary 4k limit on how far we will search.
   for (size_t i = 0, e = std::min(size_t(4096), Buffer.size()); i != e; ++i) {
     if (Buffer[i] == '\n')
       ++NumLinesForward;
 
     // Patterns have leading whitespace stripped, so skip whitespace when
     // looking for something which looks like a pattern.
     if (Buffer[i] == ' ' || Buffer[i] == '\t')
       continue;
 
     // Compute the "quality" of this match as an arbitrary combination of the
     // match distance and the number of lines skipped to get to this match.
-    unsigned Distance = ComputeMatchDistance(Buffer.substr(i), VariableTable);
+    unsigned Distance = computeMatchDistance(Buffer.substr(i));
     double Quality = Distance + (NumLinesForward / 100.);
 
     if (Quality < BestQuality || Best == StringRef::npos) {
       Best = i;
       BestQuality = Quality;
     }
   }
 
   // Print the "possible intended match here" line if we found something
   // reasonable and not equal to what we showed in the "scanning from here"
   // line.
   if (Best && Best != StringRef::npos && BestQuality < 50) {
     SMRange MatchRange =
         ProcessMatchResult(FileCheckDiag::MatchFuzzy, SM, getLoc(),
                            getCheckTy(), Buffer, Best, 0, Diags);
     SM.PrintMessage(MatchRange.Start, SourceMgr::DK_Note,
                     "possible intended match here");
 
     // FIXME: If we wanted to be really friendly we would show why the match
     // failed, as it can be hard to spot simple one character differences.
   }
 }
 
+/// Return the value of variable \p VarName or None if no such variable has
+/// been defined.
+llvm::Optional<StringRef>
+FileCheckPatternContext::getVarValue(StringRef VarName) {
+  auto VarIter = GlobalVariableTable.find(VarName);
+  if (VarIter == GlobalVariableTable.end())
+    return llvm::None;
+
+  return VarIter->second;
+}
+
 /// Finds the closing sequence of a regex variable usage or definition.
 ///
 /// \p Str has to point in the beginning of the definition (right after the
 /// opening sequence). Returns the offset of the closing sequence within Str,
 /// or npos if it was not found.
 size_t FileCheckPattern::FindRegexVarEnd(StringRef Str, SourceMgr &SM) {
   // Offset keeps track of the current offset within the input Str
   size_t Offset = 0;
FindFirstMatchingPrefix(Regex &PrefixRE, StringRef &Buffer,
   // We ran out of buffer while skipping partial matches so give up.
   return {StringRef(), StringRef()};
 }
 
 /// Read the check file, which specifies the sequence of expected strings.
 ///
 /// The strings are added to the CheckStrings vector. Returns true in case of
 /// an error, false otherwise.
-bool llvm::FileCheck::ReadCheckFile(SourceMgr &SM, StringRef Buffer,
-                                    Regex &PrefixRE,
-                                    std::vector<FileCheckString> &CheckStrings) {
+bool llvm::FileCheck::ReadCheckFile(
+    SourceMgr &SM, StringRef Buffer, Regex &PrefixRE,
+  PatternContext.defineCmdlineVariables(Req.GlobalDefines);
+
   std::vector<FileCheckPattern> ImplicitNegativeChecks;
   for (const auto &PatternString : Req.ImplicitCheckNot) {
     // Create a buffer with fake command line content in order to display the
     // command line option responsible for the specific implicit CHECK-NOT.
     std::string Prefix = "-implicit-check-not='";
     std::string Suffix = "'";
     std::unique_ptr<MemoryBuffer> CmdLine = MemoryBuffer::getMemBufferCopy(
         Prefix + PatternString + Suffix, "command line");
 
     StringRef PatternInBuffer =
         CmdLine->getBuffer().substr(Prefix.size(), PatternString.size());
     SM.AddNewSourceBuffer(std::move(CmdLine), SMLoc());
 
-    ImplicitNegativeChecks.push_back(FileCheckPattern(Check::CheckNot));
+    ImplicitNegativeChecks.push_back(
+        FileCheckPattern(Check::CheckNot, &PatternContext));
     ImplicitNegativeChecks.back().ParsePattern(PatternInBuffer,
                                                "IMPLICIT-CHECK", SM, 0, Req);
   }
 
   std::vector<FileCheckPattern> DagNotMatches = ImplicitNegativeChecks;
 
   // LineNumber keeps track of the line on which CheckPrefix instances are
   // found.
while (1) {
 
     // Scan ahead to the end of line.
     size_t EOL = Buffer.find_first_of("\n\r");
 
     // Remember the location of the start of the pattern, for diagnostics.
     SMLoc PatternLoc = SMLoc::getFromPointer(Buffer.data());
 
     // Parse the pattern.
-    FileCheckPattern P(CheckTy);
+    FileCheckPattern P(CheckTy, &PatternContext);
     if (P.ParsePattern(Buffer.substr(0, EOL), UsedPrefix, SM, LineNumber, Req))
       return true;
 
     // Verify that CHECK-LABEL lines do not define or use variables
     if ((CheckTy == Check::CheckLabel) && P.hasVariable()) {
       SM.PrintMessage(
           SMLoc::getFromPointer(UsedPrefixStart), SourceMgr::DK_Error,
           "found '" + UsedPrefix + "-LABEL:'"
while (1) {
     CheckStrings.emplace_back(P, UsedPrefix, PatternLoc);
     std::swap(DagNotMatches, CheckStrings.back().DagNotStrings);
     DagNotMatches = ImplicitNegativeChecks;
   }
 
   // Add an EOF pattern for any trailing CHECK-DAG/-NOTs, and use the first
   // prefix as a filler for the error message.
   if (!DagNotMatches.empty()) {
-    CheckStrings.emplace_back(FileCheckPattern(Check::CheckEOF), *Req.CheckPrefixes.begin(),
-                              SMLoc::getFromPointer(Buffer.data()));
+    CheckStrings.emplace_back(
+        FileCheckPattern(Check::CheckEOF, &PatternContext),
+        *Req.CheckPrefixes.begin(), SMLoc::getFromPointer(Buffer.data()));
     std::swap(DagNotMatches, CheckStrings.back().DagNotStrings);
   }
 
   if (CheckStrings.empty()) {
     errs() << "error: no check strings found with prefix"
            << (Req.CheckPrefixes.size() > 1 ? "es " : " ");
     auto I = Req.CheckPrefixes.begin();
     auto E = Req.CheckPrefixes.end();
     if (I != E) {
       errs() << "\'" << *I << ":'";
       ++I;
     }
     for (; I != E; ++I)
       errs() << ", \'" << *I << ":'";
 
     errs() << '\n';
     return true;
   }
 
   return false;
 }
 
 static void PrintMatch(bool ExpectedMatch, const SourceMgr &SM,
                        StringRef Prefix, SMLoc Loc, const FileCheckPattern &Pat,
-                       int MatchedCount, StringRef Buffer,
-                       StringMap<StringRef> &VariableTable, size_t MatchPos,
+                       int MatchedCount, StringRef Buffer, size_t MatchPos,
                        size_t MatchLen, const FileCheckRequest &Req,
                        std::vector<FileCheckDiag> *Diags) {
   bool PrintDiag = true;
   if (ExpectedMatch) {
     if (!Req.Verbose)
       return;
     if (!Req.VerboseVerbose && Pat.getCheckTy() == Check::CheckEOF)
       return;
std::string Message = formatv("{0}: {1} string found in input",
                             .str();
   if (Pat.getCount() > 1)
     Message += formatv(" ({0} out of {1})", MatchedCount, Pat.getCount()).str();
 
   SM.PrintMessage(
       Loc, ExpectedMatch ? SourceMgr::DK_Remark : SourceMgr::DK_Error, Message);
   SM.PrintMessage(MatchRange.Start, SourceMgr::DK_Note, "found here",
                   {MatchRange});
-  Pat.PrintVariableUses(SM, Buffer, VariableTable, MatchRange);
+  Pat.printVariableUses(SM, Buffer, MatchRange);
 }
 
 static void PrintMatch(bool ExpectedMatch, const SourceMgr &SM,
                        const FileCheckString &CheckStr, int MatchedCount,
-                       StringRef Buffer, StringMap<StringRef> &VariableTable,
-                       size_t MatchPos, size_t MatchLen, FileCheckRequest &Req,
+                       StringRef Buffer, size_t MatchPos, size_t MatchLen,
+                       FileCheckRequest &Req,
                        std::vector<FileCheckDiag> *Diags) {
   PrintMatch(ExpectedMatch, SM, CheckStr.Prefix, CheckStr.Loc, CheckStr.Pat,
-             MatchedCount, Buffer, VariableTable, MatchPos, MatchLen, Req,
-             Diags);
+             MatchedCount, Buffer, MatchPos, MatchLen, Req, Diags);
 }
 
 static void PrintNoMatch(bool ExpectedMatch, const SourceMgr &SM,
                          StringRef Prefix, SMLoc Loc,
                          const FileCheckPattern &Pat, int MatchedCount,
-                         StringRef Buffer, StringMap<StringRef> &VariableTable,
-                         bool VerboseVerbose,
+                         StringRef Buffer, bool VerboseVerbose,
                          std::vector<FileCheckDiag> *Diags) {
   bool PrintDiag = true;
   if (!ExpectedMatch) {
     if (!VerboseVerbose)
       return;
     // Due to their verbosity, we don't print verbose diagnostics here if we're
     // gathering them for a different rendering, but we always print other
     // diagnostics.
if (Pat.getCount() > 1)
     Message += formatv(" ({0} out of {1})", MatchedCount, Pat.getCount()).str();
   SM.PrintMessage(
       Loc, ExpectedMatch ? SourceMgr::DK_Error : SourceMgr::DK_Remark, Message);
 
   // Print the "scanning from here" line.
   SM.PrintMessage(SearchRange.Start, SourceMgr::DK_Note, "scanning from here");
 
   // Allow the pattern to print additional information if desired.
-  Pat.PrintVariableUses(SM, Buffer, VariableTable);
+  Pat.printVariableUses(SM, Buffer);
 
   if (ExpectedMatch)
-    Pat.PrintFuzzyMatch(SM, Buffer, VariableTable, Diags);
+    Pat.printFuzzyMatch(SM, Buffer, Diags);
 }
 
 static void PrintNoMatch(bool ExpectedMatch, const SourceMgr &SM,
                          const FileCheckString &CheckStr, int MatchedCount,
-                         StringRef Buffer, StringMap<StringRef> &VariableTable,
-                         bool VerboseVerbose,
+                         StringRef Buffer, bool VerboseVerbose,
                          std::vector<FileCheckDiag> *Diags) {
   PrintNoMatch(ExpectedMatch, SM, CheckStr.Prefix, CheckStr.Loc, CheckStr.Pat,
-               MatchedCount, Buffer, VariableTable, VerboseVerbose, Diags);
+               MatchedCount, Buffer, VerboseVerbose, Diags);
 }
 
 /// Count the number of newlines in the specified range.
 static unsigned CountNumNewlinesBetween(StringRef Range,
                                         const char *&FirstNewLine) {
   unsigned NumNewLines = 0;
   while (1) {
     // Scan for newline.
while (1) {
     if (NumNewLines == 1)
       FirstNewLine = Range.begin();
   }
 }
 
 /// Match check string and its "not strings" and/or "dag strings".
 size_t FileCheckString::Check(const SourceMgr &SM, StringRef Buffer,
                               bool IsLabelScanMode, size_t &MatchLen,
-                              StringMap<StringRef> &VariableTable,
                               FileCheckRequest &Req,
                               std::vector<FileCheckDiag> *Diags) const {
   size_t LastPos = 0;
   std::vector<const FileCheckPattern *> NotStrings;
 
   // IsLabelScanMode is true when we are scanning forward to find CHECK-LABEL
   // bounds; we have not processed variable definitions within the bounded block
   // yet so cannot handle any final CHECK-DAG yet; this is handled when going
   // over the block again (including the last CHECK-LABEL) in normal mode.
   if (!IsLabelScanMode) {
     // Match "dag strings" (with mixed "not strings" if any).
-    LastPos = CheckDag(SM, Buffer, NotStrings, VariableTable, Req, Diags);
+    LastPos = CheckDag(SM, Buffer, NotStrings, Req, Diags);
     if (LastPos == StringRef::npos)
       return StringRef::npos;
   }
 
   // Match itself from the last position after matching CHECK-DAG.
   size_t LastMatchEnd = LastPos;
   size_t FirstMatchPos = 0;
   // Go match the pattern Count times. Majority of patterns only match with
   // count 1 though.
   assert(Pat.getCount() != 0 && "pattern count can not be zero");
   for (int i = 1; i <= Pat.getCount(); i++) {
     StringRef MatchBuffer = Buffer.substr(LastMatchEnd);
     size_t CurrentMatchLen;
     // get a match at current start point
-    size_t MatchPos = Pat.Match(MatchBuffer, CurrentMatchLen, VariableTable);
+    size_t MatchPos = Pat.match(MatchBuffer, CurrentMatchLen);
     if (i == 1)
       FirstMatchPos = LastPos + MatchPos;
 
     // report
     if (MatchPos == StringRef::npos) {
-      PrintNoMatch(true, SM, *this, i, MatchBuffer, VariableTable,
-                   Req.VerboseVerbose, Diags);
+      PrintNoMatch(true, SM, *this, i, MatchBuffer, Req.VerboseVerbose, Diags);
       return StringRef::npos;
     }
-    PrintMatch(true, SM, *this, i, MatchBuffer, VariableTable, MatchPos,
-               CurrentMatchLen, Req, Diags);
+    PrintMatch(true, SM, *this, i, MatchBuffer, MatchPos, CurrentMatchLen, Req,
+               Diags);
 
     // move start point after the match
     LastMatchEnd += MatchPos + CurrentMatchLen;
   }
   // Full match len counts from first match pos.
   MatchLen = LastMatchEnd - FirstMatchPos;
 
   // Similar to the above, in "label-scan mode" we can't yet handle CHECK-NEXT
if (CheckSame(SM, SkippedRegion)) {
       ProcessMatchResult(FileCheckDiag::MatchFoundButWrongLine, SM, Loc,
                          Pat.getCheckTy(), MatchBuffer, MatchPos, MatchLen,
                          Diags, Req.Verbose);
       return StringRef::npos;
     }
 
     // If this match had "not strings", verify that they don't exist in the
     // skipped region.
-    if (CheckNot(SM, SkippedRegion, NotStrings, VariableTable, Req, Diags))
+    if (CheckNot(SM, SkippedRegion, NotStrings, Req, Diags))
       return StringRef::npos;
   }
 
   return FirstMatchPos;
 }
 
 /// Verify there is a single line in the given buffer.
 bool FileCheckString::CheckNext(const SourceMgr &SM, StringRef Buffer) const {
bool FileCheckString::CheckSame(const SourceMgr &SM, StringRef Buffer) const {
 
   return false;
 }
 
 /// Verify there's no "not strings" in the given buffer.
 bool FileCheckString::CheckNot(
     const SourceMgr &SM, StringRef Buffer,
     const std::vector<const FileCheckPattern *> &NotStrings,
-    StringMap<StringRef> &VariableTable, const FileCheckRequest &Req,
-    std::vector<FileCheckDiag> *Diags) const {
+    const FileCheckRequest &Req, std::vector<FileCheckDiag> *Diags) const {
   for (const FileCheckPattern *Pat : NotStrings) {
     assert((Pat->getCheckTy() == Check::CheckNot) && "Expect CHECK-NOT!");
 
     size_t MatchLen = 0;
-    size_t Pos = Pat->Match(Buffer, MatchLen, VariableTable);
+    size_t Pos = Pat->match(Buffer, MatchLen);
 
     if (Pos == StringRef::npos) {
       PrintNoMatch(false, SM, Prefix, Pat->getLoc(), *Pat, 1, Buffer,
-                   VariableTable, Req.VerboseVerbose, Diags);
+                   Req.VerboseVerbose, Diags);
       continue;
     }
 
-    PrintMatch(false, SM, Prefix, Pat->getLoc(), *Pat, 1, Buffer, VariableTable,
-               Pos, MatchLen, Req, Diags);
+    PrintMatch(false, SM, Prefix, Pat->getLoc(), *Pat, 1, Buffer, Pos, MatchLen,
+               Req, Diags);
 
     return true;
   }
 
   return false;
 }
 
 /// Match "dag strings" and their mixed "not strings".
 size_t
 FileCheckString::CheckDag(const SourceMgr &SM, StringRef Buffer,
                           std::vector<const FileCheckPattern *> &NotStrings,
-                          StringMap<StringRef> &VariableTable,
                           const FileCheckRequest &Req,
                           std::vector<FileCheckDiag> *Diags) const {
   if (DagNotStrings.empty())
     return 0;
 
   // The start of the search range.
   size_t StartPos = 0;
 
for (auto PatItr = DagNotStrings.begin(), PatEnd = DagNotStrings.end();
 
     // CHECK-DAG always matches from the start.
     size_t MatchLen = 0, MatchPos = StartPos;
 
     // Search for a match that doesn't overlap a previous match in this
     // CHECK-DAG group.
     for (auto MI = MatchRanges.begin(), ME = MatchRanges.end(); true; ++MI) {
       StringRef MatchBuffer = Buffer.substr(MatchPos);
-      size_t MatchPosBuf = Pat.Match(MatchBuffer, MatchLen, VariableTable);
+      size_t MatchPosBuf = Pat.match(MatchBuffer, MatchLen);
       // With a group of CHECK-DAGs, a single mismatching means the match on
       // that group of CHECK-DAGs fails immediately.
       if (MatchPosBuf == StringRef::npos) {
         PrintNoMatch(true, SM, Prefix, Pat.getLoc(), Pat, 1, MatchBuffer,
-                     VariableTable, Req.VerboseVerbose, Diags);
+                     Req.VerboseVerbose, Diags);
         return StringRef::npos;
       }
       // Re-calc it as the offset relative to the start of the original string.
       MatchPos += MatchPosBuf;
       if (Req.VerboseVerbose)
-        PrintMatch(true, SM, Prefix, Pat.getLoc(), Pat, 1, Buffer,
-                   VariableTable, MatchPos, MatchLen, Req, Diags);
+        PrintMatch(true, SM, Prefix, Pat.getLoc(), Pat, 1, Buffer, MatchPos,
+                   MatchLen, Req, Diags);
       MatchRange M{MatchPos, MatchPos + MatchLen};
       if (Req.AllowDeprecatedDagOverlap) {
         // We don't need to track all matches in this mode, so we just maintain
         // one match range that encompasses the current CHECK-DAG group's
         // matches.
         if (MatchRanges.empty())
           MatchRanges.insert(MatchRanges.end(), M);
         else {
for (auto MI = MatchRanges.begin(), ME = MatchRanges.end(); true; ++MI) {
                           "match discarded, overlaps earlier DAG match here",
                           {OldRange});
         } else
           Diags->rbegin()->MatchTy = FileCheckDiag::MatchFoundButDiscarded;
       }
       MatchPos = MI->End;
     }
     if (!Req.VerboseVerbose)
-      PrintMatch(true, SM, Prefix, Pat.getLoc(), Pat, 1, Buffer, VariableTable,
-                 MatchPos, MatchLen, Req, Diags);
+      PrintMatch(true, SM, Prefix, Pat.getLoc(), Pat, 1, Buffer, MatchPos,
+                 MatchLen, Req, Diags);
 
     // Handle the end of a CHECK-DAG group.
     if (std::next(PatItr) == PatEnd ||
         std::next(PatItr)->getCheckTy() == Check::CheckNot) {
       if (!NotStrings.empty()) {
         // If there are CHECK-NOTs between two CHECK-DAGs or from CHECK to
         // CHECK-DAG, verify that there are no 'not' strings occurred in that
         // region.
         StringRef SkippedRegion =
             Buffer.slice(StartPos, MatchRanges.begin()->Pos);
-        if (CheckNot(SM, SkippedRegion, NotStrings, VariableTable, Req, Diags))
+        if (CheckNot(SM, SkippedRegion, NotStrings, Req, Diags))
           return StringRef::npos;
         // Clear "not strings".
         NotStrings.clear();
       }
       // All subsequent CHECK-DAGs and CHECK-NOTs should be matched from the
       // end of this CHECK-DAG group's match range.
       StartPos = MatchRanges.rbegin()->End;
       // Don't waste time checking for (impossible) overlaps before that.
if (Prefix != Req.CheckPrefixes.front())
       PrefixRegexStr.push_back('|');
 
     PrefixRegexStr.append(Prefix);
   }
 
   return Regex(PrefixRegexStr);
 }
 
-// Remove local variables from \p VariableTable. Global variables
-// (start with '$') are preserved.
-static void ClearLocalVars(StringMap<StringRef> &VariableTable) {
-  SmallVector<StringRef, 16> LocalVars;
-  for (const auto &Var : VariableTable)
+/// Define variables from definitions given on the command line passed as a
+/// vector of VAR=VAL strings in \p CmdlineDefines.
+void FileCheckPatternContext::defineCmdlineVariables(
+    std::vector<std::string> &CmdlineDefines) {
+  assert(GlobalVariableTable.empty() &&
+         "Overriding defined variable with command-line variable definitions");
+  for (StringRef CmdlineDef : CmdlineDefines)
+    GlobalVariableTable.insert(CmdlineDef.split('='));
+}
+
+/// Undefine local variables (variables whose name does not start with a '$'
+/// sign), i.e. remove them from GlobalVariableTable.
+void FileCheckPatternContext::clearLocalVars(void) {
+  SmallVector<StringRef, 16> LocalPatternVars, LocalNumericVars;
+  for (const StringMapEntry<StringRef> &Var : GlobalVariableTable)
     if (Var.first()[0] != '$')
-      LocalVars.push_back(Var.first());
+      LocalPatternVars.push_back(Var.first());
 
-  for (const auto &Var : LocalVars)
-    VariableTable.erase(Var);
+  for (const auto &Var : LocalPatternVars)
+    GlobalVariableTable.erase(Var);
 }
 
 /// Check the input to FileCheck provided in the \p Buffer against the \p
 /// CheckStrings read from the check file.
 ///
 /// Returns false if the input fails to satisfy the checks.
 bool llvm::FileCheck::CheckInput(SourceMgr &SM, StringRef Buffer,
                                  ArrayRef<FileCheckString> CheckStrings,
                                  std::vector<FileCheckDiag> *Diags) {
   bool ChecksFailed = false;
 
-  /// VariableTable - This holds all the current filecheck variables.
-  StringMap<StringRef> VariableTable;
-
-  for (const auto& Def : Req.GlobalDefines)
-    VariableTable.insert(StringRef(Def).split('='));
-
   unsigned i = 0, j = 0, e = CheckStrings.size();
   while (true) {
     StringRef CheckRegion;
     if (j == e) {
       CheckRegion = Buffer;
     } else {
       const FileCheckString &CheckLabelStr = CheckStrings[j];
       if (CheckLabelStr.Pat.getCheckTy() != Check::CheckLabel) {
         ++j;
         continue;
       }
 
       // Scan to next CHECK-LABEL match, ignoring CHECK-NOT and CHECK-DAG
       size_t MatchLabelLen = 0;
-      size_t MatchLabelPos = CheckLabelStr.Check(
-          SM, Buffer, true, MatchLabelLen, VariableTable, Req, Diags);
+      size_t MatchLabelPos =
+          CheckLabelStr.Check(SM, Buffer, true, MatchLabelLen, Req, Diags);
       if (MatchLabelPos == StringRef::npos)
-        // Immediately bail of CHECK-LABEL fails, nothing else we can do.
+        // Immediately bail if CHECK-LABEL fails, nothing else we can do.
         return false;
 
       CheckRegion = Buffer.substr(0, MatchLabelPos + MatchLabelLen);
       Buffer = Buffer.substr(MatchLabelPos + MatchLabelLen);
       ++j;
     }
 
     if (Req.EnableVarScope)
-      ClearLocalVars(VariableTable);
+      PatternContext.clearLocalVars();
 
     for (; i != j; ++i) {
       const FileCheckString &CheckStr = CheckStrings[i];
 
       // Check each string within the scanned region, including a second check
       // of any final CHECK-LABEL (to verify CHECK-NOT and CHECK-DAG)
       size_t MatchLen = 0;
-      size_t MatchPos = CheckStr.Check(SM, CheckRegion, false, MatchLen,
-                                       VariableTable, Req, Diags);
+      size_t MatchPos =
+          CheckStr.Check(SM, CheckRegion, false, MatchLen, Req, Diags);
 
       if (MatchPos == StringRef::npos) {
         ChecksFailed = true;
         i = j;
         break;
       }
 
       CheckRegion = CheckRegion.substr(MatchPos + MatchLen);

llvm/unittests/Support/CMakeLists.txt

add_llvm_unittest(SupportTests
   DebugTest.cpp
   DebugCounterTest.cpp
   DJBTest.cpp
   EndianStreamTest.cpp
   EndianTest.cpp
   ErrnoTest.cpp
   ErrorOrTest.cpp
   ErrorTest.cpp
+  FileCheckTest.cpp
   FileOutputBufferTest.cpp
   FormatVariadicTest.cpp
   GlobPatternTest.cpp
   Host.cpp
   ItaniumManglingCanonicalizerTest.cpp
   JSONTest.cpp
   LEB128Test.cpp
   LineIteratorTest.cpp

llvm/unittests/Support/FileCheckTest.cpp

+//===- llvm/unittest/Support/FileCheckTest.cpp - FileCheck tests --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/FileCheck.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+namespace {
+
+class FileCheckTest : public ::testing::Test {};
+
+TEST_F(FileCheckTest, FileCheckContext) {
+  FileCheckPatternContext Cxt;
+  std::vector<std::string> GlobalDefines;
+
+  // Define local and global variables from command-line.
+  GlobalDefines.emplace_back(std::string("LocalVar=FOO"));
+  GlobalDefines.emplace_back(std::string("$GlobalVar=BAR"));
+  Cxt.defineCmdlineVariables(GlobalDefines);
+
+  // Check defined variables are present and undefined is absent.
+  StringRef LocalVarStr = "LocalVar";
+  StringRef GlobalVarStr = "$GlobalVar";
+  StringRef UnknownVarStr = "UnknownVar";
+  llvm::Optional<StringRef> LocalVar = Cxt.getVarValue(LocalVarStr);
+  llvm::Optional<StringRef> GlobalVar = Cxt.getVarValue(GlobalVarStr);
+  llvm::Optional<StringRef> UnknownVar = Cxt.getVarValue(UnknownVarStr);
+  EXPECT_TRUE(LocalVar);
+  EXPECT_EQ(*LocalVar, "FOO");
+  EXPECT_TRUE(GlobalVar);
+  EXPECT_EQ(*GlobalVar, "BAR");
+  EXPECT_FALSE(UnknownVar);
+
+  // Clear local variables and check they become absent.
+  Cxt.clearLocalVars();
+  LocalVar = Cxt.getVarValue(LocalVarStr);
+  GlobalVar = Cxt.getVarValue(GlobalVarStr);
+  EXPECT_FALSE(LocalVar);
+  EXPECT_TRUE(GlobalVar);
+}
+} // namespace