Index: clang-tools-extra/trunk/clangd/AST.h =================================================================== --- clang-tools-extra/trunk/clangd/AST.h +++ clang-tools-extra/trunk/clangd/AST.h @@ -24,6 +24,11 @@ namespace clangd { +/// Returns true if the declaration is considered implementation detail based on +/// heuristics. For example, a declaration whose name is not explicitly spelled +/// in code is considered implementation detail. +bool isImplementationDetail(const Decl *D); + /// Find the identifier source location of the given D. /// /// The returned location is usually the spelling location where the name of the Index: clang-tools-extra/trunk/clangd/AST.cpp =================================================================== --- clang-tools-extra/trunk/clangd/AST.cpp +++ clang-tools-extra/trunk/clangd/AST.cpp @@ -11,6 +11,7 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" +#include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Index/USRGeneration.h" @@ -18,25 +19,34 @@ namespace clangd { using namespace llvm; -SourceLocation findNameLoc(const clang::Decl* D) { - const auto& SM = D->getASTContext().getSourceManager(); +// Returns true if the complete name of decl \p D is spelled in the source code. +// This is not the case for +// * symbols formed via macro concatenation, the spelling location will +// be "" +// * symbols controlled and defined by a compile command-line option +// `-DName=foo`, the spelling location will be "". +bool isSpelledInSourceCode(const Decl *D) { + const auto &SM = D->getASTContext().getSourceManager(); + auto Loc = D->getLocation(); // FIXME: Revisit the strategy, the heuristic is limitted when handling // macros, we should use the location where the whole definition occurs. - SourceLocation SpellingLoc = SM.getSpellingLoc(D->getLocation()); - if (D->getLocation().isMacroID()) { - std::string PrintLoc = SpellingLoc.printToString(SM); + if (Loc.isMacroID()) { + std::string PrintLoc = SM.getSpellingLoc(Loc).printToString(SM); if (llvm::StringRef(PrintLoc).startswith("")) { - // We use the expansion location for the following symbols, as spelling - // locations of these symbols are not interesting to us: - // * symbols formed via macro concatenation, the spelling location will - // be "" - // * symbols controlled and defined by a compile command-line option - // `-DName=foo`, the spelling location will be "". - SpellingLoc = SM.getExpansionRange(D->getLocation()).getBegin(); - } + llvm::StringRef(PrintLoc).startswith("")) + return false; } - return SpellingLoc; + return true; +} + +bool isImplementationDetail(const Decl *D) { return !isSpelledInSourceCode(D); } + +SourceLocation findNameLoc(const clang::Decl* D) { + const auto &SM = D->getASTContext().getSourceManager(); + if (!isSpelledInSourceCode(D)) + // Use the expansion location as spelling location is not interesting. + return SM.getExpansionRange(D->getLocation()).getBegin(); + return SM.getSpellingLoc(D->getLocation()); } std::string printQualifiedName(const NamedDecl &ND) { Index: clang-tools-extra/trunk/clangd/Quality.h =================================================================== --- clang-tools-extra/trunk/clangd/Quality.h +++ clang-tools-extra/trunk/clangd/Quality.h @@ -57,6 +57,7 @@ bool Deprecated = false; bool ReservedName = false; // __foo, _Foo are usually implementation details. // FIXME: make these findable once user types _. + bool ImplementationDetail = false; unsigned References = 0; enum SymbolCategory { Index: clang-tools-extra/trunk/clangd/Quality.cpp =================================================================== --- clang-tools-extra/trunk/clangd/Quality.cpp +++ clang-tools-extra/trunk/clangd/Quality.cpp @@ -7,6 +7,7 @@ // //===----------------------------------------------------------------------===// #include "Quality.h" +#include "AST.h" #include "FileDistance.h" #include "URI.h" #include "index/Index.h" @@ -177,6 +178,7 @@ Category = categorize(SemaCCResult); if (SemaCCResult.Declaration) { + ImplementationDetail |= isImplementationDetail(SemaCCResult.Declaration); if (auto *ID = SemaCCResult.Declaration->getIdentifier()) ReservedName = ReservedName || isReserved(ID->getName()); } else if (SemaCCResult.Kind == CodeCompletionResult::RK_Macro) @@ -185,6 +187,7 @@ 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); @@ -212,6 +215,8 @@ Score *= 0.1f; if (ReservedName) Score *= 0.1f; + if (ImplementationDetail) + Score *= 0.2f; switch (Category) { case Keyword: // Often relevant, but misses most signals. Index: clang-tools-extra/trunk/clangd/index/Index.h =================================================================== --- clang-tools-extra/trunk/clangd/index/Index.h +++ clang-tools-extra/trunk/clangd/index/Index.h @@ -259,6 +259,8 @@ IndexedForCodeCompletion = 1 << 0, /// Indicates if the symbol is deprecated. Deprecated = 1 << 1, + // Symbol is an implementation detail. + ImplementationDetail = 1 << 2, }; SymbolFlag Flags = SymbolFlag::None; Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp =================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp @@ -536,6 +536,8 @@ if (isIndexedForCodeCompletion(ND, Ctx)) S.Flags |= Symbol::IndexedForCodeCompletion; + if (isImplementationDetail(&ND)) + S.Flags |= Symbol::ImplementationDetail; S.SymInfo = index::getSymbolInfo(&ND); std::string FileURI; if (auto DeclLoc = getTokenLocation(findNameLoc(&ND), SM, Opts, Index: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp @@ -45,17 +45,34 @@ [[deprecated]] int _f() { return _X; } + + #define DECL_NAME(x, y) x##_##y##_Decl + #define DECL(x, y) class DECL_NAME(x, y) {}; + DECL(X, Y); // X_Y_Decl + + class MAC {}; )cpp"); + Header.ExtraArgs = {"-DMAC=mac_name"}; + 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); + + Quality.ImplementationDetail = false; + Quality.merge( + CodeCompletionResult(&findDecl(AST, "mac_name"), /*Priority=*/42)); + EXPECT_TRUE(Quality.ImplementationDetail); + Symbol F = findSymbol(Symbols, "_f"); F.References = 24; // TestTU doesn't count references, so fake it. Quality = {}; @@ -182,6 +199,10 @@ ReservedName.ReservedName = true; EXPECT_LT(ReservedName.evaluate(), Default.evaluate()); + SymbolQualitySignals ImplementationDetail; + ImplementationDetail.ImplementationDetail = true; + EXPECT_LT(ImplementationDetail.evaluate(), Default.evaluate()); + SymbolQualitySignals WithReferences, ManyReferences; WithReferences.References = 20; ManyReferences.References = 1000; Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp @@ -83,6 +83,9 @@ IsIndexedForCodeCompletion; } MATCHER(Deprecated, "") { return arg.Flags & Symbol::Deprecated; } +MATCHER(ImplementationDetail, "") { + return arg.Flags & Symbol::ImplementationDetail; +} MATCHER(RefRange, "") { const Ref &Pos = testing::get<0>(arg); const Range &Range = testing::get<1>(arg); @@ -1037,6 +1040,21 @@ AllOf(QName("TestClangd"), Not(Deprecated())))); } +TEST_F(SymbolCollectorTest, ImplementationDetail) { + const std::string Header = R"( + #define DECL_NAME(x, y) x##_##y##_Decl + #define DECL(x, y) class DECL_NAME(x, y) {}; + DECL(X, Y); // X_Y_Decl + + class Public {}; + )"; + runSymbolCollector(Header, /**/ ""); + EXPECT_THAT(Symbols, + UnorderedElementsAre( + AllOf(QName("X_Y_Decl"), ImplementationDetail()), + AllOf(QName("Public"), Not(ImplementationDetail())))); +} + } // namespace } // namespace clangd } // namespace clang