[clang] Add PPCallbacks list to preprocessor when building a preacompiled preamble.

[Nebiroth]

Summary

Revision D38639 needs this commit in order to properly make open definition calls on include statements work.
Since this modifies clang and not clangd, I decided to include it in a different patch.

[malaperle]

Can you revert the formatting changes in places you haven’t touched (mainly astunit.cpp)? I think you should be able to do that with git checkout -p HEAD~1

[ilya-biryukov]

Could you please take a different approach and add the callbacks you need into PreambleCallbacks class?
It already has this one:

virtual void HandleMacroDefined(const Token &MacroNameTok,
                               const MacroDirective *MD);

Otherwise we'll be creating too many ways to do the same thing.

We should also definitely get rid of all the unrelated formatting changes.

[Nebiroth]

Reverted formatting changes to ASTUnit

[ilya-biryukov]

lib/Frontend/PrecompiledPreamble.cpp

242

Could we add a method to PreambleCallbacks to create PPCallbacks instead?
Otherwise we have both MacroDefined in PreambleCallbacks and a separate set of PPCallbacks, so we have two ways of doing the same thing.

class PreambleCallbacks {
public:
// ...

  /// Remove this.
  virtual void HandleMacroDefined(...);

  // Add this instead.
  virtual std::unique_ptr<PPCallbacks> createPPCallbacks();

}

Alternatively, follow the current pattern in PreambleCallbacks and add some extra functions from the PPCallbacks interface to it. This would not require changes to the existing usages of PrecompiledPreamble in ASTUnit. Albeit, I'd prefer the first option.

class PreambleCallbacks {
public:
// ...

  // Keep this
  virtual void HandleMacroDefined(...);
  // Add the ones you need, e.g.
  virtual void InclusionDirective(...);
  virtual void FileChanged(...);
};

[Nebiroth]

lib/Frontend/PrecompiledPreamble.cpp

242

If we were to do that, one would then be required to define a wrapper class for PPCallbacks and create an instance of it inside createPPCallbacks() for the purpose of creating a unique_ptr? Then that unique_ptr would be sent from within the PreambleCallbacks parameter in the Build function?

[ilya-biryukov]

lib/Frontend/PrecompiledPreamble.cpp

242

We're already passing our own wrapper around PreambleCallbacks anyway (see PreambleMacroCallbacks), we'll pass the unique_ptr<PPCallbacks> instead.
But, yes, the clients will have to write their own implementation of PPCallbacks and pass it as unique_ptr. Is there something wrong with that?

Or, have I misunderstood the question entirely?

[Nebiroth]

lib/Frontend/PrecompiledPreamble.cpp

242

No this is fine. I was just making sure I understood correctly.

[Nebiroth]

Removed unique_ptr parameter in PrecompiledPreamble::Build
Added method createPPCallbacks() in PreambleCallback to be overriden

[malaperle]

include/clang/Frontend/PrecompiledPreamble.h

263

It doesn't add, it creates.It's also not a "list". Is it true that it will process includes outside the preamble? I would think building the preamble stopped at the end of the preamble.

lib/Frontend/PrecompiledPreamble.cpp

354

extract this to a local variable

[Nebiroth]

Minor code cleanup

[malaperle]

Looks good to me. But this is not really my area :)

[ilya-biryukov]

LGTM modulo a few comments.
AFAIK, @Nebiroth does not have a commit access, so I'll land this with minor tweaks to unblock the other patch.

include/clang/Frontend/PrecompiledPreamble.h

261

This method can now be removed as its use-case is now covered by createPPCallbacks

lib/Frontend/PrecompiledPreamble.cpp

713

Please clang-format the code when submitting patches.

include/clang/Frontend/PrecompiledPreamble.h

public:
   ///
   /// \param Callbacks A set of callbacks to be executed when building
   /// the preamble.
   static llvm::ErrorOr<PrecompiledPreamble>
   Build(const CompilerInvocation &Invocation,
         const llvm::MemoryBuffer *MainFileBuffer, PreambleBounds Bounds,
         DiagnosticsEngine &Diagnostics, IntrusiveRefCntPtr<vfs::FileSystem> VFS,
         std::shared_ptr<PCHContainerOperations> PCHContainerOps,
-        bool StoreInMemory, PreambleCallbacks &Callbacks);
+        bool StoreInMemory, PreambleCallbacks &Callbacks, std::unique_ptr<PPCallbacks> PPCallbacks);
 
   PrecompiledPreamble(PrecompiledPreamble &&) = default;
   PrecompiledPreamble &operator=(PrecompiledPreamble &&) = default;
 
   /// PreambleBounds used to build the preamble.
   PreambleBounds getBounds() const;
 
   /// Check whether PrecompiledPreamble can be reused for the new contents(\p
