Merge branch 'main' into stmt-attributes

Author: ericastor

.github/workflows/release-binaries-all.yml

@@ -43,6 +43,7 @@ on:
       - '.github/workflows/release-binaries.yml'
       - '.github/workflows/release-binaries-setup-stage/*'
       - '.github/workflows/release-binaries-save-stage/*'
+      - 'clang/cmake/caches/Release.cmake'
 
 concurrency:
   group: ${{ github.workflow }}-${{ github.event.pull_request.number || 'dispatch' }}

clang-tools-extra/clangd/CodeComplete.cpp

@@ -1409,6 +1409,9 @@ bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
   Clang->getPreprocessorOpts().SingleFileParseMode = CompletingInPreamble;
   Clang->setCodeCompletionConsumer(Consumer.release());
 
+  if (Input.Preamble.RequiredModules)
+    Input.Preamble.RequiredModules->adjustHeaderSearchOptions(Clang->getHeaderSearchOpts());
+
   SyntaxOnlyAction Action;
   if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])) {
     log("BeginSourceFile() failed when running codeComplete for {0}",
@@ -2122,7 +2125,7 @@ clang::CodeCompleteOptions CodeCompleteOptions::getClangCompleteOpts() const {
   // When an is used, Sema is responsible for completing the main file,
   // the index can provide results from the preamble.
   // Tell Sema not to deserialize the preamble to look for results.
-  Result.LoadExternal = !Index;
+  Result.LoadExternal = ForceLoadPreamble || !Index;
   Result.IncludeFixIts = IncludeFixIts;
 
   return Result;

clang-tools-extra/clangd/CodeComplete.h

@@ -52,6 +52,11 @@ struct CodeCompleteOptions {
   /// For example, private members are usually inaccessible.
   bool IncludeIneligibleResults = false;
 
+  /// Force sema to load decls from preamble even if an index is provided.
+  /// This is helpful for cases the index can't provide symbols, e.g. with
+  /// experimental c++20 modules
+  bool ForceLoadPreamble = false;
+
   /// Combine overloads into a single completion item where possible.
   /// If none, the implementation may choose an appropriate behavior.
   /// (In practice, ClangdLSPServer enables bundling if the client claims

clang-tools-extra/clangd/index/SymbolCollector.cpp

@@ -41,6 +41,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -75,18 +76,62 @@ bool isPrivateProtoDecl(const NamedDecl &ND) {
   if (ND.getIdentifier() == nullptr)
     return false;
   auto Name = ND.getIdentifier()->getName();
-  if (!Name.contains('_'))
-    return false;
-  // Nested proto entities (e.g. Message::Nested) have top-level decls
-  // that shouldn't be used (Message_Nested). Ignore them completely.
-  // The nested entities are dangling type aliases, we may want to reconsider
-  // including them in the future.
-  // For enum constants, SOME_ENUM_CONSTANT is not private and should be
-  // indexed. Outer_INNER is private. This heuristic relies on naming style, it
-  // will include OUTER_INNER and exclude some_enum_constant.
-  // FIXME: the heuristic relies on naming style (i.e. no underscore in
-  // user-defined names) and can be improved.
-  return (ND.getKind() != Decl::EnumConstant) || llvm::any_of(Name, islower);
+  // There are some internal helpers like _internal_set_foo();
+  if (Name.contains("_internal_"))
+    return true;
+
+  // https://protobuf.dev/reference/cpp/cpp-generated/#nested-types
+  // Nested entities (messages/enums) has two names, one at the top-level scope,
+  // with a mangled name created by prepending all the outer types. These names
+  // are almost never preferred by the developers, so exclude them from index.
+  // e.g.
+  //   message Foo {
+  //    message Bar {}
+  //    enum E { A }
+  //   }
+  //
+  // yields:
+  //   class Foo_Bar {};
+  //   enum Foo_E { Foo_E_A };
+  //   class Foo {
+  //    using Bar = Foo_Bar;
+  //    static constexpr Foo_E A = Foo_E_A;
+  //   };
+
+  // We get rid of Foo_Bar and Foo_E by discarding any top-level entries with
+  // `_` in the name. This relies on original message/enum not having `_` in the
+  // name. Hence might go wrong in certain cases.
+  if (ND.getDeclContext()->isNamespace()) {
+    // Strip off some known public suffix helpers for enums, rest of the helpers
+    // are generated inside record decls so we don't care.
+    // https://protobuf.dev/reference/cpp/cpp-generated/#enum
+    Name.consume_back("_descriptor");
+    Name.consume_back("_IsValid");
+    Name.consume_back("_Name");
+    Name.consume_back("_Parse");
+    Name.consume_back("_MIN");
+    Name.consume_back("_MAX");
+    Name.consume_back("_ARRAYSIZE");
+    return Name.contains('_');
+  }
+
+  // EnumConstantDecls need some special attention, despite being nested in a
+  // TagDecl, they might still have mangled names. We filter those by checking
+  // if it has parent's name as a prefix.
+  // This might go wrong if a nested entity has a name that starts with parent's
+  // name, e.g: enum Foo { Foo_X }.
+  if (llvm::isa<EnumConstantDecl>(&ND)) {
+    auto *DC = llvm::cast<EnumDecl>(ND.getDeclContext());
+    if (!DC || !DC->getIdentifier())
+      return false;
+    auto CtxName = DC->getIdentifier()->getName();
+    return !CtxName.empty() && Name.consume_front(CtxName) &&
+           Name.consume_front("_");
+  }
+
+  // Now we're only left with fields/methods without an `_internal_` in the
+  // name, they're intended for public use.
+  return false;
 }
 
 // We only collect #include paths for symbols that are suitable for global code

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

@@ -919,6 +919,9 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
   Opts.CodeComplete.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
   Opts.CodeComplete.RunParser = CodeCompletionParse;
   Opts.CodeComplete.RankingModel = RankingModel;
+  // FIXME: If we're using C++20 modules, force the lookup process to load
+  // external decls, since currently the index doesn't support C++20 modules.
+  Opts.CodeComplete.ForceLoadPreamble = ExperimentalModulesSupport;
 
   RealThreadsafeFS TFS;
   std::vector<std::unique_ptr<config::Provider>> ProviderStack;

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

@@ -402,6 +402,86 @@ import A;
   EXPECT_TRUE(D.isFromASTFile());
 }
 
+// An end to end test for code complete in modules
+TEST_F(PrerequisiteModulesTests, CodeCompleteTest) {
+  MockDirectoryCompilationDatabase CDB(TestDir, FS);
+
+  CDB.addFile("A.cppm", R"cpp(
+export module A;
+export void printA();
+  )cpp");
+
+  llvm::StringLiteral UserContents = R"cpp(
+import A;
+void func() {
+  print^
+}
+)cpp";
+
+  CDB.addFile("Use.cpp", UserContents);
+  Annotations Test(UserContents);
+
+  ModulesBuilder Builder(CDB);
+
+  ParseInputs Use = getInputs("Use.cpp", CDB);
+  Use.ModulesManager = &Builder;
+
+  std::unique_ptr<CompilerInvocation> CI =
+      buildCompilerInvocation(Use, DiagConsumer);
+  EXPECT_TRUE(CI);
+
+  auto Preamble =
+      buildPreamble(getFullPath("Use.cpp"), *CI, Use, /*InMemory=*/true,
+                    /*Callback=*/nullptr);
+  EXPECT_TRUE(Preamble);
+  EXPECT_TRUE(Preamble->RequiredModules);
+  
+  auto Result = codeComplete(getFullPath("Use.cpp"), Test.point(),
+                             Preamble.get(), Use, {});
+  EXPECT_FALSE(Result.Completions.empty());
+  EXPECT_EQ(Result.Completions[0].Name, "printA");
+}
+
+TEST_F(PrerequisiteModulesTests, SignatureHelpTest) {
+  MockDirectoryCompilationDatabase CDB(TestDir, FS);
+
+  CDB.addFile("A.cppm", R"cpp(
+export module A;
+export void printA(int a);
+  )cpp");
+
+  llvm::StringLiteral UserContents = R"cpp(
+import A;
+void func() {
+  printA(^);
+}
+)cpp";
+
+  CDB.addFile("Use.cpp", UserContents);
+  Annotations Test(UserContents);
+
+  ModulesBuilder Builder(CDB);
+
+  ParseInputs Use = getInputs("Use.cpp", CDB);
+  Use.ModulesManager = &Builder;
+
+  std::unique_ptr<CompilerInvocation> CI =
+      buildCompilerInvocation(Use, DiagConsumer);
+  EXPECT_TRUE(CI);
+
+  auto Preamble =
+      buildPreamble(getFullPath("Use.cpp"), *CI, Use, /*InMemory=*/true,
+                    /*Callback=*/nullptr);
+  EXPECT_TRUE(Preamble);
+  EXPECT_TRUE(Preamble->RequiredModules);
+  
+  auto Result = signatureHelp(getFullPath("Use.cpp"), Test.point(),
+                              *Preamble.get(), Use, MarkupKind::PlainText);
+  EXPECT_FALSE(Result.signatures.empty());
+  EXPECT_EQ(Result.signatures[0].label, "printA(int a) -> void");
+  EXPECT_EQ(Result.signatures[0].parameters[0].labelString, "int a");
+}
+
 } // namespace
 } // namespace clang::clangd
 

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

