[clangd] Introduce --skip-preamble-build command line option (#189284)

This option allows to disable preamble optimization in clangd. By
default it's false, but became true for TUs which import modules (and
experimental modules support is enabled).

This PR is a try to address C++20 modules problems described here
https://github.com/llvm/llvm-project/pull/187432
Fixes https://github.com/llvm/llvm-project/issues/181770

clang-tools-extra/clangd/ClangdServer.cpp

@@ -223,6 +223,7 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
       UseDirtyHeaders(Opts.UseDirtyHeaders),
       LineFoldingOnly(Opts.LineFoldingOnly),
       PreambleParseForwardingFunctions(Opts.PreambleParseForwardingFunctions),
+      SkipPreambleBuild(Opts.SkipPreambleBuild),
       ImportInsertions(Opts.ImportInsertions),
       PublishInactiveRegions(Opts.PublishInactiveRegions),
       WorkspaceRoot(Opts.WorkspaceRoot),
@@ -313,6 +314,7 @@ void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
   Inputs.ClangTidyProvider = ClangTidyProvider;
   Inputs.FeatureModules = FeatureModules;
   Inputs.ModulesManager = ModulesManager;
+  adjustParseInputs(Inputs, File);
   bool NewFile = WorkScheduler->update(File, Inputs, WantDiags);
   // If we loaded Foo.h, we want to make sure Foo.cpp is indexed.
   if (NewFile && BackgroundIdx)
@@ -459,6 +461,7 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
         Config::current().Completion.HeaderInsertion;
     CodeCompleteOpts.CodePatterns = Config::current().Completion.CodePatterns;
     CodeCompleteOpts.MacroFilter = Config::current().Completion.MacroFilter;
+    adjustParseInputs(ParseInput, File);
     // FIXME(ibiryukov): even if Preamble is non-null, we may want to check
     // both the old and the new version in case only one of them matches.
     CodeCompleteResult Result = clangd::codeComplete(
@@ -1189,5 +1192,16 @@ void ClangdServer::profile(MemoryTree &MT) const {
     BackgroundIdx->profile(MT.child("background_index"));
   WorkScheduler->profile(MT.child("tuscheduler"));
 }
+
+void ClangdServer::adjustParseInputs(ParseInputs &Inputs, PathRef File) const {
+  // FIXME: Don't perform optimization when the TU requires C++20
+  // named modules. Mixing PCH and modules may cause different issues (incorrect
+  // diagnostics, crashes) due to instability of such scenario support in the
+  // clang.
+  Inputs.Opts.SkipPreambleBuild =
+      SkipPreambleBuild ||
+      (ModulesManager && ModulesManager->hasRequiredModules(File));
+}
+
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/ClangdServer.h

@@ -191,6 +191,9 @@ public:
     // If true, parse emplace-like functions in the preamble.
     bool PreambleParseForwardingFunctions = true;
 
+    // If true, skip preamble build.
+    bool SkipPreambleBuild = false;
+
     /// Whether include fixer insertions for Objective-C code should use #import
     /// instead of #include.
     bool ImportInsertions = false;
@@ -482,6 +485,8 @@ private:
   }
   const ThreadsafeFS &TFS;
 
+  void adjustParseInputs(ParseInputs &Inputs, PathRef File) const;
+
   Path ResourceDir;
   // The index used to look up symbols. This could be:
   //   - null (all index functionality is optional)
@@ -508,6 +513,8 @@ private:
 
   bool PreambleParseForwardingFunctions = true;
 
+  bool SkipPreambleBuild = false;
+
   bool ImportInsertions = false;
 
   bool PublishInactiveRegions = false;

clang-tools-extra/clangd/CodeComplete.cpp

@@ -1420,7 +1420,8 @@ bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
   // overriding the preamble will break sema completion. Fortunately we can just
   // skip all includes in this case; these completions are really simple.
   PreambleBounds PreambleRegion =
-      ComputePreambleBounds(CI->getLangOpts(), *ContentsBuffer, 0);
+      computePreambleBounds(CI->getLangOpts(), *ContentsBuffer,
+                            Input.ParseInput.Opts.SkipPreambleBuild);
   bool CompletingInPreamble = Input.Offset < PreambleRegion.Size ||
                               (!PreambleRegion.PreambleEndsAtStartOfLine &&
                                Input.Offset == PreambleRegion.Size);
@@ -1433,7 +1434,10 @@ bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
   if (Input.Preamble.StatCache)
     VFS = Input.Preamble.StatCache->getConsumingFS(std::move(VFS));
   auto Clang = prepareCompilerInstance(
-      std::move(CI), !CompletingInPreamble ? &Input.Preamble.Preamble : nullptr,
+      std::move(CI),
+      (!CompletingInPreamble && !Input.ParseInput.Opts.SkipPreambleBuild)
+          ? &Input.Preamble.Preamble
+          : nullptr,
       std::move(ContentsBuffer), std::move(VFS), IgnoreDiags);
   Clang->getPreprocessorOpts().SingleFileParseMode = CompletingInPreamble;
   Clang->setCodeCompletionConsumer(Consumer.release());