public:
   virtual void AfterPCHEmitted(ASTWriter &Writer);
   /// Called for each TopLevelDecl.
   /// NOTE: To allow more flexibility a custom ASTConsumer could probably be
   /// used instead, but having only this method allows a simpler API.
   virtual void HandleTopLevelDecl(DeclGroupRef DG);
   /// Called for each macro defined in the Preamble.
   /// NOTE: To allow more flexibility a custom PPCallbacks could probably be
   /// used instead, but having only this method allows a simpler API.
   virtual void HandleMacroDefined(const Token &MacroNameTok,
                                   const MacroDirective *MD);
 };
 
 enum class BuildPreambleError {
   PreambleIsEmpty = 1,
   CouldntCreateTempFile,
   CouldntCreateTargetInfo,
   CouldntCreateVFSOverlay,
   BeginSourceFileFailed,
   CouldntEmitPCH

lib/Frontend/ASTUnit.cpp

if (CaptureDiagnostics)
                       &NewPreambleDiagsStandalone);
 
     // We did not previously compute a preamble, or it can't be reused anyway.
     SimpleTimer PreambleTimer(WantTiming);
     PreambleTimer.setOutput("Precompiling preamble");
 
     llvm::ErrorOr<PrecompiledPreamble> NewPreamble = PrecompiledPreamble::Build(
         PreambleInvocationIn, MainFileBuffer.get(), Bounds, *Diagnostics, VFS,
-        PCHContainerOps, /*StoreInMemory=*/false, Callbacks);
+        PCHContainerOps, /*StoreInMemory=*/false, Callbacks, nullptr);
     if (NewPreamble) {
       Preamble = std::move(*NewPreamble);
       PreambleRebuildCounter = 1;
     } else {
       switch (static_cast<BuildPreambleError>(NewPreamble.getError().value())) {
       case BuildPreambleError::CouldntCreateTempFile:
       case BuildPreambleError::PreambleIsEmpty:
         // Try again next time.

lib/Frontend/PrecompiledPreamble.cpp

PreambleBounds clang::ComputePreambleBounds(const LangOptions &LangOpts,
   return Lexer::ComputePreamble(Buffer->getBuffer(), LangOpts, MaxLines);
 }
 
 llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build(
     const CompilerInvocation &Invocation,
     const llvm::MemoryBuffer *MainFileBuffer, PreambleBounds Bounds,
     DiagnosticsEngine &Diagnostics, IntrusiveRefCntPtr<vfs::FileSystem> VFS,
     std::shared_ptr<PCHContainerOperations> PCHContainerOps, bool StoreInMemory,
-    PreambleCallbacks &Callbacks) {
+    PreambleCallbacks &Callbacks, std::unique_ptr<PPCallbacks> PPCallbacks) {
   assert(VFS && "VFS is null");
 
   if (!Bounds.Size)
     return BuildPreambleError::PreambleIsEmpty;
 
   auto PreambleInvocation = std::make_shared<CompilerInvocation>(Invocation);
   FrontendOptions &FrontendOpts = PreambleInvocation->getFrontendOpts();
   PreprocessorOptions &PreprocessorOpts =
llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build(
   }
 
   std::unique_ptr<PrecompilePreambleAction> Act;
   Act.reset(new PrecompilePreambleAction(
       StoreInMemory ? &Storage.asMemory().Data : nullptr, Callbacks));
   if (!Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0]))
     return BuildPreambleError::BeginSourceFileFailed;
 
+  Clang->getPreprocessor().addPPCallbacks(std::move(PPCallbacks));
   Act->Execute();
 
   // Run the callbacks.
   Callbacks.AfterExecute(*Clang);
 
   Act->EndSourceFile();
 
   if (!Act->hasEmittedPreamblePCH())
void PrecompiledPreamble::setupPreambleStorage(
   }
 }
 
 void PreambleCallbacks::AfterExecute(CompilerInstance &CI) {}
 void PreambleCallbacks::AfterPCHEmitted(ASTWriter &Writer) {}
 void PreambleCallbacks::HandleTopLevelDecl(DeclGroupRef DG) {}
 void PreambleCallbacks::HandleMacroDefined(const Token &MacroNameTok,
                                            const MacroDirective *MD) {}
 
 std::error_code clang::make_error_code(BuildPreambleError Error) {
   return std::error_code(static_cast<int>(Error), BuildPreambleErrorCategory());
 }
 
 const char *BuildPreambleErrorCategory::name() const noexcept {
   return "build-preamble.error";
 }