Add -vfsoverlay option for Clang-Tidy

[vladimir.plyashkun]

Summary

These changes introduce support for -vfsoverlay option in Clang-Tidy
The main reason for it:
In IDE integration we have two different pieces of source: editor and physical file.
File is placed in some project directory (let's call it "A")
We want to run clang-tidy exactly in this project directory "A", in order to get all .clang-tidy files in parent directories along with correct resolution for all relative include paths.
But in IDE, saving mechanism perfoms periodically.
So, to perform analysis of actual text on the fly, we need to save document on every modification (which is inefficient) or generate file in the temporary directory with actual text (it also leads to skipping .clang-tidy files and complicates include search paths resolution)
-vfsoverlay approach allows us to analyze source file in the project directory "A" but with actual data placed in another directory.

[vladimir.plyashkun]

This comment was removed by vladimir.plyashkun.

[ilya-biryukov]

IIUC, you want to pass an overlay so that clang-tidy will read the file opened in the IDE from a different directory?

Could you add -ivfsoverlay option to your compile command instead?

$ clang --help | grep vfs
-ivfsoverlay <value>    Overlay the virtual filesystem described by file over the real file system

[vladimir.plyashkun]

@ilya-biryukov
Yes, this is exactly what i need.
Unfortunately, -ivfsoverlay in the compile commands works for the compiler invocation, but it doesn't work for tooling.
E.g. this call:

clang-tidy -checks=* <file> -- -ivfsoverlay=<yaml_file>

has no effect.

[vladimir.plyashkun]

FYI, i've create revision to support ivfsoverlay option in Tooling: https://reviews.llvm.org/D41594

[ilya-biryukov]

In D41535#963859, @vladimir.plyashkun wrote:

Unfortunately, -ivfsoverlay in the compile commands works for the compiler invocation, but it doesn't work for tooling.

This looks like a bug in tooling, but let's wait for responses on the other therad.

It seems that clang-tidy will terribly broken with per-translation-unit overlays anyway. The problem is that clang-tidy seems to report errors and fixits in ErrorReporter class after running the tooling invocation, therefore it won't see any overlays that were local to each translation unit and may report wrong ranges, etc.
Probably global overlays (i.e. this patch) is probably the way to go.

clang-tidy/ClangTidy.cpp

528

Could we add a defaulted vfs::FileSystem BaseFS = getRealFileSystem() parameter to a constructor of ClangTool instead?
It seems like we're exposing implementation details of ClangTool here for no good reason.

[vladimir.plyashkun]

Second part here (provided default virtual filesystem argument to ClangTool constructor): https://reviews.llvm.org/D41947

[vladimir.plyashkun]

Friendly ping

[ilya-biryukov]

Sorry for the delay

clang-tidy/ClangTidy.cpp

92–94

We should pass BaseFS here as a parameter, similar to how we do that in ClangTool.
Piping it here from clangdTidyMain seems trivial.

93

DiagnosticsEngine& seems to be used to report errors in the source code, while the things we're seeing here are errors when initializing clang-tidy.
Could we move the code that creates vfs into ClangTidyMain.cpp and report errors directly into llvm::errs, similar to how we do that for other flags like list-checks, etc.

527

We should construct vfs before passing it to ClangTool and not modify it later.
I suggest creating BaseFS in clangTidyMain, see other comments too.

clang-tidy/ClangTidyOptions.h

114 ↗

(On Diff #129440)

I think we should avoid putting VfsOverlay into ClangTidyOptions. Is there any reason why having a flag in ClangTidyMain.cpp is not enough?

[vladimir.plyashkun]

Moved logic to ClangTidyMain

[ilya-biryukov]

clang-tidy/tool/ClangTidyMain.cpp

345

This code will print only the enum's integral value, we want to print an actual error message.
I don't think there's an easy way to reuse clang's diagnostics here, we should spell out the error message explicitly.

441

We should only create an overlay is -vfsoverlay was passed and use getRealFileSystem without wrappers in the common case.

444

Could we stop clang-tidy with an error if we couldn't create an overlay? That seems like a better option than silently running without an overlay when it was actually specified.

[vladimir.plyashkun]

Fixed code review remarks.

[ilya-biryukov]

Just a few more NITs and we're good to go

clang-tidy/tool/ClangTidyMain.cpp

348

We should probably print the error message (in addition to "not found", we could have permission errors, etc.)
Buffer.getError().message() would give a more informative message.

354

NIT: if (!FS) is equivalent, but less typing

439

NIT: remove braces around a single-statement return.

[vladimir.plyashkun]

Some more nits were fixed.

[ilya-biryukov]

This looks good, but we should add a test.
Should've noticed that before, sorry for the slowing this down a bit more. After the test is there, I'm happy to commit this change for you.

[vladimir.plyashkun]

In D41535#981928, @ilya-biryukov wrote:

This looks good, but we should add a test.
Should've noticed that before, sorry for the slowing this down a bit more. After the test is there, I'm happy to commit this change for you.

IIUC, it will be little bit difficult to test it, because whole logic placed in the ClangTidyMain.
All existing clang-tidy unit tests use direct calls of ToolInvocation which is doesn't know about vfsoverlay options.

[ilya-biryukov]

In D41535#983435, @vladimir.plyashkun wrote:

IIUC, it will be little bit difficult to test it, because whole logic placed in the ClangTidyMain.
All existing clang-tidy unit tests use direct calls of ToolInvocation which is doesn't know about vfsoverlay options.

We could test it with a lit test that runs clang-tidy directly and checks output using FileCheck. See clang-tools-extra/test/clang-tidy for clang-tidy tests using lit and clang/test/VFS for lit tests of vfs overlays in clang.

[vladimir.plyashkun]

Thanks, now all should be fine!

[ilya-biryukov]

Thanks for adding the test! LGTM, will submit it shortly.

clang-tidy/ClangTidy.h

 
 /// \brief Run a set of clang-tidy checks on a set of files.
 ///
 /// \param Profile if provided, it enables check profile collection in
 /// MatchFinder, and will contain the result of the profile.
 void runClangTidy(clang::tidy::ClangTidyContext &Context,
                   const tooling::CompilationDatabase &Compilations,
                   ArrayRef<std::string> InputFiles,
+                  llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS,
                   ProfileData *Profile = nullptr);
 
 // FIXME: This interface will need to be significantly extended to be useful.
 // FIXME: Implement confidence levels for displaying/fixing errors.
 //
 /// \brief Displays the found \p Errors to the users. If \p Fix is true, \p
 /// Errors containing fixes are automatically applied and reformatted. If no
 /// clang-format configuration file is found, the given \P FormatStyle is used.
 void handleErrors(ClangTidyContext &Context, bool Fix,
-                  unsigned &WarningsAsErrorsCount);
+                  unsigned &WarningsAsErrorsCount,
+                  llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS);
 
 /// \brief Serializes replacements into YAML and writes them to the specified
 /// output stream.
 void exportReplacements(StringRef MainFilePath,
                         const std::vector<ClangTidyError> &Errors,
                         raw_ostream &OS);
 
 } // end namespace tidy
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDY_H