@@ -201,19 +201,63 @@ TEST_F(ShouldCollectSymbolTest, NoPrivateProtoSymbol) {
   build(
       R"(// Generated by the protocol buffer compiler.  DO NOT EDIT!
          namespace nx {
-           class Top_Level {};
-           class TopLevel {};
-           enum Kind {
-             KIND_OK,
-             Kind_Not_Ok,
+           enum Outer_Enum : int {
+             Outer_Enum_KIND1,
+             Outer_Enum_Kind_2,
            };
+           bool Outer_Enum_IsValid(int);
+
+           class Outer_Inner {};
+           class Outer {
+             using Inner = Outer_Inner;
+             using Enum = Outer_Enum;
+             static constexpr Enum KIND1 = Outer_Enum_KIND1;
+             static constexpr Enum Kind_2 = Outer_Enum_Kind_2;
+             static bool Enum_IsValid(int);
+             int &x();
+             void set_x();
+             void _internal_set_x();
+
+             int &Outer_y();
+           };
+           enum Foo {
+             FOO_VAL1,
+             Foo_VAL2,
+           };
+           bool Foo_IsValid(int);
          })");
-  EXPECT_TRUE(shouldCollect("nx::TopLevel"));
-  EXPECT_TRUE(shouldCollect("nx::Kind::KIND_OK"));
-  EXPECT_TRUE(shouldCollect("nx::Kind"));
 
-  EXPECT_FALSE(shouldCollect("nx::Top_Level"));
-  EXPECT_FALSE(shouldCollect("nx::Kind::Kind_Not_Ok"));
+  // Make sure all the mangled names for Outer::Enum is discarded.
+  EXPECT_FALSE(shouldCollect("nx::Outer_Enum"));
+  EXPECT_FALSE(shouldCollect("nx::Outer_Enum_KIND1"));
+  EXPECT_FALSE(shouldCollect("nx::Outer_Enum_Kind_2"));
+  EXPECT_FALSE(shouldCollect("nx::Outer_Enum_IsValid"));
+  // But nested aliases are preserved.
+  EXPECT_TRUE(shouldCollect("nx::Outer::Enum"));
+  EXPECT_TRUE(shouldCollect("nx::Outer::KIND1"));
+  EXPECT_TRUE(shouldCollect("nx::Outer::Kind_2"));
+  EXPECT_TRUE(shouldCollect("nx::Outer::Enum_IsValid"));
+
+  // Check for Outer::Inner.
+  EXPECT_FALSE(shouldCollect("nx::Outer_Inner"));
+  EXPECT_TRUE(shouldCollect("nx::Outer"));
+  EXPECT_TRUE(shouldCollect("nx::Outer::Inner"));
+
+  // Make sure field related information is preserved, unless it's explicitly
+  // marked with `_internal_`.
+  EXPECT_TRUE(shouldCollect("nx::Outer::x"));
+  EXPECT_TRUE(shouldCollect("nx::Outer::set_x"));
+  EXPECT_FALSE(shouldCollect("nx::Outer::_internal_set_x"));
+  EXPECT_TRUE(shouldCollect("nx::Outer::Outer_y"));
+
+  // Handling of a top-level enum
+  EXPECT_TRUE(shouldCollect("nx::Foo::FOO_VAL1"));
+  EXPECT_TRUE(shouldCollect("nx::FOO_VAL1"));
+  EXPECT_TRUE(shouldCollect("nx::Foo_IsValid"));
+  // Our heuristic goes wrong here, if the user has a nested name that starts
+  // with parent's name.
+  EXPECT_FALSE(shouldCollect("nx::Foo::Foo_VAL2"));
+  EXPECT_FALSE(shouldCollect("nx::Foo_VAL2"));
 }
 
 TEST_F(ShouldCollectSymbolTest, DoubleCheckProtoHeaderComment) {

clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h

@@ -21,6 +21,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include <string>
+#include <utility>
 
 namespace clang {
 class SourceLocation;
@@ -62,7 +63,8 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
 
 struct AnalysisResults {
   std::vector<const Include *> Unused;
-  std::vector<std::string> Missing; // Spellings, like "<vector>"
+  // Spellings, like "<vector>" paired with the Header that generated it.
+  std::vector<std::pair<std::string, Header>> Missing;
 };
 
 /// Determine which headers should be inserted or removed from the main file.

clang-tools-extra/include-cleaner/lib/Analysis.cpp

@@ -26,8 +26,8 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cassert>
@@ -84,7 +84,7 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
   auto &SM = PP.getSourceManager();
   const auto MainFile = *SM.getFileEntryRefForID(SM.getMainFileID());
   llvm::DenseSet<const Include *> Used;
-  llvm::StringSet<> Missing;
+  llvm::StringMap<Header> Missing;
   if (!HeaderFilter)
     HeaderFilter = [](llvm::StringRef) { return false; };
   OptionalDirectoryEntryRef ResourceDir =
@@ -119,7 +119,7 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
                Satisfied = true;
              }
              if (!Satisfied)
-               Missing.insert(std::move(Spelling));
+               Missing.try_emplace(std::move(Spelling), Providers.front());
            });
 
   AnalysisResults Results;
