[clangd] Don't index __reserved_names in headers.

[sammccall]

Summary

Main use of these is in the standard library, where they generally clutter up
the index.

Certain macros are also common, we don't touch indexing of macros in this patch.

[kadircet]

thanks!

clang-tools-extra/clangd/SourceCode.h

334

nit: maybe move definition out-of-line.

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

60

can we still have a test that indexes main file symbols and ensures ReservedName is set correctly?

[sammccall]

clang-tools-extra/clangd/SourceCode.h

334

This "just a few instructions", I'd rather let the compiler make that call...

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

60

Whoops, of course.

clang-tools-extra/clangd/AST.h

 /// Print the Objective-C method name, including the full container name, e.g.
 /// `-[MyClass(Category) method:]`
 std::string printObjCMethod(const ObjCMethodDecl &Method);
 
 /// Print the Objective-C container name including categories, e.g. `MyClass`,
 // `MyClass()`, `MyClass(Category)`, and `MyProtocol`.
 std::string printObjCContainer(const ObjCContainerDecl &C);
 
+/// Returns true if this is a NamedDecl with a reserved name.
+bool hasReservedName(const Decl &);
+/// Returns true if this scope would be written with a reserved name.
+/// This does not include unwritten scope elements like __1 in std::__1::vector.
+bool hasReservedScope(const DeclContext &);
+
 /// Gets the symbol ID for a declaration. Returned SymbolID might be null.
 SymbolID getSymbolID(const Decl *D);
 
 /// Gets the symbol ID for a macro. Returned SymbolID might be null.
 /// Currently, this is an encoded USR of the macro, which incorporates macro
 /// locations (e.g. file name, offset in file).
 /// FIXME: the USR semantics might not be stable enough as the ID for index
 /// macro (e.g. a change in definition offset can result in a different USR). We

clang-tools-extra/clangd/AST.cpp

std::string printType(const QualType QT, const DeclContext &CurContext) {
   };
   PrintCB PCB(&CurContext);
   PP.Callbacks = &PCB;
 
   QT.print(OS, PP);
   return OS.str();
 }
 