clang-tidy/ClangTidy.cpp

public:
   bool supportsCrossFileDiagnostics() const override { return true; }
 
 private:
   ClangTidyContext &Context;
 };
 
 class ErrorReporter {
 public:
-  ErrorReporter(ClangTidyContext &Context, bool ApplyFixes)
-      : Files(FileSystemOptions()), DiagOpts(new DiagnosticOptions()),
+  ErrorReporter(ClangTidyContext &Context, bool ApplyFixes,
+                llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS)
+      : Files(FileSystemOptions(), BaseFS), DiagOpts(new DiagnosticOptions()),
         DiagPrinter(new TextDiagnosticPrinter(llvm::outs(), &*DiagOpts)),
         Diags(IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs), &*DiagOpts,
               DiagPrinter),
         SourceMgr(Diags, Files), Context(Context), ApplyFixes(ApplyFixes),
         TotalFixes(0), AppliedFixes(0), WarningsAsErrors(0) {
     DiagOpts->ShowColors = llvm::sys::Process::StandardOutHasColors();
     DiagPrinter->BeginSourceFile(LangOpts);
   }
clang::tidy::ClangTidyContext Context(
       llvm::make_unique<DefaultOptionsProvider>(ClangTidyGlobalOptions(),
                                                 Options));
   ClangTidyASTConsumerFactory Factory(Context);
   return Factory.getCheckOptions();
 }
 
 void runClangTidy(clang::tidy::ClangTidyContext &Context,
                   const CompilationDatabase &Compilations,
-                  ArrayRef<std::string> InputFiles, ProfileData *Profile) {
-  ClangTool Tool(Compilations, InputFiles);
+                  ArrayRef<std::string> InputFiles,
+                  llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS,
+                  ProfileData *Profile) {
+  ClangTool Tool(Compilations, InputFiles,
+                 std::make_shared<PCHContainerOperations>(), BaseFS);
 
   // Add extra arguments passed by the clang-tidy command-line.
   ArgumentsAdjuster PerFileExtraArgumentsInserter =
       [&Context](const CommandLineArguments &Args, StringRef Filename) {
         ClangTidyOptions Opts = Context.getOptionsForFile(Filename);
         CommandLineArguments AdjustedArgs = Args;
         if (Opts.ExtraArgsBefore) {
           auto I = AdjustedArgs.begin();
void runClangTidy(clang::tidy::ClangTidyContext &Context,
   Tool.appendArgumentsAdjuster(PluginArgumentsRemover);
   if (Profile)
     Context.setCheckProfileData(Profile);
 
   ClangTidyDiagnosticConsumer DiagConsumer(Context);
 
   Tool.setDiagnosticConsumer(&DiagConsumer);
 
   class ActionFactory : public FrontendActionFactory {
   public:
     ActionFactory(ClangTidyContext &Context) : ConsumerFactory(Context) {}
     FrontendAction *create() override { return new Action(&ConsumerFactory); }
 
   private:
     class Action : public ASTFrontendAction {
     public:
       Action(ClangTidyASTConsumerFactory *Factory) : Factory(Factory) {}
       std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &Compiler,
                                                      StringRef File) override {
         return Factory->CreateASTConsumer(Compiler, File);
       }
 
     private:
       ClangTidyASTConsumerFactory *Factory;
     };
 
     ClangTidyASTConsumerFactory ConsumerFactory;
   };
 
   ActionFactory Factory(Context);
   Tool.run(&Factory);
 }
 
 void handleErrors(ClangTidyContext &Context, bool Fix,
-                  unsigned &WarningsAsErrorsCount) {
-  ErrorReporter Reporter(Context, Fix);
+                  unsigned &WarningsAsErrorsCount,
+                  llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS) {
+  ErrorReporter Reporter(Context, Fix, BaseFS);
   vfs::FileSystem &FileSystem =
       *Reporter.getSourceManager().getFileManager().getVirtualFileSystem();
   auto InitialWorkingDir = FileSystem.getCurrentWorkingDirectory();
   if (!InitialWorkingDir)
     llvm::report_fatal_error("Cannot get current working path.");
 
   for (const ClangTidyError &Error : Context.getErrors()) {
     if (!Error.BuildDirectory.empty()) {

clang-tidy/tool/ClangTidyMain.cpp

 Run clang-tidy in quiet mode. This suppresses
 printing statistics about ignored warnings and
 warnings treated as errors if the respective
 options are specified.
 )"),
                            cl::init(false),
                            cl::cat(ClangTidyCategory));
 
+static cl::opt<std::string> VfsOverlay("vfsoverlay", cl::desc(R"(
+Overlay the virtual filesystem described by file
+over the real file system.
+)"),
+                                       cl::value_desc("filename"),
+                                       cl::cat(ClangTidyCategory));
+
 namespace clang {
 namespace tidy {
 
 static void printStats(const ClangTidyStats &Stats) {
   if (Stats.errorsIgnored()) {
     llvm::errs() << "Suppressed " << Stats.errorsIgnored() << " warnings (";
     StringRef Separator = "";
     if (Stats.ErrorsIgnoredNonUserCode) {
if (llvm::ErrorOr<ClangTidyOptions> ParsedConfig =
                    << ParsedConfig.getError().message() << "\n";
       return nullptr;
     }
   }
   return llvm::make_unique<FileOptionsProvider>(GlobalOptions, DefaultOptions,
                                                 OverrideOptions);
 }
 
+llvm::IntrusiveRefCntPtr<vfs::FileSystem>
+getVfsOverlayFromFile(const std::string &OverlayFile) {
+  llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> OverlayFS(
+      new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
+  llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buffer =
+      OverlayFS->getBufferForFile(OverlayFile);
+  if (!Buffer) {
+    llvm::errs() << "Can't load virtual filesystem overlay file '"
+                 << OverlayFile << "': " << Buffer.getError().message()
+                 << ".\n";
+    return nullptr;
+  }
+
+  IntrusiveRefCntPtr<vfs::FileSystem> FS = vfs::getVFSFromYAML(
+      std::move(Buffer.get()), /*DiagHandler*/ nullptr, OverlayFile);
+  if (!FS) {
+    llvm::errs() << "Error: invalid virtual filesystem overlay file '"
+                 << OverlayFile << "'.\n";
+    return nullptr;
+  }
+  OverlayFS->pushOverlay(FS);
+  return OverlayFS;
+}
+
 static int clangTidyMain(int argc, const char **argv) {
   CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory,
                                     cl::ZeroOrMore);
 
   auto OwningOptionsProvider = createOptionsProvider();
   auto *OptionsProvider = OwningOptionsProvider.get();
   if (!OptionsProvider)
     return 1;
if (EnabledChecks.empty()) {
     return 0;
   }
 
   if (PathList.empty()) {
     llvm::errs() << "Error: no input files specified.\n";
     llvm::cl::PrintHelpMessage(/*Hidden=*/false, /*Categorized=*/true);
     return 0;
   }
+  llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS(
+      VfsOverlay.empty() ? vfs::getRealFileSystem()
+                         : getVfsOverlayFromFile(VfsOverlay));
+  if (!BaseFS)
+    return 1;
 
   ProfileData Profile;
 
   llvm::InitializeAllTargetInfos();
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllAsmParsers();
 
   ClangTidyContext Context(std::move(OwningOptionsProvider));
-  runClangTidy(Context, OptionsParser.getCompilations(), PathList,
+  runClangTidy(Context, OptionsParser.getCompilations(), PathList, BaseFS,
                EnableCheckProfile ? &Profile : nullptr);
   ArrayRef<ClangTidyError> Errors = Context.getErrors();
   bool FoundErrors =
       std::find_if(Errors.begin(), Errors.end(), [](const ClangTidyError &E) {
         return E.DiagLevel == ClangTidyError::Error;
       }) != Errors.end();
 
   const bool DisableFixes = Fix && FoundErrors && !FixErrors;
 
   unsigned WErrorCount = 0;
 
   // -fix-errors implies -fix.
-  handleErrors(Context, (FixErrors || Fix) && !DisableFixes, WErrorCount);
+  handleErrors(Context, (FixErrors || Fix) && !DisableFixes, WErrorCount,
+               BaseFS);
 
   if (!ExportFixes.empty() && !Errors.empty()) {
     std::error_code EC;
     llvm::raw_fd_ostream OS(ExportFixes, EC, llvm::sys::fs::F_None);
     if (EC) {
       llvm::errs() << "Error opening output file: " << EC.message() << '\n';
       return 1;
     }

test/clang-tidy/Inputs/vfsoverlay/actual_header.h

+struct X {};

test/clang-tidy/Inputs/vfsoverlay/vfsoverlay.yaml

+{
+  'version': 0,
+  'roots': [
+    { 'name': 'OUT_DIR', 'type': 'directory',
+      'contents': [
+        { 'name': 'not_real.h', 'type': 'file',
+          'external-contents': 'INPUT_DIR/actual_header.h'
+        }
+      ]
+    }
+  ]
+}

test/clang-tidy/vfsoverlay.cpp

+// RUN: sed -e "s:INPUT_DIR:%S/Inputs/vfsoverlay:g" -e "s:OUT_DIR:%t:g" %S/Inputs/vfsoverlay/vfsoverlay.yaml > %t.yaml
+// RUN: clang-tidy %s -checks='-*,modernize-use-nullptr' -vfsoverlay %t.yaml -- -I %t | FileCheck %s
+// REQUIRES: shell
+
+#include "not_real.h"
+
+X *ptr = 0;
+// CHECK: warning: use nullptr [modernize-use-nullptr]