[libclang] Add index option to store preambles in memory

[vedgy]

Summary

This commit allows libclang API users to opt into storing PCH in memory
instead of temporary files. The option can be set only during CXIndex
construction to avoid multithreading issues and confusion or bugs if
some preambles are stored in temporary files and others - in memory.

The added API works as expected in KDevelop:
https://invent.kde.org/kdevelop/kdevelop/-/merge_requests/283

[vedgy]

I have implemented the setter for the new option locally and tested it in KDevelop.

void clang_CXIndex_setStorePreamblesInMemory(CXIndex CIdx,
                                             int storePreamblesInMemory) {
  if (CIdx)
    static_cast<CIndexer *>(CIdx)->setStorePreamblesInMemory(
        storePreamblesInMemory);
}

Works as expected: new preambles are created in memory or the temporary directory depending on the option selected in the UI. Even if the temporary storage option ends up in memory, all remaining preambles are removed from the temporary directory on exit.

This simple setter approach was rejected in recent comments under D143418 for valid reasons. But this quick test proves that changing the option on the fly is viable. Currently I don't think that the option change taking effect without restarting KDevelop justifies the effort of implementing the setter as clang_parseTranslationUnitWithOptions(). I expect this option's value to be changed very rarely. So I don't plan to change preamble storage API after this review.

[aaron.ballman]

LGTM!

[vedgy]

@aaron.ballman, can you land this revision for me please?

[aaron.ballman]

In D145974#4195643, @vedgy wrote:

@aaron.ballman, can you land this revision for me please?

Oops, thank you for the reminder! I've landed it on your behalf in 55f7e00afc56c68421220d60d29c079b58fe9c79

clang/docs/ReleaseNotes.rst

- Introduced the new function ``clang_CXXMethod_isExplicit``,
   was marked with the explicit identifier.
 - Added check in ``clang_getFieldDeclBitWidth`` for whether a bit field
   has an evaluable bit width. Fixes undefined behavior when called on a
   bit field whose width depends on a template paramter.
 - Added function ``clang_isBitFieldDecl`` to check if a struct/class field is a
   bit field.
 
 - Introduced the new ``CXIndex`` constructor function
-  ``clang_createIndexWithOptions``, which allows overriding precompiled preamble
-  storage path.
+  ``clang_createIndexWithOptions``, which allows storing precompiled preambles
+  in memory or overriding precompiled preamble storage path.
 
 - Deprecated two functions ``clang_CXIndex_setGlobalOptions`` and
   ``clang_CXIndex_setInvocationEmissionPathOption`` in favor of the new
   function ``clang_createIndexWithOptions`` in order to improve thread safety.
 
 Static Analyzer
 ---------------
 - Fix incorrect alignment attribute on the this parameter of certain

clang/include/clang-c/Index.h

typedef struct CXIndexOptions {
   /**
    * \see clang_createIndex()
    */
   unsigned ExcludeDeclarationsFromPCH : 1;
   /**
    * \see clang_createIndex()
    */
   unsigned DisplayDiagnostics : 1;
-  unsigned /*Reserved*/ : 14;
+  /**
+   * Store PCH in memory. If zero, PCH are stored in temporary files.
+   */
+  unsigned StorePreamblesInMemory : 1;
+  unsigned /*Reserved*/ : 13;
 
   /**
    * The path to a directory, in which to store temporary PCH files. If null or
    * empty, the default system temporary directory is used. These PCH files are
    * deleted on clean exit but stay on disk if the program crashes or is killed.
    *
+   * This option is ignored if \a StorePreamblesInMemory is non-zero.
+   *
    * Libclang does not create the directory at the specified path in the file
    * system. Therefore it must exist, or storing PCH files will fail.
    */
   const char *PreambleStoragePath;
   /**
    * Specifies a path which will contain log files for certain libclang
    * invocations. A null value implies that libclang invocations are not logged.
    */

clang/include/clang/Frontend/ASTUnit.h

private:
   IntrusiveRefCntPtr<TargetInfo>          Target;
   std::shared_ptr<Preprocessor>           PP;
   IntrusiveRefCntPtr<ASTContext>          Ctx;
   std::shared_ptr<TargetOptions>          TargetOpts;
   std::shared_ptr<HeaderSearchOptions>    HSOpts;
   std::shared_ptr<PreprocessorOptions>    PPOpts;
   IntrusiveRefCntPtr<ASTReader> Reader;
   bool HadModuleLoaderFatalFailure = false;
+  bool StorePreamblesInMemory = false;
 
   struct ASTWriterData;
   std::unique_ptr<ASTWriterData> WriterData;
 
   FileSystemOptions FileSystemOpts;
   std::string PreambleStoragePath;
 
   /// The AST consumer that received information about the translation
public:
   /// \param PCHContainerOps - The PCHContainerOperations to use for loading and
   /// creating modules.
   ///
   /// \param Diags - The diagnostics engine to use for reporting errors; its
   /// lifetime is expected to extend past that of the returned ASTUnit.
   ///
   /// \param ResourceFilesPath - The path to the compiler resource files.
   ///
+  /// \param StorePreamblesInMemory - Whether to store PCH in memory. If false,
+  /// PCH are stored in temporary files.
+  ///
   /// \param PreambleStoragePath - The path to a directory, in which to create
   /// temporary PCH files. If empty, the default system temporary directory is
-  /// used.
+  /// used. This parameter is ignored if \p StorePreamblesInMemory is true.
   ///
   /// \param ModuleFormat - If provided, uses the specific module format.
   ///
   /// \param ErrAST - If non-null and parsing failed without any AST to return
   /// (e.g. because the PCH could not be loaded), this accepts the ASTUnit
   /// mainly to allow the caller to see the diagnostics.
   ///
   /// \param VFS - A llvm::vfs::FileSystem to be used for all file accesses.
   /// Note that preamble is saved to a temporary directory on a RealFileSystem,
   /// so in order for it to be loaded correctly, VFS should have access to
   /// it(i.e., be an overlay over RealFileSystem). RealFileSystem will be used
   /// if \p VFS is nullptr.
   ///
   // FIXME: Move OnlyLocalDecls, UseBumpAllocator to setters on the ASTUnit, we
   // shouldn't need to specify them at construction time.
   static ASTUnit *LoadFromCommandLine(
       const char **ArgBegin, const char **ArgEnd,
       std::shared_ptr<PCHContainerOperations> PCHContainerOps,
       IntrusiveRefCntPtr<DiagnosticsEngine> Diags, StringRef ResourceFilesPath,
+      bool StorePreamblesInMemory = false,
       StringRef PreambleStoragePath = StringRef(), bool OnlyLocalDecls = false,
       CaptureDiagsKind CaptureDiagnostics = CaptureDiagsKind::None,
       ArrayRef<RemappedFile> RemappedFiles = std::nullopt,
       bool RemappedFilesKeepOriginalName = true,
       unsigned PrecompilePreambleAfterNParses = 0,
       TranslationUnitKind TUKind = TU_Complete,
       bool CacheCodeCompletionResults = false,
       bool IncludeBriefCommentsInCodeCompletion = false,

clang/lib/Frontend/ASTUnit.cpp

ASTUnitPreambleCallbacks Callbacks;
 
     const bool PreviousSkipFunctionBodies =
         PreambleInvocationIn.getFrontendOpts().SkipFunctionBodies;
     if (SkipFunctionBodies == SkipFunctionBodiesScope::Preamble)
       PreambleInvocationIn.getFrontendOpts().SkipFunctionBodies = true;
 
     llvm::ErrorOr<PrecompiledPreamble> NewPreamble = PrecompiledPreamble::Build(
         PreambleInvocationIn, MainFileBuffer.get(), Bounds, *Diagnostics, VFS,
-        PCHContainerOps, /*StoreInMemory=*/false, PreambleStoragePath,
+        PCHContainerOps, StorePreamblesInMemory, PreambleStoragePath,
         Callbacks);
 
     PreambleInvocationIn.getFrontendOpts().SkipFunctionBodies =
         PreviousSkipFunctionBodies;
 
     if (NewPreamble) {
       Preamble = std::move(*NewPreamble);
       PreambleRebuildCountdown = 1;
if (AST->LoadFromCompilerInvocation(std::move(PCHContainerOps),
     return nullptr;
   return AST;
 }
 
 ASTUnit *ASTUnit::LoadFromCommandLine(
     const char **ArgBegin, const char **ArgEnd,
     std::shared_ptr<PCHContainerOperations> PCHContainerOps,
     IntrusiveRefCntPtr<DiagnosticsEngine> Diags, StringRef ResourceFilesPath,
-    StringRef PreambleStoragePath, bool OnlyLocalDecls,
-    CaptureDiagsKind CaptureDiagnostics, ArrayRef<RemappedFile> RemappedFiles,
-    bool RemappedFilesKeepOriginalName, unsigned PrecompilePreambleAfterNParses,
-    TranslationUnitKind TUKind, bool CacheCodeCompletionResults,
-    bool IncludeBriefCommentsInCodeCompletion, bool AllowPCHWithCompilerErrors,
-    SkipFunctionBodiesScope SkipFunctionBodies, bool SingleFileParse,
-    bool UserFilesAreVolatile, bool ForSerialization,
+    bool StorePreamblesInMemory, StringRef PreambleStoragePath,
+    bool OnlyLocalDecls, CaptureDiagsKind CaptureDiagnostics,
+    ArrayRef<RemappedFile> RemappedFiles, bool RemappedFilesKeepOriginalName,
+    unsigned PrecompilePreambleAfterNParses, TranslationUnitKind TUKind,
+    bool CacheCodeCompletionResults, bool IncludeBriefCommentsInCodeCompletion,
+    bool AllowPCHWithCompilerErrors, SkipFunctionBodiesScope SkipFunctionBodies,
+    bool SingleFileParse, bool UserFilesAreVolatile, bool ForSerialization,
     bool RetainExcludedConditionalBlocks, std::optional<StringRef> ModuleFormat,
     std::unique_ptr<ASTUnit> *ErrAST,
     IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
   assert(Diags.get() && "no DiagnosticsEngine was provided");
 
   SmallVector<StoredDiagnostic, 4> StoredDiagnostics;
 
   std::shared_ptr<CompilerInvocation> CI;
ASTUnit *ASTUnit::LoadFromCommandLine(
   AST->StoredDiagnostics.swap(StoredDiagnostics);
   ConfigureDiags(Diags, *AST, CaptureDiagnostics);
   AST->Diagnostics = Diags;
   AST->FileSystemOpts = CI->getFileSystemOpts();
   if (!VFS)
     VFS = llvm::vfs::getRealFileSystem();
   VFS = createVFSFromCompilerInvocation(*CI, *Diags, VFS);
   AST->FileMgr = new FileManager(AST->FileSystemOpts, VFS);
+  AST->StorePreamblesInMemory = StorePreamblesInMemory;
   AST->PreambleStoragePath = PreambleStoragePath;
   AST->ModuleCache = new InMemoryModuleCache;
   AST->OnlyLocalDecls = OnlyLocalDecls;
   AST->CaptureDiagnostics = CaptureDiagnostics;
   AST->TUKind = TUKind;
   AST->ShouldCacheCodeCompletionResults = CacheCodeCompletionResults;
   AST->IncludeBriefCommentsInCodeCompletion
     = IncludeBriefCommentsInCodeCompletion;

clang/tools/libclang/CIndex.cpp

CXIndex clang_createIndexWithOptions(const CXIndexOptions *options) {
   // advantage of catching more usage bugs and no disadvantages while there is a
   // single supported struct version (the initial version).
   if (options->Size != sizeof(CXIndexOptions))
     return nullptr;
   CIndexer *const CIdxr = clang_createIndex_Impl(
       options->ExcludeDeclarationsFromPCH, options->DisplayDiagnostics,
       options->ThreadBackgroundPriorityForIndexing,
       options->ThreadBackgroundPriorityForEditing);
+  CIdxr->setStorePreamblesInMemory(options->StorePreamblesInMemory);
   CIdxr->setPreambleStoragePath(options->PreambleStoragePath);
   CIdxr->setInvocationEmissionPath(options->InvocationEmissionPath);
   return CIdxr;
 }
 
 void clang_CXIndex_setGlobalOptions(CXIndex CIdx, unsigned options) {
   if (CIdx)
     static_cast<CIndexer *>(CIdx)->setCXGlobalOptFlags(options);
clang_parseTranslationUnit_Impl(CXIndex CIdx, const char *source_filename,
 
   LibclangInvocationReporter InvocationReporter(
       *CXXIdx, LibclangInvocationReporter::OperationKind::ParseOperation,
       options, llvm::ArrayRef(*Args), /*InvocationArgs=*/std::nullopt,
       unsaved_files);
   std::unique_ptr<ASTUnit> Unit(ASTUnit::LoadFromCommandLine(
       Args->data(), Args->data() + Args->size(),
       CXXIdx->getPCHContainerOperations(), Diags,
-      CXXIdx->getClangResourcesPath(), CXXIdx->getPreambleStoragePath(),
-      CXXIdx->getOnlyLocalDecls(), CaptureDiagnostics, *RemappedFiles.get(),
+      CXXIdx->getClangResourcesPath(), CXXIdx->getStorePreamblesInMemory(),
+      CXXIdx->getPreambleStoragePath(), CXXIdx->getOnlyLocalDecls(),
+      CaptureDiagnostics, *RemappedFiles.get(),
       /*RemappedFilesKeepOriginalName=*/true, PrecompilePreambleAfterNParses,
       TUKind, CacheCodeCompletionResults, IncludeBriefCommentsInCodeCompletion,
       /*AllowPCHWithCompilerErrors=*/true, SkipFunctionBodies, SingleFileParse,
       /*UserFilesAreVolatile=*/true, ForSerialization, RetainExcludedCB,
       CXXIdx->getPCHContainerOperations()->getRawReader().getFormat(),
       &ErrUnit));
 
   // Early failures in LoadFromCommandLine may return with ErrUnit unset.

clang/tools/libclang/CIndexer.h

 class MacroDefinitionRecord;
 class SourceLocation;
 class Token;
 class IdentifierInfo;
 
 class CIndexer {
   bool OnlyLocalDecls;
   bool DisplayDiagnostics;
+  bool StorePreamblesInMemory = false;
   unsigned Options; // CXGlobalOptFlags.
 
   std::string ResourcesPath;
   std::shared_ptr<PCHContainerOperations> PCHContainerOps;
 
   std::string ToolchainPath;
 
   std::string PreambleStoragePath;
bool isOptEnabled(CXGlobalOptFlags opt) const {
     return Options & opt;
   }
 
   /// Get the path of the clang resource files.
   const std::string &getClangResourcesPath();
 
   StringRef getClangToolchainPath();
 
+  void setStorePreamblesInMemory(bool StoreInMemory) {
+    StorePreamblesInMemory = StoreInMemory;
+  }
+  bool getStorePreamblesInMemory() const { return StorePreamblesInMemory; }
+
   void setPreambleStoragePath(StringRef Str) {
     PreambleStoragePath = Str.str();
   }
 
   StringRef getPreambleStoragePath() const { return PreambleStoragePath; }
 
   void setInvocationEmissionPath(StringRef Str) {
     InvocationEmissionPath = std::string(Str);

clang/unittests/Frontend/ASTUnitTest.cpp

TEST_F(ASTUnitTest, LoadFromCommandLineEarlyError) {
 
   const char *Args[] = {"clang", "-target", "foobar", InputFileName.c_str()};
 
   auto Diags = CompilerInstance::createDiagnostics(new DiagnosticOptions());
   auto PCHContainerOps = std::make_shared<PCHContainerOperations>();
   std::unique_ptr<clang::ASTUnit> ErrUnit;
 
   ASTUnit *AST = ASTUnit::LoadFromCommandLine(
-      &Args[0], &Args[4], PCHContainerOps, Diags, "", "", false,
+      &Args[0], &Args[4], PCHContainerOps, Diags, "", false, "", false,
       CaptureDiagsKind::All, std::nullopt, true, 0, TU_Complete, false, false,
       false, SkipFunctionBodiesScope::None, false, true, false, false,
       std::nullopt, &ErrUnit, nullptr);
 
   ASSERT_EQ(AST, nullptr);
   ASSERT_NE(ErrUnit, nullptr);
   ASSERT_TRUE(Diags->hasErrorOccurred());
   ASSERT_NE(ErrUnit->stored_diag_size(), 0U);
 }
 
 } // anonymous namespace

clang/unittests/libclang/LibclangTest.cpp

 protected:
   void CreateIndex() override {
     InitializePreambleDir();
     LibclangPreambleStorageTest::CreateIndex();
   }
 };
 
 class LibclangSetPreambleStoragePathTest : public LibclangPreambleStorageTest {
+  virtual bool StorePreamblesInMemory() { return false; }
   virtual const char *PreambleStoragePath() = 0;
 
 protected:
   void CreateIndex() override {
     InitializePreambleDir();
 
     CXIndexOptions Opts{};
     Opts.Size = sizeof(CXIndexOptions);
+    Opts.StorePreamblesInMemory = StorePreamblesInMemory();
     Opts.PreambleStoragePath = PreambleStoragePath();
     Index = clang_createIndexWithOptions(&Opts);
     ASSERT_TRUE(Index);
   }
 };
 
 class LibclangNullPreambleStoragePathTest
     : public LibclangSetPreambleStoragePathTest {
   const char *PreambleStoragePath() override { return nullptr; }
 };
 class LibclangEmptyPreambleStoragePathTest
     : public LibclangSetPreambleStoragePathTest {
   const char *PreambleStoragePath() override { return ""; }
 };
 class LibclangPreambleDirPreambleStoragePathTest
     : public LibclangSetPreambleStoragePathTest {
   const char *PreambleStoragePath() override { return PreambleDir.c_str(); }
 };
 
+class LibclangStoreInMemoryNullPreambleStoragePathTest
+    : public LibclangNullPreambleStoragePathTest {
+  bool StorePreamblesInMemory() override { return true; }
+};
+class LibclangStoreInMemoryEmptyPreambleStoragePathTest
+    : public LibclangEmptyPreambleStoragePathTest {
+  bool StorePreamblesInMemory() override { return true; }
+};
+class LibclangStoreInMemoryPreambleDirPreambleStoragePathTest
+    : public LibclangPreambleDirPreambleStoragePathTest {
+  bool StorePreamblesInMemory() override { return true; }
+};
+
 TEST_F(LibclangNotOverriddenPreambleStoragePathTest, CountPreambles) {
   CountPreamblesInPreambleDir(0);
 }
 TEST_F(LibclangNullPreambleStoragePathTest, CountPreambles) {
   CountPreamblesInPreambleDir(0);
 }
 TEST_F(LibclangEmptyPreambleStoragePathTest, CountPreambles) {
   CountPreamblesInPreambleDir(0);
 }
 TEST_F(LibclangPreambleDirPreambleStoragePathTest, CountPreambles) {
   CountPreamblesInPreambleDir(1);
 }
+TEST_F(LibclangStoreInMemoryNullPreambleStoragePathTest, CountPreambles) {
+  CountPreamblesInPreambleDir(0);
+}
+TEST_F(LibclangStoreInMemoryEmptyPreambleStoragePathTest, CountPreambles) {
+  CountPreamblesInPreambleDir(0);
+}
+TEST_F(LibclangStoreInMemoryPreambleDirPreambleStoragePathTest,
+       CountPreambles) {
+  CountPreamblesInPreambleDir(0);
+}
 
 TEST_F(LibclangParseTest, AllSkippedRanges) {
   std::string Header = "header.h", Main = "main.cpp";
   WriteFile(Header,
     "#ifdef MANGOS\n"
     "printf(\"mmm\");\n"
     "#endif");
   WriteFile(Main,