@@ -144,8 +144,8 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
     }
     Results.Unused.push_back(&I);
   }
-  for (llvm::StringRef S : Missing.keys())
-    Results.Missing.push_back(S.str());
+  for (auto &E : Missing)
+    Results.Missing.emplace_back(E.first().str(), E.second);
   llvm::sort(Results.Missing);
   return Results;
 }
@@ -158,9 +158,9 @@ std::string fixIncludes(const AnalysisResults &Results,
   // Encode insertions/deletions in the magic way clang-format understands.
   for (const Include *I : Results.Unused)
     cantFail(R.add(tooling::Replacement(FileName, UINT_MAX, 1, I->quote())));
-  for (llvm::StringRef Spelled : Results.Missing)
-    cantFail(R.add(tooling::Replacement(FileName, UINT_MAX, 0,
-                                        ("#include " + Spelled).str())));
+  for (auto &[Spelled, _] : Results.Missing)
+    cantFail(R.add(
+        tooling::Replacement(FileName, UINT_MAX, 0, "#include " + Spelled)));
   // "cleanup" actually turns the UINT_MAX replacements into concrete edits.
   auto Positioned = cantFail(format::cleanupAroundReplacements(Code, R, Style));
   return cantFail(tooling::applyAllReplacements(Code, Positioned));

clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp

@@ -192,7 +192,7 @@ class Action : public clang::ASTFrontendAction {
       case PrintStyle::Changes:
         for (const Include *I : Results.Unused)
           llvm::outs() << "- " << I->quote() << " @Line:" << I->Line << "\n";
-        for (const auto &I : Results.Missing)
+        for (const auto &[I, _] : Results.Missing)
           llvm::outs() << "+ " << I << "\n";
         break;
       case PrintStyle::Final: