diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -74,6 +74,12 @@ // `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); diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -376,6 +376,24 @@ return OS.str(); } +bool hasReservedName(const Decl &D) { + if (const auto *ND = llvm::dyn_cast(&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(D)) + if (hasReservedName(*ND)) + return true; + } + return false; +} + QualType declaredType(const TypeDecl *D) { if (const auto *CTSD = llvm::dyn_cast(D)) if (const auto *TSI = CTSD->getTypeAsWritten()) diff --git a/clang-tools-extra/clangd/Quality.cpp b/clang-tools-extra/clangd/Quality.cpp --- a/clang-tools-extra/clangd/Quality.cpp +++ b/clang-tools-extra/clangd/Quality.cpp @@ -24,7 +24,6 @@ #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" @@ -35,11 +34,6 @@ 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(); @@ -188,9 +182,10 @@ 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) { @@ -198,7 +193,7 @@ 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 { diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h --- a/clang-tools-extra/clangd/SourceCode.h +++ b/clang-tools-extra/clangd/SourceCode.h @@ -16,6 +16,7 @@ #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" @@ -27,7 +28,6 @@ #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSet.h" #include "llvm/Support/Error.h" -#include "llvm/Support/SHA1.h" #include namespace clang { @@ -330,6 +330,13 @@ 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 diff --git a/clang-tools-extra/clangd/index/FileIndex.cpp b/clang-tools-extra/clangd/index/FileIndex.cpp --- a/clang-tools-extra/clangd/index/FileIndex.cpp +++ b/clang-tools-extra/clangd/index/FileIndex.cpp @@ -56,6 +56,9 @@ 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. diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h --- a/clang-tools-extra/clangd/index/SymbolCollector.h +++ b/clang-tools-extra/clangd/index/SymbolCollector.h @@ -81,6 +81,9 @@ 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. diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -356,6 +356,10 @@ // 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; } diff --git a/clang-tools-extra/clangd/unittests/ASTTests.cpp b/clang-tools-extra/clangd/unittests/ASTTests.cpp --- a/clang-tools-extra/clangd/unittests/ASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ASTTests.cpp @@ -427,6 +427,29 @@ 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 diff --git a/clang-tools-extra/clangd/unittests/QualityTests.cpp b/clang-tools-extra/clangd/unittests/QualityTests.cpp --- a/clang-tools-extra/clangd/unittests/QualityTests.cpp +++ b/clang-tools-extra/clangd/unittests/QualityTests.cpp @@ -54,13 +54,6 @@ 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); @@ -83,6 +76,16 @@ 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) { diff --git a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp --- a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp +++ b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp @@ -315,6 +315,16 @@ } } +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( diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp --- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp +++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp @@ -17,7 +17,6 @@ #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" @@ -1849,6 +1848,22 @@ 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