clang-tools-extra/clangd/Compiler.h

@@ -43,6 +43,8 @@ struct ParseOptions {
   bool PreambleParseForwardingFunctions = true;
 
   bool ImportInsertions = false;
+
+  bool SkipPreambleBuild = false;
 };
 
 /// Information required to run clang, e.g. to parse AST or do code completion.

clang-tools-extra/clangd/ModulesBuilder.cpp

@@ -658,6 +658,16 @@ llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
   return llvm::Error::success();
 }
 
+bool ModulesBuilder::hasRequiredModules(PathRef File) {
+  std::unique_ptr<ProjectModules> MDB = Impl->getCDB().getProjectModules(File);
+  if (!MDB)
+    return false;
+
+  CachingProjectModules CachedMDB(std::move(MDB),
+                                  Impl->getProjectModulesCache());
+  return !CachedMDB.getRequiredModules(File).empty();
+}
+
 std::unique_ptr<PrerequisiteModules>
 ModulesBuilder::buildPrerequisiteModulesFor(PathRef File,
                                             const ThreadsafeFS &TFS) {

clang-tools-extra/clangd/ModulesBuilder.h

@@ -97,6 +97,8 @@ public:
   std::unique_ptr<PrerequisiteModules>
   buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS &TFS);
 
+  bool hasRequiredModules(PathRef File);
+
 private:
   class ModulesBuilderImpl;
   std::unique_ptr<ModulesBuilderImpl> Impl;

clang-tools-extra/clangd/ParsedAST.cpp

@@ -464,7 +464,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
     Patch->apply(*CI);
   }
   auto Clang = prepareCompilerInstance(
-      std::move(CI), PreamblePCH,
+      std::move(CI), Inputs.Opts.SkipPreambleBuild ? nullptr : PreamblePCH,
       llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, Filename), VFS,
       *DiagConsumer);
 

clang-tools-extra/clangd/Preamble.cpp

