[FileCheck] Remove implementation types from API

[thopre]

Summary

Remove use of FileCheckPatternContext and FileCheckString concrete types
from FileCheck API to allow moving it and the other implementation only
only declarations into a private header file.

[jhenderson]

LGTM, with a couple of minor nits.

llvm/include/llvm/Support/FileCheck.h

732

Why not C++17?

736–737

Should this be explicit?

[thopre]

Address feedback

llvm/include/llvm/Support/FileCheck.h

size_t CheckDag(const SourceMgr &SM, StringRef Buffer,
                   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;
+  std::unique_ptr<FileCheckPatternContext> PatternContext;
+  // C++17 TODO: make this a plain std::vector.
+  std::unique_ptr<std::vector<FileCheckString>> CheckStrings;
 
 public:
-  FileCheck(FileCheckRequest Req) : Req(Req) {}
+  explicit FileCheck(FileCheckRequest Req);
+  ~FileCheck();
 
   // 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
   // library.
   Regex buildCheckPrefixRegex();
 
   /// Reads the check file from \p Buffer and records the expected strings it
-  /// contains in the \p CheckStrings vector. Errors are reported against
-  /// \p SM.
+  /// contains. Errors are reported against \p SM.
   ///
   /// Only expected strings whose prefix is one of those listed in \p PrefixRE
   /// are recorded. \returns true in case of an error, false otherwise.
-  bool ReadCheckFile(SourceMgr &SM, StringRef Buffer, Regex &PrefixRE,
-                     std::vector<FileCheckString> &CheckStrings);
+  bool readCheckFile(SourceMgr &SM, StringRef Buffer, Regex &PrefixRE);
 
   bool ValidateCheckPrefixes();
 
   /// Canonicalizes whitespaces in the file. Line endings are replaced with
   /// UNIX-style '\n'.
   StringRef CanonicalizeFile(MemoryBuffer &MB,
                              SmallVectorImpl<char> &OutputBuffer);
 
   /// Checks the input to FileCheck provided in the \p Buffer against the
-  /// \p CheckStrings read from the check file and record diagnostics emitted
+  /// expected strings read from the check file and record diagnostics emitted
   /// in \p Diags. Errors are recorded against \p SM.
   ///
   /// \returns false if the input fails to satisfy the checks.
-  bool CheckInput(SourceMgr &SM, StringRef Buffer,
-                  ArrayRef<FileCheckString> CheckStrings,
+  bool checkInput(SourceMgr &SM, StringRef Buffer,
                   std::vector<FileCheckDiag> *Diags = nullptr);
 };
 } // namespace llvm
 #endif

llvm/lib/Support/FileCheck.cpp

 
 void FileCheckPatternContext::createLineVariable() {
   assert(!LineVariable && "@LINE pseudo numeric variable already created");
   StringRef LineName = "@LINE";
   LineVariable = makeNumericVariable(LineName);
   GlobalNumericVariableTable[LineName] = LineVariable;
 }
 
-bool FileCheck::ReadCheckFile(SourceMgr &SM, StringRef Buffer, Regex &PrefixRE,
-                              std::vector<FileCheckString> &CheckStrings) {
+FileCheck::FileCheck(FileCheckRequest Req)
+    : Req(Req), PatternContext(std::make_unique<FileCheckPatternContext>()),
+      CheckStrings(std::make_unique<std::vector<FileCheckString>>()) {}
+
+FileCheck::~FileCheck() = default;
+
+bool FileCheck::readCheckFile(SourceMgr &SM, StringRef Buffer,
+                              Regex &PrefixRE) {
   Error DefineError =
-      PatternContext.defineCmdlineVariables(Req.GlobalDefines, SM);
+      PatternContext->defineCmdlineVariables(Req.GlobalDefines, SM);
   if (DefineError) {
     logAllUnhandledErrors(std::move(DefineError), errs());
     return true;
   }
 
-  PatternContext.createLineVariable();
+  PatternContext->createLineVariable();
 
   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, &PatternContext));
+        FileCheckPattern(Check::CheckNot, PatternContext.get()));
     ImplicitNegativeChecks.back().parsePattern(PatternInBuffer,
                                                "IMPLICIT-CHECK", SM, 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, &PatternContext, LineNumber);
+    FileCheckPattern P(CheckTy, PatternContext.get(), LineNumber);
     if (P.parsePattern(Buffer.substr(0, EOL), UsedPrefix, SM, 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:'"
                                    " with variable definition or use");
       return true;
     }
 
     Buffer = Buffer.substr(EOL);
 
     // Verify that CHECK-NEXT/SAME/EMPTY lines have at least one CHECK line before them.
     if ((CheckTy == Check::CheckNext || CheckTy == Check::CheckSame ||
          CheckTy == Check::CheckEmpty) &&
-        CheckStrings.empty()) {
+        CheckStrings->empty()) {
       StringRef Type = CheckTy == Check::CheckNext
                            ? "NEXT"
                            : CheckTy == Check::CheckEmpty ? "EMPTY" : "SAME";
       SM.PrintMessage(SMLoc::getFromPointer(UsedPrefixStart),
                       SourceMgr::DK_Error,
                       "found '" + UsedPrefix + "-" + Type +
                           "' without previous '" + UsedPrefix + ": line");
       return true;
     }
 
     // Handle CHECK-DAG/-NOT.
     if (CheckTy == Check::CheckDAG || CheckTy == Check::CheckNot) {
       DagNotMatches.push_back(P);
       continue;
     }
 
     // Okay, add the string we captured to the output vector and move on.
-    CheckStrings.emplace_back(P, UsedPrefix, PatternLoc);
-    std::swap(DagNotMatches, CheckStrings.back().DagNotStrings);
+    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, &PatternContext, LineNumber + 1),
+    CheckStrings->emplace_back(
+        FileCheckPattern(Check::CheckEOF, PatternContext.get(), LineNumber + 1),
         *Req.CheckPrefixes.begin(), SMLoc::getFromPointer(Buffer.data()));
-    std::swap(DagNotMatches, CheckStrings.back().DagNotStrings);
+    std::swap(DagNotMatches, CheckStrings->back().DagNotStrings);
   }
 
-  if (CheckStrings.empty()) {
+  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 (const auto &Var : GlobalNumericVariableTable)
     }
 
   for (const auto &Var : LocalPatternVars)
     GlobalVariableTable.erase(Var);
   for (const auto &Var : LocalNumericVars)
     GlobalNumericVariableTable.erase(Var);
 }
 
-bool FileCheck::CheckInput(SourceMgr &SM, StringRef Buffer,
-                           ArrayRef<FileCheckString> CheckStrings,
+bool FileCheck::checkInput(SourceMgr &SM, StringRef Buffer,
                            std::vector<FileCheckDiag> *Diags) {
   bool ChecksFailed = false;
 
-  unsigned i = 0, j = 0, e = CheckStrings.size();
+  unsigned i = 0, j = 0, e = CheckStrings->size();
   while (true) {
     StringRef CheckRegion;
     if (j == e) {
       CheckRegion = Buffer;
     } else {
-      const FileCheckString &CheckLabelStr = CheckStrings[j];
+      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, Req, Diags);
       if (MatchLabelPos == StringRef::npos)
         // 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;
     }
 
     // Do not clear the first region as it's the one before the first
     // CHECK-LABEL and it would clear variables defined on the command-line
     // before they get used.
     if (i != 0 && Req.EnableVarScope)
-      PatternContext.clearLocalVars();
+      PatternContext->clearLocalVars();
 
     for (; i != j; ++i) {
-      const FileCheckString &CheckStr = CheckStrings[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, Req, Diags);
 
       if (MatchPos == StringRef::npos) {

llvm/unittests/CodeGen/GlobalISel/GISelMITest.h

static inline bool CheckMachineFunction(const MachineFunction &MF,
   FileCheckRequest Req;
   FileCheck FC(Req);
   StringRef CheckFileText =
       FC.CanonicalizeFile(*CheckBuf.get(), CheckFileBuffer);
   SourceMgr SM;
   SM.AddNewSourceBuffer(MemoryBuffer::getMemBuffer(CheckFileText, "CheckFile"),
                         SMLoc());
   Regex PrefixRE = FC.buildCheckPrefixRegex();
-  std::vector<FileCheckString> CheckStrings;
-  if (FC.ReadCheckFile(SM, CheckFileText, PrefixRE, CheckStrings))
+  if (FC.readCheckFile(SM, CheckFileText, PrefixRE))
     return false;
 
   auto OutBuffer = OutputBuf->getBuffer();
   SM.AddNewSourceBuffer(std::move(OutputBuf), SMLoc());
-  return FC.CheckInput(SM, OutBuffer, CheckStrings);
+  return FC.checkInput(SM, OutBuffer);
 }
 #endif

llvm/utils/FileCheck/FileCheck.cpp

int main(int argc, char **argv) {
 
   SmallString<4096> CheckFileBuffer;
   StringRef CheckFileText = FC.CanonicalizeFile(CheckFile, CheckFileBuffer);
 
   SM.AddNewSourceBuffer(MemoryBuffer::getMemBuffer(
                             CheckFileText, CheckFile.getBufferIdentifier()),
                         SMLoc());
 
-  std::vector<FileCheckString> CheckStrings;
-  if (FC.ReadCheckFile(SM, CheckFileText, PrefixRE, CheckStrings))
+  if (FC.readCheckFile(SM, CheckFileText, PrefixRE))
     return 2;
 
   // Open the file to check and add it to SourceMgr.
   ErrorOr<std::unique_ptr<MemoryBuffer>> InputFileOrErr =
       MemoryBuffer::getFileOrSTDIN(InputFilename);
   if (std::error_code EC = InputFileOrErr.getError()) {
     errs() << "Could not open input file '" << InputFilename
            << "': " << EC.message() << '\n';
int main(int argc, char **argv) {
   SM.AddNewSourceBuffer(MemoryBuffer::getMemBuffer(
                             InputFileText, InputFile.getBufferIdentifier()),
                         SMLoc());
 
   if (DumpInput == DumpInputDefault)
     DumpInput = DumpInputOnFailure ? DumpInputFail : DumpInputNever;
 
   std::vector<FileCheckDiag> Diags;
-  int ExitCode = FC.CheckInput(SM, InputFileText, CheckStrings,
+  int ExitCode = FC.checkInput(SM, InputFileText,
                                DumpInput == DumpInputNever ? nullptr : &Diags)
                      ? EXIT_SUCCESS
                      : 1;
   if (DumpInput == DumpInputAlways ||
       (ExitCode == 1 && DumpInput == DumpInputFail)) {
     errs() << "\n"
            << "Input file: "
            << (InputFilename == "-" ? "<stdin>" : InputFilename.getValue())