[clang-tidy] Expand modular headers for PPCallbacks

[alexfh]

Summary

Add a way to expand modular headers for PPCallbacks. Checks can opt-in for this
expansion by overriding the new registerPPCallbacks virtual method and
registering their PPCallbacks in the preprocessor created for this specific
purpose.

Use module expansion in the readability-identifier-naming check

[alexfh]

• Fix a typo in the comment. Rearrange paragraphs to make the comment more readable.

[alexfh]

• Fix naming.

[gribozavr]

clang-tools-extra/clang-tidy/ClangTidy.h

156

Please document that ModuleExpanderPP also provides events from the main file (until I read the implementation in the check, I thought the check should subscribe to both preprocessors).

clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp

20

Three slashes in doc comments (everywhere in the patch).

21

"Records that a given file entry is needed for replaying callbacks."

"addNecessaryFile"?

24

"recordFileContent"?

25

const & for File and ContentCache ?

39

"remove the file from the set of necessary files..."

72

of => off

However I don't see value in a comment that repeats the source code.

116

Recorder->addFileToRecord(IF.getFile())

The intermediate variable doesn't add anything...

172

What's target_callbacks_? It is only mentioned in comments.

clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h

10

This looks like a documentation comment for the class itself, I think it should be moved there.

44

"Returns the preprocessor that provides callbacks for contents of modular headers.

This preprocessor is separate from the one used by the rest of the compiler."

clang-tools-extra/test/clang-tidy/expand-modular-headers-ppcallbacks.cpp

32

Please also add a check for a diagnostic coming from the main file.

[gribozavr]

clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h

33

replays => re-runs

[alexfh]

• Addressed review comments. Parse code to the end (seems right to do this, but no specific test that would fail otherwise).

clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp

25

Done for ContentCache. The File is stored into a DenseSet, thus a pointer makes sense here.

172

It's an artifact of an older version. Removed.

[alexfh]

Thank you for the review! I hope I covered all the points.

[gribozavr]

clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp

167

Haha, so the test that I asked to add did catch a bug -- just not the one I expected :)

clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h

45

"... callbacks for the whole translation unit, including the main file, textual headers, and modular headers."

Sorry, I wrote the comment before I fully understood what this preprocessor does.

[alexfh]

clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp

167

Nope, the test passes without this. I'm not even sure this override is changing any observable behavior of PPCallbacks. I hope to find this out by running other checks on real code with modules enabled.

clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h

45

Indeed. Thanks for the text.

[alexfh]

• - Update a comment according to the review comments.

[uabelho]

Hi,

I noticed that with this commit I get a whole bunch (~40) of warnings like the below when compiling with gcc 7.4:

[10/16] Building CXX object tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/clangTidyUtils.dir/HeaderGuard.cpp.o
In file included from ../tools/clang/tools/extra/clang-tidy/utils/HeaderGuard.h:12:0,
                 from ../tools/clang/tools/extra/clang-tidy/utils/HeaderGuard.cpp:9:
../tools/clang/tools/extra/clang-tidy/utils/../ClangTidy.h:161:16: warning: 'virtual void clang::tidy::ClangTidyCheck::registerPPCallbacks(const clang::SourceManager&, clang::Preprocessor*, clang::Preprocessor*)' was hidden [-Woverloaded-virtual]
   virtual void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
                ^~~~~~~~~~~~~~~~~~~
In file included from ../tools/clang/tools/extra/clang-tidy/utils/HeaderGuard.cpp:9:0:
../tools/clang/tools/extra/clang-tidy/utils/HeaderGuard.h:35:8: warning:   by 'virtual void clang::tidy::utils::HeaderGuardCheck::registerPPCallbacks(clang::CompilerInstance&)' [-Woverloaded-virtual]
   void registerPPCallbacks(CompilerInstance &Compiler) override;
        ^~~~~~~~~~~~~~~~~~~

One of the registerPPCallbacks methods has the following comment

/// DEPRECATED: Use the other overload.
virtual void registerPPCallbacks(CompilerInstance &Compiler) {}

so I suppose it will be removed at some point?

I'm not super familiar with the warning, but there is at least one discussion about it here:
https://stackoverflow.com/questions/9995421/gcc-woverloaded-virtual-warnings

[PiotrZSL]

Looks like this change is responsible for https://github.com/llvm/llvm-project/issues/59807 and https://github.com/llvm/llvm-project/issues/62337 issues.