@@ -320,8 +320,9 @@ struct ScannedPreamble {
 /// running preprocessor over \p Contents. Returned includes do not contain
 /// resolved paths. \p Cmd is used to build the compiler invocation, which might
 /// stat/read files.
-llvm::Expected<ScannedPreamble>
-scanPreamble(llvm::StringRef Contents, const tooling::CompileCommand &Cmd) {
+llvm::Expected<ScannedPreamble> scanPreamble(llvm::StringRef Contents,
+                                             const tooling::CompileCommand &Cmd,
+                                             bool SkipPreambleBuild) {
   class EmptyFS : public ThreadsafeFS {
   private:
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> viewImpl() const override {
@@ -345,7 +346,8 @@ scanPreamble(llvm::StringRef Contents, const tooling::CompileCommand &Cmd) {
   // This means we're scanning (though not preprocessing) the preamble section
   // twice. However, it's important to precisely follow the preamble bounds used
   // elsewhere.
-  auto Bounds = ComputePreambleBounds(CI->getLangOpts(), *ContentsBuffer, 0);
+  auto Bounds = computePreambleBounds(CI->getLangOpts(), *ContentsBuffer,
+                                      SkipPreambleBuild);
   auto PreambleContents = llvm::MemoryBuffer::getMemBufferCopy(
       llvm::StringRef(PI.Contents).take_front(Bounds.Size));
   auto Clang = prepareCompilerInstance(
@@ -576,7 +578,8 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
   // without those.
   auto ContentsBuffer =
       llvm::MemoryBuffer::getMemBuffer(Inputs.Contents, FileName);
-  auto Bounds = ComputePreambleBounds(CI.getLangOpts(), *ContentsBuffer, 0);
+  auto Bounds = computePreambleBounds(CI.getLangOpts(), *ContentsBuffer,
+                                      Inputs.Opts.SkipPreambleBuild);
 
   trace::Span Tracer("BuildPreamble");
   SPAN_ATTACH(Tracer, "File", FileName);
@@ -722,7 +725,8 @@ bool isPreambleCompatible(const PreambleData &Preamble,
                           const CompilerInvocation &CI) {
   auto ContentsBuffer =
       llvm::MemoryBuffer::getMemBuffer(Inputs.Contents, FileName);
-  auto Bounds = ComputePreambleBounds(CI.getLangOpts(), *ContentsBuffer, 0);
+  auto Bounds = computePreambleBounds(CI.getLangOpts(), *ContentsBuffer,
+                                      Inputs.Opts.SkipPreambleBuild);
   auto VFS = Inputs.TFS->view(Inputs.CompileCommand.Directory);
   return compileCommandsAreEqual(Inputs.CompileCommand,
                                  Preamble.CompileCommand) &&
@@ -785,13 +789,15 @@ PreamblePatch PreamblePatch::create(llvm::StringRef FileName,
   // - If scanning for Modified fails, cannot figure out newly added ones so
   //   there's nothing to do but generate an empty patch.
   auto BaselineScan =
-      scanPreamble(Baseline.Preamble.getContents(), Modified.CompileCommand);
+      scanPreamble(Baseline.Preamble.getContents(), Modified.CompileCommand,
+                   Modified.Opts.SkipPreambleBuild);
   if (!BaselineScan) {
     elog("Failed to scan baseline of {0}: {1}", FileName,
          BaselineScan.takeError());
     return PreamblePatch::unmodified(Baseline);
   }
-  auto ModifiedScan = scanPreamble(Modified.Contents, Modified.CompileCommand);
+  auto ModifiedScan = scanPreamble(Modified.Contents, Modified.CompileCommand,
+                                   Modified.Opts.SkipPreambleBuild);
   if (!ModifiedScan) {
     elog("Failed to scan modified contents of {0}: {1}", FileName,
          ModifiedScan.takeError());
@@ -957,5 +963,12 @@ OptionalFileEntryRef PreamblePatch::getPatchEntry(llvm::StringRef MainFilePath,
   auto PatchFilePath = getPatchName(MainFilePath);
   return SM.getFileManager().getOptionalFileRef(PatchFilePath);
 }
+
+PreambleBounds computePreambleBounds(const LangOptions &LangOpts,
+                                     const llvm::MemoryBufferRef &Buffer,
+                                     bool SkipPreambleBuild) {
+  return SkipPreambleBuild ? PreambleBounds(0, false)
+                           : ComputePreambleBounds(LangOpts, Buffer, 0);
+}
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/Preamble.h

@@ -239,6 +239,10 @@ private:
   MainFileMacros PatchedMacros;
 };
 
+PreambleBounds computePreambleBounds(const LangOptions &LangOpts,
+                                     const llvm::MemoryBufferRef &Buffer,
+                                     bool SkipPreambleBuild);
+
 } // namespace clangd
 } // namespace clang
 

clang-tools-extra/clangd/tool/ClangdMain.cpp

@@ -525,6 +525,14 @@ opt<bool> PreambleParseForwardingFunctions{
     init(ParseOptions().PreambleParseForwardingFunctions),
 };
 
+opt<bool> SkipPreambleBuild{
+    "skip-preamble-build",
+    cat(Misc),
+    desc("If ture, skip preamble build"),
+    Hidden,
+    init(ParseOptions().SkipPreambleBuild),
+};
+
 #if defined(__GLIBC__) && CLANGD_MALLOC_TRIM
 opt<bool> EnableMallocTrim{
     "malloc-trim",
@@ -1005,6 +1013,7 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
   }
   Opts.UseDirtyHeaders = UseDirtyHeaders;
   Opts.PreambleParseForwardingFunctions = PreambleParseForwardingFunctions;
+  Opts.SkipPreambleBuild = SkipPreambleBuild;
   Opts.ImportInsertions = ImportInsertions;
   Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
   Opts.TweakFilter = [&](const Tweak &T) {

clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp

@@ -28,6 +28,8 @@
 namespace clang::clangd {
 namespace {
 
+MATCHER_P(named, Name, "") { return arg.Name == Name; }
+
 class GlobalScanningCounterProjectModules : public ProjectModules {
 public:
   GlobalScanningCounterProjectModules(
@@ -830,6 +832,56 @@ int use() { return m_value; }
       << "\nRelative path used: " << RelativeBMPath;
 }
 
+TEST_F(PrerequisiteModulesTests, ModuleImportThroughInclude) {
+  MockDirectoryCompilationDatabase CDB(TestDir, FS);
+
+  Annotations UseCpp(R"cpp(
+#include "Header.hpp"
+void use() {
+  TypeFrom^Module t1;
+  TypeFromHeader t2;
+}
+)cpp");
+
+  CDB.addFile("M.cppm", R"cpp(
+export module M;
+export struct TypeFromModule {};
+)cpp");
+
+  CDB.addFile("Header.hpp", R"cpp(
+import M;
+struct TypeFromHeader {};
+)cpp");
+
+  CDB.addFile("Use.cpp", UseCpp.code());
+
+  ModulesBuilder Builder(CDB);
+
+  auto Inputs = getInputs("Use.cpp", CDB);
+  Inputs.ModulesManager = &Builder;
+  Inputs.Opts.SkipPreambleBuild = true;
+
+  auto CI = buildCompilerInvocation(Inputs, DiagConsumer);
+  ASSERT_TRUE(CI);
+
+  auto Preamble =
+      buildPreamble(getFullPath("Use.cpp"), *CI, Inputs, /*StoreInMemory=*/true,
+                    /*PeambleCallback=*/nullptr);
+  ASSERT_TRUE(Preamble);
+  EXPECT_EQ(Preamble->Preamble.getBounds().Size, 0u);
+
+  auto AST = ParsedAST::build(getFullPath("Use.cpp"), Inputs, std::move(CI), {},
+                              Preamble);
+  ASSERT_TRUE(AST);
+  EXPECT_TRUE(AST->getDiagnostics().empty());
+
+  auto Result = codeComplete(getFullPath("Use.cpp"), UseCpp.point(),
+                             Preamble.get(), Inputs, {});
+  EXPECT_THAT(Result.Completions,
+              testing::UnorderedElementsAre(named("TypeFromModule"),
+                                            named("TypeFromHeader")));
+}
+
 } // namespace
 } // namespace clang::clangd