+bool hasReservedName(const Decl &D) {
+  if (const auto *ND = llvm::dyn_cast<NamedDecl>(&D))
+    if (const auto *II = ND->getIdentifier())
+      return isReservedName(II->getName());
+  return false;
+}
+
+bool hasReservedScope(const DeclContext &DC) {
+  for (const DeclContext *D = &DC; D; D = D->getParent()) {
+    if (D->isTransparentContext() || D->isInlineNamespace())
+      continue;
+    if (const auto *ND = llvm::dyn_cast<NamedDecl>(D))
+      if (hasReservedName(*ND))
+        return true;
+  }
+  return false;
+}
+
 QualType declaredType(const TypeDecl *D) {
   if (const auto *CTSD = llvm::dyn_cast<ClassTemplateSpecializationDecl>(D))
     if (const auto *TSI = CTSD->getTypeAsWritten())
       return TSI->getType();
   return D->getASTContext().getTypeDeclType(D);
 }
 
 namespace {

clang-tools-extra/clangd/Quality.cpp

 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclVisitor.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cmath>
 
 namespace clang {
 namespace clangd {
-static bool isReserved(llvm::StringRef Name) {
-  // FIXME: Should we exclude _Bool and others recognized by the standard?
-  return Name.size() >= 2 && Name[0] == '_' &&
-         (isUppercase(Name[1]) || Name[1] == '_');
-}
 
 static bool hasDeclInMainFile(const Decl &D) {
   auto &SourceMgr = D.getASTContext().getSourceManager();
   for (auto *Redecl : D.redecls()) {
     if (isInsideMainFile(Redecl->getLocation(), SourceMgr))
       return true;
   }
   return false;

 
 void SymbolQualitySignals::merge(const CodeCompletionResult &SemaCCResult) {
   Deprecated |= (SemaCCResult.Availability == CXAvailability_Deprecated);
   Category = categorize(SemaCCResult);
 
   if (SemaCCResult.Declaration) {
     ImplementationDetail |= isImplementationDetail(SemaCCResult.Declaration);
     if (auto *ID = SemaCCResult.Declaration->getIdentifier())
-      ReservedName = ReservedName || isReserved(ID->getName());
+      ReservedName = ReservedName || isReservedName(ID->getName());
   } else if (SemaCCResult.Kind == CodeCompletionResult::RK_Macro)
-    ReservedName = ReservedName || isReserved(SemaCCResult.Macro->getName());
+    ReservedName =
+        ReservedName || isReservedName(SemaCCResult.Macro->getName());
 }
 
 void SymbolQualitySignals::merge(const Symbol &IndexResult) {
   Deprecated |= (IndexResult.Flags & Symbol::Deprecated);
   ImplementationDetail |= (IndexResult.Flags & Symbol::ImplementationDetail);
   References = std::max(IndexResult.References, References);
   Category = categorize(IndexResult.SymInfo);
-  ReservedName = ReservedName || isReserved(IndexResult.Name);
+  ReservedName = ReservedName || isReservedName(IndexResult.Name);
 }
 
 float SymbolQualitySignals::evaluateHeuristics() const {
   float Score = 1;
 
   // This avoids a sharp gradient for tail symbols, and also neatly avoids the
   // question of whether 0 references means a bad symbol or missing data.
   if (References >= 10) {

clang-tools-extra/clangd/SourceCode.h

 //
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
 
 #include "Protocol.h"
 #include "support/Context.h"
 #include "support/ThreadsafeFS.h"
+#include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
-#include "llvm/Support/SHA1.h"
 #include <string>
 
 namespace clang {
 class SourceManager;
 
 namespace clangd {
 
 // We tend to generate digests for source codes in a lot of different places.

 /// Returns true if the given location is in a generated protobuf file.
 bool isProtoFile(SourceLocation Loc, const SourceManager &SourceMgr);
 
 /// This scans source code, and should not be called when using a preamble.
 /// Prefer to access the cache in IncludeStructure::isSelfContained if you can.
 bool isSelfContainedHeader(const FileEntry *FE, FileID ID,
                            const SourceManager &SM, HeaderSearch &HeaderInfo);
 
+/// Returns true if Name is reserved, like _Foo or __Vector_base.
+inline bool isReservedName(llvm::StringRef Name) {
+  // This doesn't catch all cases, but the most common.
+  return Name.size() >= 2 && Name[0] == '_' &&
+         (isUppercase(Name[1]) || Name[1] == '_');
+}
+
 } // namespace clangd
 } // namespace clang
 #endif

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

SlabTuple indexSymbols(ASTContext &AST, Preprocessor &PP,
                        const CanonicalIncludes &Includes, bool IsIndexMainAST,
                        llvm::StringRef Version, bool CollectMainFileRefs) {
   SymbolCollector::Options CollectorOpts;
   CollectorOpts.CollectIncludePath = true;
   CollectorOpts.Includes = &Includes;
   CollectorOpts.CountReferences = false;
   CollectorOpts.Origin = SymbolOrigin::Dynamic;
   CollectorOpts.CollectMainFileRefs = CollectMainFileRefs;
+  // We want stdlib implementation details in the index only if we've opened the
+  // file in question. This does means xrefs won't work, though.
+  CollectorOpts.CollectReserved = IsIndexMainAST;
 
   index::IndexingOptions IndexOpts;
   // We only need declarations, because we don't count references.
   IndexOpts.SystemSymbolFilter =
       index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly;
   // We index function-local classes and its member functions only.
   IndexOpts.IndexFunctionLocals = true;
   if (IsIndexMainAST) {

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

struct Options {
     /// macro even if this is true.
     bool CollectMacro = false;
     /// Collect symbols local to main-files, such as static functions, symbols
     /// inside an anonymous namespace, function-local classes and its member
     /// functions.
     bool CollectMainFileSymbols = true;
     /// Collect references to main-file symbols.
     bool CollectMainFileRefs = false;
+    /// Collect symbols with reserved names, like __Vector_base.
+    /// This does not currently affect macros (many like _WIN32 are important!)
+    bool CollectReserved = false;
     /// If set to true, SymbolCollector will collect doc for all symbols.
     /// Note that documents of symbols being indexed for completion will always
     /// be collected regardless of this option.
     bool StoreAllDocumentation = false;
     /// If this is set, only collect symbols/references from a file if
     /// `FileFilter(SM, FID)` is true. If not set, all files are indexed.
     std::function<bool(const SourceManager &, FileID)> FileFilter = nullptr;
   };

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

default:
     // easier to cast.
     if (!isa<RecordDecl>(DeclCtx))
       return false;
   }
 
   // Avoid indexing internal symbols in protobuf generated headers.
   if (isPrivateProtoDecl(ND))
     return false;
+  if (!Opts.CollectReserved &&
+      (hasReservedName(ND) || hasReservedScope(*ND.getDeclContext())))
+    return false;
+
   return true;
 }
 
 // Always return true to continue indexing.
 bool SymbolCollector::handleDeclOccurrence(
     const Decl *D, index::SymbolRoleSet Roles,
     llvm::ArrayRef<index::SymbolRelation> Relations, SourceLocation Loc,
     index::IndexDataConsumer::ASTNodeInfo ASTNode) {

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

TEST(ClangdAST, GetAttributes) {
   Stmt *FooBody = cast<FunctionDecl>(findDecl(AST, "foo")).getBody();
   IfStmt *FooIf = cast<IfStmt>(cast<CompoundStmt>(FooBody)->body_front());
   ASSERT_THAT(getAttributes(DynTypedNode::create(*FooIf)),
               Each(implicitAttr()));
   ASSERT_THAT(getAttributes(DynTypedNode::create(*FooIf->getThen())),
               Contains(attrKind(attr::Unlikely)));
 }
 
+TEST(ClangdAST, HasReservedName) {
+  ParsedAST AST = TestTU::withCode(R"cpp(
+    void __foo();
+    namespace std {
+      inline namespace __1 { class error_code; }
+      namespace __detail { int secret; }
+    }
+  )cpp")
+                      .build();
+
+  EXPECT_TRUE(hasReservedName(findUnqualifiedDecl(AST, "__foo")));
+  EXPECT_FALSE(
+      hasReservedScope(*findUnqualifiedDecl(AST, "__foo").getDeclContext()));
+
+  EXPECT_FALSE(hasReservedName(findUnqualifiedDecl(AST, "error_code")));
+  EXPECT_FALSE(hasReservedScope(
+      *findUnqualifiedDecl(AST, "error_code").getDeclContext()));
+
+  EXPECT_FALSE(hasReservedName(findUnqualifiedDecl(AST, "secret")));
+  EXPECT_TRUE(
+      hasReservedScope(*findUnqualifiedDecl(AST, "secret").getDeclContext()));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

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

auto Header = TestTU::withHeaderCode(R"cpp(
     #define DECL(x, y) class DECL_NAME(x, y) {};
     DECL(X, Y); // X_Y_Decl
   )cpp");
 
   auto Symbols = Header.headerSymbols();
   auto AST = Header.build();
 
   SymbolQualitySignals Quality;
-  Quality.merge(findSymbol(Symbols, "_X"));
-  EXPECT_FALSE(Quality.Deprecated);
-  EXPECT_FALSE(Quality.ImplementationDetail);
-  EXPECT_TRUE(Quality.ReservedName);
-  EXPECT_EQ(Quality.References, SymbolQualitySignals().References);
-  EXPECT_EQ(Quality.Category, SymbolQualitySignals::Variable);
-
   Quality.merge(findSymbol(Symbols, "X_Y_Decl"));
   EXPECT_TRUE(Quality.ImplementationDetail);
 
   Symbol F = findSymbol(Symbols, "_f");
   F.References = 24; // TestTU doesn't count references, so fake it.
   Quality = {};
   Quality.merge(F);
   EXPECT_TRUE(Quality.Deprecated);
   EXPECT_FALSE(Quality.ReservedName);
   EXPECT_EQ(Quality.References, 24u);
   EXPECT_EQ(Quality.Category, SymbolQualitySignals::Function);
 
   Quality = {};
   Quality.merge(CodeCompletionResult(&findDecl(AST, "_f"), /*Priority=*/42));
   EXPECT_TRUE(Quality.Deprecated);
   EXPECT_FALSE(Quality.ReservedName);
   EXPECT_EQ(Quality.References, SymbolQualitySignals().References);
   EXPECT_EQ(Quality.Category, SymbolQualitySignals::Function);
 
   Quality = {};
   Quality.merge(CodeCompletionResult("if"));
   EXPECT_EQ(Quality.Category, SymbolQualitySignals::Keyword);
+
+  // Testing ReservedName in main file, we don't index those symbols in headers.
+  auto MainAST = TestTU::withCode("int _X;").build();
+  SymbolSlab MainSymbols = std::get<0>(indexMainDecls(MainAST));
+
+  Quality = {};
+  Quality.merge(findSymbol(MainSymbols, "_X"));
+  EXPECT_FALSE(Quality.Deprecated);
+  EXPECT_FALSE(Quality.ImplementationDetail);
+  EXPECT_TRUE(Quality.ReservedName);
 }
 
 TEST(QualityTests, SymbolRelevanceSignalExtraction) {
   TestTU Test;
   Test.HeaderCode = R"cpp(
   int header();
   int header_main();
 

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

)cpp");
   // Check all positions mentioned in the test return valid results.
   for (auto P : Source.points()) {
     size_t Offset = llvm::cantFail(positionToOffset(Source.code(), P));
     EXPECT_THAT_EXPECTED(sourceLocationInMainFile(SM, P),
                          HasValue(StartOfFile.getLocWithOffset(Offset)));
   }
 }
 
+TEST(SourceCodeTests, isReservedName) {
+  EXPECT_FALSE(isReservedName(""));
+  EXPECT_FALSE(isReservedName("_"));
+  EXPECT_FALSE(isReservedName("foo"));
+  EXPECT_FALSE(isReservedName("_foo"));
+  EXPECT_TRUE(isReservedName("__foo"));
+  EXPECT_TRUE(isReservedName("_Foo"));
+  EXPECT_FALSE(isReservedName("foo__bar")) << "FIXME";
+}
+
 TEST(SourceCodeTests, CollectIdentifiers) {
   auto Style = format::getLLVMStyle();
   auto IDs = collectIdentifiers(R"cpp(
   #include "a.h"
   void foo() { int xyz; int abc = xyz; return foo(); }
   )cpp",
                                 Style);
   EXPECT_EQ(IDs.size(), 7u);

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

 #include "index/SymbolCollector.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Index/IndexingOptions.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "gmock/gmock-matchers.h"
 #include "gmock/gmock-more-matchers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
TEST_F(SymbolCollectorTest, NoCrashOnObjCMethodCStyleParam) {
   TU.ExtraArgs.push_back("-xobjective-c++");
 
   TU.build();
   // We mostly care about not crashing.
   EXPECT_THAT(TU.headerSymbols(),
               UnorderedElementsAre(QName("Foo"), QName("Foo::fun:")));
 }
 
+TEST_F(SymbolCollectorTest, Reserved) {
+  const char *Header = R"cpp(
+    void __foo();
+    namespace _X { int secret; }
+  )cpp";
+
+  CollectorOpts.CollectReserved = true;
+  runSymbolCollector("", Header);
+  EXPECT_THAT(Symbols, UnorderedElementsAre(QName("__foo"), QName("_X"),
+                                            QName("_X::secret")));
+
+  CollectorOpts.CollectReserved = false;
+  runSymbolCollector("", Header); //
+  EXPECT_THAT(Symbols, IsEmpty());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang