FileCheck Enhancement - Including files

[eklepilkina]

Summary

This patch adds new directive CHECK-INCLUDE - directive to include other file with checks to another.

[vsk]

Looks super useful :). I have some initial comments -- please clang-format and upload a diff with more context and I'll take a closer look.

utils/FileCheck/FileCheck.cpp

38

This seems a bit arbitrary. Why not use a DFS? E.g;

containsIncludeCycle(MainFile, IncludeFile) {
  if (Includes[IncludeFile].contains(MainFile)) return true
  for (Include in Includes[IncludeFile]) if (containsIncludeCycle(Include, MainFile)) return true
  return false
}

990

const std::string &Directory to avoid a copy.

991

Use llvm::sys::path::append. Also note that in this line, you're taking a reference to a stack temporary (the result of the std::string concatenation should be destroyed after this line). This looks like a use-after-free.

1003

Is there some way to avoid moving the code into this else branch? If not that's OK. Otherwise, it would really help to keep unmodified code out of this diff (makes it easier to inspect the diff).

[eklepilkina]

utils/FileCheck/FileCheck.cpp

38

May be I didn't understand something, but as I understood next situation will fail. Am I wrong?

1->2;
1->3;
3->3;
2->4;
2->5;
4->1;
5->2;

1003

I understood that it'll be easier to merge, but I don't see correct way to avoid branching.

utils/FileCheck/FileCheck.cpp

 #include "llvm/Support/Signals.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/Path.h"
 #include <algorithm>
 #include <cctype>
 #include <map>
 #include <string>
 #include <system_error>
 #include <vector>
+
+#define MAX_INCLUDING_DEPTH 20 // maximum depth for including files.
+
 using namespace llvm;
 
 static cl::opt<std::string>

              "Allows leading and trailing whitespace if --strict-whitespace\n"
              "is not also passed."));
 
+static cl::list<std::string> IncludedDirectories(
+    "I",
+    cl::desc("Add directories for searching included files in CHECK-INCLUDE"));
+
 typedef cl::list<std::string>::const_iterator prefix_iterator;
 
 //===----------------------------------------------------------------------===//

     CheckNot,
     CheckDAG,
     CheckLabel,
+    CheckInclude,
 
     /// MatchEOF - When set, this pattern only matches the end of file. This is
     /// used for trailing CHECK-NOTs.

   /// getLoc - Return the location in source code.
   SMLoc getLoc() const { return PatternLoc; }
 
+  StringRef getFixedString() const { return FixedStr; }
+
   /// ParsePattern - Parse the given string into the Pattern. Prefix provides
   /// which prefix is being matched, SM provides the SourceMgr used for error
   /// reports, and LineNumber is the line number in the input file from which

   case Check::CheckLabel:
     return sizeof("-LABEL:") - 1;
 
+  case Check::CheckInclude:
+    return sizeof("-INCLUDE:") - 1;
+
   case Check::CheckEOF:
     llvm_unreachable("Should not be using EOF size");
   }

   if (Rest.startswith("LABEL:"))
     return Check::CheckLabel;
 
+  if (Rest.startswith("INCLUDE:"))
+    return Check::CheckInclude;
+
   // You can't combine -NOT with another suffix.
   if (Rest.startswith("DAG-NOT:") || Rest.startswith("NOT-DAG:") ||
       Rest.startswith("NEXT-NOT:") || Rest.startswith("NOT-NEXT:") ||

 /// expected strings.  The strings are added to the CheckStrings vector.
 /// Returns true in case of an error, false otherwise.
 static bool ReadCheckFile(SourceMgr &SM,
-                          std::vector<CheckString> &CheckStrings) {
+                          std::vector<CheckString> &CheckStrings,
+                          std::string Filename = CheckFilename, 
+                          int IncludingLevel = 0) {
   ErrorOr<std::unique_ptr<MemoryBuffer>> FileOrErr =
-      MemoryBuffer::getFileOrSTDIN(CheckFilename);
+      MemoryBuffer::getFileOrSTDIN(Filename);
   if (std::error_code EC = FileOrErr.getError()) {
-    errs() << "Could not open check file '" << CheckFilename
+    errs() << "Could not open check file '" << Filename
            << "': " << EC.message() << '\n';
     return true;
   }

     if (P.ParsePattern(Buffer.substr(0, EOL), UsedPrefix, SM, LineNumber))
       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;
-    }
+    // if included another file
+    if (CheckTy == Check::CheckInclude) {
+      // Check for escaping infinite including
+      if (IncludingLevel > MAX_INCLUDING_DEPTH) {
+        SM.PrintMessage(SMLoc::getFromPointer(Buffer.data()),
+                        SourceMgr::DK_Error,
+                        "Maximum including level!");
+        return true;
+      }
+      // Get included file name.
+      StringRef IncludedFileName = P.getFixedString();
+
+      std::vector<CheckString> IncludedCheckStrings;
+      // Try find file in included directories.
+      ErrorOr<std::unique_ptr<MemoryBuffer>> FileOrErr =
+        MemoryBuffer::getFileOrSTDIN(StringRef(IncludedFileName));
+      if (FileOrErr.getError()) {
+        for (std::string Directory : IncludedDirectories) {
+          StringRef FullIncludedFileName = StringRef(Directory + '/' + IncludedFileName.str());
+          FileOrErr = MemoryBuffer::getFileOrSTDIN(FullIncludedFileName);
+          if (!FileOrErr.getError()) {
+            IncludedFileName = FullIncludedFileName;
+            break;
+          }
+        }
+      }
+      
+      // Read it and get check strings from it.
+      ReadCheckFile(SM, IncludedCheckStrings, IncludedFileName, IncludingLevel+1);
+      CheckStrings.insert(CheckStrings.end(), IncludedCheckStrings.begin(), IncludedCheckStrings.end());
+    } else {
 
-    Buffer = Buffer.substr(EOL);
+      // 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;
+      }
 
-    // Verify that CHECK-NEXT lines have at least one CHECK line before them.
-    if ((CheckTy == Check::CheckNext || CheckTy == Check::CheckSame) &&
-        CheckStrings.empty()) {
-      StringRef Type = CheckTy == Check::CheckNext ? "NEXT" : "SAME";
-      SM.PrintMessage(SMLoc::getFromPointer(UsedPrefixStart),
-                      SourceMgr::DK_Error,
-                      "found '" + UsedPrefix + "-" + Type + "' without previous '"
-                      + UsedPrefix + ": line");
-      return true;
-    }
+      Buffer = Buffer.substr(EOL);
 
-    // Handle CHECK-DAG/-NOT.
-    if (CheckTy == Check::CheckDAG || CheckTy == Check::CheckNot) {
-      DagNotMatches.push_back(P);
-      continue;
-    }
+      // Verify that CHECK-NEXT lines have at least one CHECK line before them.
+      if ((CheckTy == Check::CheckNext || CheckTy == Check::CheckSame) &&
+          CheckStrings.empty()) {
+        StringRef Type = CheckTy == Check::CheckNext ? "NEXT" : "SAME";
+        SM.PrintMessage(SMLoc::getFromPointer(UsedPrefixStart),
+                        SourceMgr::DK_Error,
+                        "found '" + UsedPrefix + "-" + Type + "' without previous '"
+                        + UsedPrefix + ": line");
+        return true;
+      }
 
-    // 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);
-    DagNotMatches = ImplicitNegativeChecks;
+      // 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);
+      DagNotMatches = ImplicitNegativeChecks;
+    }
   }
 
   // Add an EOF pattern for any trailing CHECK-DAG/-NOTs, and use the first