clang-tools-extra/clang-tidy/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 add_clang_library(clangTidy
   ClangTidy.cpp
   ClangTidyModule.cpp
   ClangTidyDiagnosticConsumer.cpp
   ClangTidyOptions.cpp
   ClangTidyProfiling.cpp
+  ExpandModularHeadersPPCallbacks.cpp
 
   DEPENDS
   ClangSACheckers
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic

clang-tools-extra/clang-tidy/ClangTidy.h

public:
   }
 
   /// \brief Override this to register ``PPCallbacks`` with ``Compiler``.
   ///
   /// This should be used for clang-tidy checks that analyze preprocessor-
   /// dependent properties, e.g. the order of include directives.
   virtual void registerPPCallbacks(CompilerInstance &Compiler) {}
 
+  /// \brief Override this to register ``PPCallbacks`` in the preprocessor.
+  ///
+  /// There are two Preprocessors to choose frome: the real one and the one that
+  /// generates callbacks for all transitively included headers even when
+  /// compiling with modules. When compiling without modules, both pointers
+  /// point to the real preprocessor.
+  ///
+  /// This should be used for clang-tidy checks that analyze preprocessor-
+  /// dependent properties, e.g. include directives and macro definitions.
+  virtual void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+                                   Preprocessor *ModuleExpanderPP) {}
+
   /// \brief Override this to register AST matchers with \p Finder.
   ///
   /// This should be used by clang-tidy checks that analyze code properties that
   /// dependent on AST knowledge.
   ///
   /// You can register as many matchers as necessary with \p Finder. Usually,
   /// "this" will be used as callback, but you can also specify other callback
   /// classes. Thereby, different matchers can trigger different callbacks.
protected:
   /// \brief Returns the language options from the context.
   LangOptions getLangOpts() const { return Context->getLangOpts(); }
 };
 
 class ClangTidyCheckFactories;
 
 class ClangTidyASTConsumerFactory {
 public:
-  ClangTidyASTConsumerFactory(ClangTidyContext &Context);
+  ClangTidyASTConsumerFactory(
+      ClangTidyContext &Context,
+      IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFS = nullptr);
 
   /// \brief Returns an ASTConsumer that runs the specified clang-tidy checks.
   std::unique_ptr<clang::ASTConsumer>
   CreateASTConsumer(clang::CompilerInstance &Compiler, StringRef File);
 
   /// \brief Get the list of enabled checks.
   std::vector<std::string> getCheckNames();
 
   /// \brief Get the union of options from all checks.
   ClangTidyOptions::OptionMap getCheckOptions();
 
 private:
   ClangTidyContext &Context;
+  IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFS;
   std::unique_ptr<ClangTidyCheckFactories> CheckFactories;
 };
 
 /// \brief Fills the list of check names that are enabled when the provided
 /// filters are applied.
 std::vector<std::string> getCheckNames(const ClangTidyOptions &Options,
                                        bool AllowEnablingAnalyzerAlphaCheckers);
 

 /// in MatchFinder, and will contain the result of the profile.
 /// \param StoreCheckProfile If provided, and EnableCheckProfile is true,
 /// the profile will not be output to stderr, but will instead be stored
 /// as a JSON file in the specified directory.
 std::vector<ClangTidyError>
 runClangTidy(clang::tidy::ClangTidyContext &Context,
              const tooling::CompilationDatabase &Compilations,
              ArrayRef<std::string> InputFiles,
-             llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS,
+             llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> BaseFS,
              bool EnableCheckProfile = false,
              llvm::StringRef StoreCheckProfile = StringRef());
 
 // 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-tools-extra/clang-tidy/ClangTidy.cpp

 ///  for details on setting it up with LLVM source tree.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "ClangTidy.h"
 #include "ClangTidyDiagnosticConsumer.h"
 #include "ClangTidyModuleRegistry.h"
 #include "ClangTidyProfiling.h"
+#include "ExpandModularHeadersPPCallbacks.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Config/config.h"
 #include "clang/Format/Format.h"
 #include "clang/Frontend/ASTConsumers.h"
 #include "clang/Frontend/CompilerInstance.h"
private:
   std::unique_ptr<ClangTidyProfiling> Profiling;
   std::unique_ptr<ast_matchers::MatchFinder> Finder;
   std::vector<std::unique_ptr<ClangTidyCheck>> Checks;
 };
 
 } // namespace
 
 ClangTidyASTConsumerFactory::ClangTidyASTConsumerFactory(
-    ClangTidyContext &Context)
-    : Context(Context), CheckFactories(new ClangTidyCheckFactories) {
+    ClangTidyContext &Context,
+    IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFS)
+    : Context(Context), OverlayFS(OverlayFS),
+      CheckFactories(new ClangTidyCheckFactories) {
   for (ClangTidyModuleRegistry::iterator I = ClangTidyModuleRegistry::begin(),
                                          E = ClangTidyModuleRegistry::end();
        I != E; ++I) {
     std::unique_ptr<ClangTidyModule> Module(I->instantiate());
     Module->addCheckFactories(*CheckFactories);
   }
 }
 

 }
 #endif // CLANG_ENABLE_STATIC_ANALYZER
 
 std::unique_ptr<clang::ASTConsumer>
 ClangTidyASTConsumerFactory::CreateASTConsumer(
     clang::CompilerInstance &Compiler, StringRef File) {
   // FIXME: Move this to a separate method, so that CreateASTConsumer doesn't
   // modify Compiler.
-  Context.setSourceManager(&Compiler.getSourceManager());
+  SourceManager *SM = &Compiler.getSourceManager();
+  Context.setSourceManager(SM);
   Context.setCurrentFile(File);
   Context.setASTContext(&Compiler.getASTContext());
 
   auto WorkingDir = Compiler.getSourceManager()
                         .getFileManager()
                         .getVirtualFileSystem()
                         ->getCurrentWorkingDirectory();
   if (WorkingDir)
if (Context.getEnableProfiling()) {
     Profiling = llvm::make_unique<ClangTidyProfiling>(
         Context.getProfileStorageParams());
     FinderOptions.CheckProfiling.emplace(Profiling->Records);
   }
 
   std::unique_ptr<ast_matchers::MatchFinder> Finder(
       new ast_matchers::MatchFinder(std::move(FinderOptions)));
 
+  Preprocessor *PP = &Compiler.getPreprocessor();
+  Preprocessor *ModuleExpanderPP = PP;
+
+  if (Context.getLangOpts().Modules && OverlayFS != nullptr) {
+    auto ModuleExpander = llvm::make_unique<ExpandModularHeadersPPCallbacks>(
+        &Compiler, OverlayFS);
+    ModuleExpanderPP = ModuleExpander->getPreprocessor();
+    PP->addPPCallbacks(std::move(ModuleExpander));
+  }
+
   for (auto &Check : Checks) {
     Check->registerMatchers(&*Finder);
     Check->registerPPCallbacks(Compiler);
+    Check->registerPPCallbacks(*SM, PP, ModuleExpanderPP);
   }
 
   std::vector<std::unique_ptr<ASTConsumer>> Consumers;
   if (!Checks.empty())
     Consumers.push_back(Finder->newASTConsumer());
 
 #if CLANG_ENABLE_STATIC_ANALYZER
   AnalyzerOptionsRef AnalyzerOptions = Compiler.getAnalyzerOpts();
getCheckOptions(const ClangTidyOptions &Options,
   ClangTidyASTConsumerFactory Factory(Context);
   return Factory.getCheckOptions();
 }
 
 std::vector<ClangTidyError>
 runClangTidy(clang::tidy::ClangTidyContext &Context,
              const CompilationDatabase &Compilations,
              ArrayRef<std::string> InputFiles,
-             llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS,
+             llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> BaseFS,
              bool EnableCheckProfile, llvm::StringRef StoreCheckProfile) {
   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);
runClangTidy(clang::tidy::ClangTidyContext &Context,
   ClangTidyDiagnosticConsumer DiagConsumer(Context);
   DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions(),
                        &DiagConsumer, /*ShouldOwnClient=*/false);
   Context.setDiagnosticsEngine(&DE);
   Tool.setDiagnosticConsumer(&DiagConsumer);
 
   class ActionFactory : public FrontendActionFactory {
   public:
-    ActionFactory(ClangTidyContext &Context) : ConsumerFactory(Context) {}
+    ActionFactory(ClangTidyContext &Context,
+                  IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> BaseFS)
+        : ConsumerFactory(Context, BaseFS) {}
     FrontendAction *create() override { return new Action(&ConsumerFactory); }
 
     bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation,
                        FileManager *Files,
                        std::shared_ptr<PCHContainerOperations> PCHContainerOps,
                        DiagnosticConsumer *DiagConsumer) override {
       // Explicitly set ProgramAction to RunAnalysis to make the preprocessor
       // define __clang_analyzer__ macro. The frontend analyzer action will not
private:
 
     private:
       ClangTidyASTConsumerFactory *Factory;
     };
 
     ClangTidyASTConsumerFactory ConsumerFactory;
   };
 
-  ActionFactory Factory(Context);
+  ActionFactory Factory(Context, BaseFS);
   Tool.run(&Factory);
   return DiagConsumer.take();
 }
 
 void handleErrors(llvm::ArrayRef<ClangTidyError> Errors,
                   ClangTidyContext &Context, bool Fix,
                   unsigned &WarningsAsErrorsCount,
                   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS) {

clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h

+//===- ExpandModularHeadersPPCallbacks.h - clang-tidy -----------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// Defines the ExpandModularHeadersPPCallbacks class that handles #includes of
+// modular headers, traverses all transitively included headers in non-modular
+// mode and generates PPCallbacks for their contents.
+// This allows existing tools based on PPCallbacks to retain their functionality
+// when running with C++ modules enabled.
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLING_EXPANDMODULARHEADERSPPCALLBACKS_H_
+#define LLVM_CLANG_TOOLING_EXPANDMODULARHEADERSPPCALLBACKS_H_
+
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/DenseSet.h"
+
+namespace clang {
+class CompilerInstance;
+
+namespace serialization {
+class ModuleFile;
+}
+
+namespace tooling {
+
+/// \brief Handles PPCallbacks and replays preprocessing with modules disabled.
+///
+/// This way it's possible to get PPCallbacks from the contents of the modular
+/// headers and all their transitive includes.
+class ExpandModularHeadersPPCallbacks : public PPCallbacks {
+public:
+  ExpandModularHeadersPPCallbacks(
+      CompilerInstance *compiler,
+      IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFS);
+  ~ExpandModularHeadersPPCallbacks();
+
+  /// \brief Users can get expanded PPCallbacks by registering their callback
+  /// handlers in the preprocessor instance returned by this method.
+  Preprocessor *getPreprocessor() const;
+
+private:
+  class FileRecorder;
+
+  void handleModuleFile(serialization::ModuleFile *MF);
+  void ParseToLocation(SourceLocation loc);
+
+  // Handle PPCallbacks.
+  void FileChanged(SourceLocation Loc, FileChangeReason Reason,
+                   SrcMgr::CharacteristicKind FileType,
+                   FileID PrevFID) override;
+
+  void InclusionDirective(SourceLocation directive_loc,
+                          const Token &include_token,
+                          StringRef included_filename, bool is_angled,
+                          CharSourceRange filename_range,
+                          const FileEntry *included_file, StringRef search_path,
+                          StringRef relative_path, const Module *imported,
+                          SrcMgr::CharacteristicKind FileType) override;
+
+  // FIXME: This crashes:
+  // void EndOfMainFile() override { target_callbacks_->EndOfMainFile(); }
+
+  // Handle all other callbacks.
+  // Just parse to the corresponding location to generate PPCallbacks for the
+  // corresponding range
+  void Ident(SourceLocation Loc, StringRef) override;
+  void PragmaDirective(SourceLocation Loc, PragmaIntroducerKind) override;
+  void PragmaComment(SourceLocation Loc, const IdentifierInfo *,
+                     StringRef) override;
+  void PragmaDetectMismatch(SourceLocation Loc, StringRef, StringRef) override;
+  void PragmaDebug(SourceLocation Loc, StringRef) override;
+  void PragmaMessage(SourceLocation Loc, StringRef, PragmaMessageKind,
+                     StringRef) override;
+  void PragmaDiagnosticPush(SourceLocation Loc, StringRef) override;
+  void PragmaDiagnosticPop(SourceLocation Loc, StringRef) override;
+  void PragmaDiagnostic(SourceLocation Loc, StringRef, diag::Severity,
+                        StringRef) override;
+  void HasInclude(SourceLocation Loc, StringRef, bool, const FileEntry *,
+                  SrcMgr::CharacteristicKind) override;
+  void PragmaOpenCLExtension(SourceLocation NameLoc, const IdentifierInfo *,
+                             SourceLocation StateLoc, unsigned) override;
+  void PragmaWarning(SourceLocation Loc, StringRef, ArrayRef<int>) override;
+  void PragmaWarningPush(SourceLocation Loc, int) override;
+  void PragmaWarningPop(SourceLocation Loc) override;
+  void PragmaAssumeNonNullBegin(SourceLocation Loc) override;
+  void PragmaAssumeNonNullEnd(SourceLocation Loc) override;
+  void MacroExpands(const Token &MacroNameTok, const MacroDefinition &,
+                    SourceRange Range, const MacroArgs *) override;
+  void MacroDefined(const Token &MacroNameTok,
+                    const MacroDirective *MD) override;
+  void MacroUndefined(const Token &, const MacroDefinition &,
+                      const MacroDirective *Undef) override;
+  void Defined(const Token &MacroNameTok, const MacroDefinition &,
+               SourceRange Range) override;
+  void SourceRangeSkipped(SourceRange Range, SourceLocation EndifLoc) override;
+  void If(SourceLocation Loc, SourceRange, ConditionValueKind) override;
+  void Elif(SourceLocation Loc, SourceRange, ConditionValueKind,
+            SourceLocation) override;
+  void Ifdef(SourceLocation Loc, const Token &,
+             const MacroDefinition &) override;
+  void Ifndef(SourceLocation Loc, const Token &,
+              const MacroDefinition &) override;
+  void Else(SourceLocation Loc, SourceLocation) override;
+  void Endif(SourceLocation Loc, SourceLocation) override;
+
+  std::unique_ptr<FileRecorder> file_recorder_;
+  // Set of all the modules visited. Avoids processing a module more than once.
+  llvm::DenseSet<serialization::ModuleFile *> visited_modules_;
+
+  CompilerInstance &compiler_;
+  // Additional filesystem for replay. Provides all input files from modules.
+  llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> in_memory_fs_;
+
+  SourceManager &sources_;
+  DiagnosticsEngine diags_;
+  LangOptions lang_opts_;
+  TrivialModuleLoader module_loader_;
+
+  std::unique_ptr<HeaderSearch> header_info_;
+  std::unique_ptr<Preprocessor> preprocessor_;
+  bool entered_main_file_ = false;
+  bool started_lexing_ = false;
+  Token current_token_;
+};
+
+} // namespace tooling
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLING_EXPANDMODULARHEADERSPPCALLBACKS_H_

clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp

+//===- ExpandModularHeadersPPCallbacks.h - clang-tidy -----------*- C++ -*-===//
+//
+// 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 "ExpandModularHeadersPPCallbacks.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Serialization/ASTReader.h"
+
+namespace clang {
+namespace tooling {
+
+class ExpandModularHeadersPPCallbacks::FileRecorder {
+public:
+  // Stores FileEntry for which contents are to be recorded later.
+  void AddFileToRecord(const FileEntry *file) { files_to_record_.insert(file); }
+
+  // Records content for a file and adds it to the FileSystem.
+  void RecordContent(const FileEntry *file,
+                     const SrcMgr::ContentCache *content_cache,
+                     llvm::vfs::InMemoryFileSystem &in_memory_fs_) {
+    // Return if we are not interested in the contents of this file.
+    if (!files_to_record_.count(file))
+      return;
+
+    //assert(content_cache->getRawBuffer() && "No raw buffer");
+    // FIXME: Why is this happening? We might be losing contents here.
+    if (!content_cache->getRawBuffer())
+      return;
+
+    in_memory_fs_.addFile(file->getName(), /*ModificationTime=*/0,
+                          llvm::MemoryBuffer::getMemBufferCopy(
+                              content_cache->getRawBuffer()->getBuffer()));
+    // Remove file since we have successfully recorded its contents.
+    files_to_record_.erase(file);
+  }
+
+  // Make sure we have contents for all the files we were interested in. Ideally
+  // `files_to_record_` should be empty.
+  void CheckAllFilesRecorded() {
+    for (auto file_entry : files_to_record_)
+      llvm::errs() << "Did not record contents for input file: "
+                   << file_entry->getName() << "\n";
+  }
+
+private:
+  // Set of files whose contents are to be recorded.
+  llvm::DenseSet<const FileEntry *> files_to_record_;
+};
+
+ExpandModularHeadersPPCallbacks::ExpandModularHeadersPPCallbacks(
+    CompilerInstance *compiler,
+    IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFS)
+    : file_recorder_(llvm::make_unique<FileRecorder>()), compiler_(*compiler),
+      in_memory_fs_(new llvm::vfs::InMemoryFileSystem),
+      sources_(compiler_.getSourceManager()),
+      // Forward the new diagnostics to the original DiagnosticConsumer.
+      diags_(new DiagnosticIDs, new DiagnosticOptions,
+             new ForwardingDiagnosticConsumer(compiler_.getDiagnosticClient())),
+      lang_opts_(compiler_.getLangOpts()) {
+  // Add FileSystem containing the extra files needed in place of modular
+  // headers.
+  OverlayFS->pushOverlay(in_memory_fs_);
+
+  diags_.setSourceManager(&sources_);
+
+  // Switch of header modules in the new preprocessor.
+  lang_opts_.Modules = false;
+
+  auto header_search_options = std::make_shared<HeaderSearchOptions>();
+  *header_search_options = compiler_.getHeaderSearchOpts();
+
+  header_info_ =
+      llvm::make_unique<HeaderSearch>(header_search_options, sources_, diags_,
+                                      lang_opts_, &compiler_.getTarget());
+
+  auto preprocessor_options = std::make_shared<PreprocessorOptions>();
+  *preprocessor_options = compiler_.getPreprocessorOpts();
+
+  preprocessor_ =
+      llvm::make_unique<Preprocessor>(preprocessor_options, diags_, lang_opts_,
+                                      sources_, *header_info_, module_loader_,
+                                      /*IILookup=*/nullptr,
+                                      /*OwnsHeaderSearch=*/false);
+  preprocessor_->Initialize(compiler_.getTarget(), compiler_.getAuxTarget());
+  InitializePreprocessor(*preprocessor_, *preprocessor_options,
+                         compiler_.getPCHContainerReader(),
+                         compiler_.getFrontendOpts());
+  ApplyHeaderSearchOptions(*header_info_, *header_search_options, lang_opts_,
+                           compiler_.getTarget().getTriple());
+}
+
+ExpandModularHeadersPPCallbacks::~ExpandModularHeadersPPCallbacks() = default;
+
+Preprocessor *ExpandModularHeadersPPCallbacks::getPreprocessor() const {
+  return preprocessor_.get();
+}
+
+void ExpandModularHeadersPPCallbacks::handleModuleFile(
+    serialization::ModuleFile *MF) {
+  if (!MF)
+    return;
+  // Avoid processing a ModuleFile more than once.
+  if (visited_modules_.count(MF))
+    return;
+  visited_modules_.insert(MF);
+
+  // Visit all the input files of this module and mark them to be record their
+  // contents later.
+  compiler_.getModuleManager()->visitInputFiles(
+      *MF, true, false,
+      [this](const serialization::InputFile &IF, bool isSystem) {
+        auto *F = IF.getFile();
+        file_recorder_->AddFileToRecord(F);
+      });
+  // Recursively handle all transitively imported modules.
+  for (auto import : MF->Imports)
+    handleModuleFile(import);
+}
+
+void ExpandModularHeadersPPCallbacks::ParseToLocation(SourceLocation loc) {
+  // Load all source locations present in the external sources.
+  for (unsigned I = 0, N = sources_.loaded_sloc_entry_size(); I != N; ++I) {
+    sources_.getLoadedSLocEntry(I, nullptr);
+  }
+  // Record contents of files we are interested in and add to the FileSystem.
+  for (auto it = sources_.fileinfo_begin(); it != sources_.fileinfo_end();
+       ++it) {
+    file_recorder_->RecordContent(it->getFirst(), it->getSecond(),
+                                  *in_memory_fs_);
+  }
+  file_recorder_->CheckAllFilesRecorded();
+
+  if (!started_lexing_) {
+    started_lexing_ = true;
+    preprocessor_->Lex(current_token_);
+  }
+  while (
+      !current_token_.is(tok::eof) &&
+      sources_.isBeforeInTranslationUnit(current_token_.getLocation(), loc)) {
+    preprocessor_->Lex(current_token_);
+  }
+}
+
+void ExpandModularHeadersPPCallbacks::FileChanged(
+    SourceLocation Loc, FileChangeReason Reason,
+    SrcMgr::CharacteristicKind FileType, FileID PrevFID = FileID()) {
+  if (!entered_main_file_) {
+    entered_main_file_ = true;
+    preprocessor_->EnterMainSourceFile();
+  }
+}
+
+void ExpandModularHeadersPPCallbacks::InclusionDirective(
+    SourceLocation directive_loc, const Token &include_token,
+    StringRef included_filename, bool is_angled, CharSourceRange filename_range,
+    const FileEntry *included_file, StringRef search_path,
+    StringRef relative_path, const Module *imported,
+    SrcMgr::CharacteristicKind FileType) {
+  if (imported) {
+    serialization::ModuleFile *MF =
+        compiler_.getModuleManager()->getModuleManager().lookup(
+            imported->getASTFile());
+    handleModuleFile(MF);
+  }
+  ParseToLocation(directive_loc);
+}
+
+// FIXME: This crashes: void EndOfMainFile() override {
+// target_callbacks_->EndOfMainFile(); }
+
+// Handle all other callbacks.
+// Just parse to the corresponding location to generate the same callback for
+// the target_callbacks_.
+void ExpandModularHeadersPPCallbacks::Ident(SourceLocation Loc, StringRef) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::PragmaDirective(SourceLocation Loc,
+                                                      PragmaIntroducerKind) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::PragmaComment(SourceLocation Loc,
+                                                    const IdentifierInfo *,
+                                                    StringRef) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::PragmaDetectMismatch(SourceLocation Loc,
+                                                           StringRef,
+                                                           StringRef) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::PragmaDebug(SourceLocation Loc,
+                                                  StringRef) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::PragmaMessage(SourceLocation Loc,
+                                                    StringRef,
+                                                    PragmaMessageKind,
+                                                    StringRef) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::PragmaDiagnosticPush(SourceLocation Loc,
+                                                           StringRef) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::PragmaDiagnosticPop(SourceLocation Loc,
+                                                          StringRef) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::PragmaDiagnostic(SourceLocation Loc,
+                                                       StringRef,
+                                                       diag::Severity,
+                                                       StringRef) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::HasInclude(SourceLocation Loc, StringRef,
+                                                 bool, const FileEntry *,
+                                                 SrcMgr::CharacteristicKind) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::PragmaOpenCLExtension(
+    SourceLocation NameLoc, const IdentifierInfo *, SourceLocation StateLoc,
+    unsigned) {
+  // FIME: Figure out whether it's the right location to parse to.
+  ParseToLocation(NameLoc);
+}
+void ExpandModularHeadersPPCallbacks::PragmaWarning(SourceLocation Loc,
+                                                    StringRef, ArrayRef<int>) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::PragmaWarningPush(SourceLocation Loc,
+                                                        int) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::PragmaWarningPop(SourceLocation Loc) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::PragmaAssumeNonNullBegin(
+    SourceLocation Loc) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::PragmaAssumeNonNullEnd(
+    SourceLocation Loc) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::MacroExpands(const Token &MacroNameTok,
+                                                   const MacroDefinition &,
+                                                   SourceRange Range,
+                                                   const MacroArgs *) {
+  // FIME: Figure out whether it's the right location to parse to.
+  ParseToLocation(Range.getBegin());
+}
+void ExpandModularHeadersPPCallbacks::MacroDefined(const Token &MacroNameTok,
+                                                   const MacroDirective *MD) {
+  ParseToLocation(MD->getLocation());
+}
+void ExpandModularHeadersPPCallbacks::MacroUndefined(
+    const Token &, const MacroDefinition &, const MacroDirective *Undef) {
+  if (Undef)
+    ParseToLocation(Undef->getLocation());
+}
+void ExpandModularHeadersPPCallbacks::Defined(const Token &MacroNameTok,
+                                              const MacroDefinition &,
+                                              SourceRange Range) {
+  // FIME: Figure out whether it's the right location to parse to.
+  ParseToLocation(Range.getBegin());
+}
+void ExpandModularHeadersPPCallbacks::SourceRangeSkipped(
+    SourceRange Range, SourceLocation EndifLoc) {
+  // FIME: Figure out whether it's the right location to parse to.
+  ParseToLocation(EndifLoc);
+}
+void ExpandModularHeadersPPCallbacks::If(SourceLocation Loc, SourceRange,
+                                         ConditionValueKind) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::Elif(SourceLocation Loc, SourceRange,
+                                           ConditionValueKind, SourceLocation) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::Ifdef(SourceLocation Loc, const Token &,
+                                            const MacroDefinition &) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::Ifndef(SourceLocation Loc, const Token &,
+                                             const MacroDefinition &) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::Else(SourceLocation Loc, SourceLocation) {
+  ParseToLocation(Loc);
+}
+void ExpandModularHeadersPPCallbacks::Endif(SourceLocation Loc,
+                                            SourceLocation) {
+  ParseToLocation(Loc);
+}
+
+} // namespace tooling
+} // namespace clang

clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h

 /// configured.
 class IdentifierNamingCheck : public ClangTidyCheck {
 public:
   IdentifierNamingCheck(StringRef Name, ClangTidyContext *Context);
 
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-  void registerPPCallbacks(CompilerInstance &Compiler) override;
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+                           Preprocessor *ModuleExpanderPP) override;
   void onEndOfTranslationUnit() override;
 
   enum CaseType {
     CT_AnyCase = 0,
     CT_LowerCase,
     CT_CamelBack,
     CT_UpperCase,
     CT_CamelCase,

clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp

void IdentifierNamingCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(usingDecl().bind("using"), this);
   Finder->addMatcher(declRefExpr().bind("declRef"), this);
   Finder->addMatcher(cxxConstructorDecl().bind("classRef"), this);
   Finder->addMatcher(cxxDestructorDecl().bind("classRef"), this);
   Finder->addMatcher(typeLoc().bind("typeLoc"), this);
   Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this);
 }
 
-void IdentifierNamingCheck::registerPPCallbacks(CompilerInstance &Compiler) {
-  Compiler.getPreprocessor().addPPCallbacks(
-      llvm::make_unique<IdentifierNamingCheckPPCallbacks>(
-          &Compiler.getPreprocessor(), this));
+void IdentifierNamingCheck::registerPPCallbacks(
+    const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  ModuleExpanderPP->addPPCallbacks(
+      llvm::make_unique<IdentifierNamingCheckPPCallbacks>(ModuleExpanderPP,
+                                                          this));
 }
 
 static bool matchesStyle(StringRef Name,
                          IdentifierNamingCheck::NamingStyle Style) {
   static llvm::Regex Matchers[] = {
       llvm::Regex("^.*$"),
       llvm::Regex("^[a-z][a-z0-9_]*$"),
       llvm::Regex("^[a-z][a-zA-Z0-9]*$"),

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

if (llvm::ErrorOr<ClangTidyOptions> ParsedConfig =
       return nullptr;
     }
   }
   return llvm::make_unique<FileOptionsProvider>(GlobalOptions, DefaultOptions,
                                                 OverrideOptions, std::move(FS));
 }
 
 llvm::IntrusiveRefCntPtr<vfs::FileSystem>
-getVfsOverlayFromFile(const std::string &OverlayFile) {
-  llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> OverlayFS(
-      new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
+getVfsFromFile(const std::string &OverlayFile,
+               llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS) {
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buffer =
-      OverlayFS->getBufferForFile(OverlayFile);
+      BaseFS->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;
+  return FS;
 }
 
 static int clangTidyMain(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory,
                                     cl::ZeroOrMore);
-  llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS(
-      VfsOverlay.empty() ? vfs::getRealFileSystem()
-                         : getVfsOverlayFromFile(VfsOverlay));
-  if (!BaseFS)
+  llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> BaseFS(
+      new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
+
+  if (!VfsOverlay.empty()) {
+    IntrusiveRefCntPtr<vfs::FileSystem> VfsFromFile =
+        getVfsFromFile(VfsOverlay, BaseFS);
+    if (!VfsFromFile)
       return 1;
+    BaseFS->pushOverlay(VfsFromFile);
+  }
 
   auto OwningOptionsProvider = createOptionsProvider(BaseFS);
   auto *OptionsProvider = OwningOptionsProvider.get();
   if (!OptionsProvider)
     return 1;
 
   auto MakeAbsolute = [](const std::string &Input) -> SmallString<256> {
     if (Input.empty())

clang-tools-extra/test/CMakeLists.txt

set(CLANG_TOOLS_TEST_DEPS
   ExtraToolsUnitTests
 
   # For the clang-tidy libclang integration test.
   c-index-test
   # clang-tidy tests require it.
   clang-resource-headers
 
   clang-tidy
+  # Clang-tidy tests need clang for building modules.
+  clang
 )
 
 if(CLANGD_BUILD_XPC_SUPPORT)
   list(APPEND CLANG_TOOLS_TEST_DEPS clangd-xpc-test-client)
 endif()
 
 set(CLANGD_TEST_DEPS
   clangd

clang-tools-extra/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/a.h

+#define a

clang-tools-extra/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/b.h

+#include "a.h"
+#define b

clang-tools-extra/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/c.h

+#include "b.h"
+#define c

clang-tools-extra/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/module.modulemap

+module a { header "a.h" export * }
+module b { header "b.h" export * use a }
+module c { header "c.h" export * use b }

clang-tools-extra/test/clang-tidy/expand-modular-headers-ppcallbacks.cpp

+// Sanity-check. Run without modules:
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cp %S/Inputs/expand-modular-headers-ppcallbacks/* %t/
+// RUN: %check_clang_tidy %s readability-identifier-naming %t/without-modules -- \
+// RUN:   -config="CheckOptions: [{ \
+// RUN:      key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
+// RUN:   -header-filter=.* \
+// RUN:   -- -x c++ -std=c++11 -I%t/
+//
+// Run clang-tidy on a file with modular includes:
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cp %S/Inputs/expand-modular-headers-ppcallbacks/* %t/
+// RUN: %check_clang_tidy %s readability-identifier-naming %t/with-modules -- \
+// RUN:   -config="CheckOptions: [{ \
+// RUN:      key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
+// RUN:   -header-filter=.* \
+// RUN:   -- -x c++ -std=c++11 -I%t/ \
+// RUN:   -fmodules -fimplicit-modules -fno-implicit-module-maps \
+// RUN:   -fmodule-map-file=%t/module.modulemap \
+// RUN:   -fmodules-cache-path=%t/module-cache/
+#include "c.h"
+
+// CHECK-MESSAGES: a.h:1:9: warning: invalid case style for macro definition 'a' [readability-identifier-naming]
+// CHECK-MESSAGES: a.h:1:9: note: FIX-IT applied suggested code changes
+// CHECK-MESSAGES: b.h:2:9: warning: invalid case style for macro definition 'b' [readability-identifier-naming]
+// CHECK-MESSAGES: b.h:2:9: note: FIX-IT applied suggested code changes
+// CHECK-MESSAGES: c.h:2:9: warning: invalid case style for macro definition 'c' [readability-identifier-naming]
+// CHECK-MESSAGES: c.h:2:9: note: FIX-IT applied suggested code changes