[clang-tools-extra] r344736 - [clangd] Names that are not spelled in source code are reserved.
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 18 05:23:06 PDT 2018
Author: ioeric
Date: Thu Oct 18 05:23:05 2018
New Revision: 344736
URL: http://llvm.org/viewvc/llvm-project?rev=344736&view=rev
Log:
[clangd] Names that are not spelled in source code are reserved.
Summary:
These are often not expected to be used directly e.g.
```
TEST_F(Fixture, X) {
^ // "Fixture_X_Test" expanded in the macro should be down ranked.
}
```
Only doing this for sema for now, as such symbols are mostly coming from sema
e.g. gtest macros expanded in the main file. We could also add a similar field
for the index symbol.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53374
Modified:
clang-tools-extra/trunk/clangd/AST.cpp
clang-tools-extra/trunk/clangd/AST.h
clang-tools-extra/trunk/clangd/Quality.cpp
clang-tools-extra/trunk/clangd/Quality.h
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Modified: clang-tools-extra/trunk/clangd/AST.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.cpp?rev=344736&r1=344735&r2=344736&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/AST.cpp (original)
+++ clang-tools-extra/trunk/clangd/AST.cpp Thu Oct 18 05:23:05 2018
@@ -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 clang {
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 "<scratch space>"
+// * symbols controlled and defined by a compile command-line option
+// `-DName=foo`, the spelling location will be "<command line>".
+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("<scratch") ||
- llvm::StringRef(PrintLoc).startswith("<command line>")) {
- // 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 "<scratch space>"
- // * symbols controlled and defined by a compile command-line option
- // `-DName=foo`, the spelling location will be "<command line>".
- SpellingLoc = SM.getExpansionRange(D->getLocation()).getBegin();
- }
+ llvm::StringRef(PrintLoc).startswith("<command line>"))
+ 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) {
Modified: clang-tools-extra/trunk/clangd/AST.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.h?rev=344736&r1=344735&r2=344736&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/AST.h (original)
+++ clang-tools-extra/trunk/clangd/AST.h Thu Oct 18 05:23:05 2018
@@ -24,6 +24,11 @@ class Decl;
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
Modified: clang-tools-extra/trunk/clangd/Quality.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.cpp?rev=344736&r1=344735&r2=344736&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Quality.cpp (original)
+++ clang-tools-extra/trunk/clangd/Quality.cpp Thu Oct 18 05:23:05 2018
@@ -7,6 +7,7 @@
//
//===----------------------------------------------------------------------===//
#include "Quality.h"
+#include "AST.h"
#include "FileDistance.h"
#include "URI.h"
#include "index/Index.h"
@@ -177,6 +178,7 @@ void SymbolQualitySignals::merge(const C
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 C
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 @@ float SymbolQualitySignals::evaluate() c
Score *= 0.1f;
if (ReservedName)
Score *= 0.1f;
+ if (ImplementationDetail)
+ Score *= 0.2f;
switch (Category) {
case Keyword: // Often relevant, but misses most signals.
Modified: clang-tools-extra/trunk/clangd/Quality.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.h?rev=344736&r1=344735&r2=344736&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Quality.h (original)
+++ clang-tools-extra/trunk/clangd/Quality.h Thu Oct 18 05:23:05 2018
@@ -57,6 +57,7 @@ struct SymbolQualitySignals {
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 {
Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=344736&r1=344735&r2=344736&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Thu Oct 18 05:23:05 2018
@@ -259,6 +259,8 @@ struct Symbol {
IndexedForCodeCompletion = 1 << 0,
/// Indicates if the symbol is deprecated.
Deprecated = 1 << 1,
+ // Symbol is an implementation detail.
+ ImplementationDetail = 1 << 2,
};
SymbolFlag Flags = SymbolFlag::None;
Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=344736&r1=344735&r2=344736&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Thu Oct 18 05:23:05 2018
@@ -536,6 +536,8 @@ const Symbol *SymbolCollector::addDeclar
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,
Modified: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp?rev=344736&r1=344735&r2=344736&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp Thu Oct 18 05:23:05 2018
@@ -45,17 +45,34 @@ TEST(QualityTests, SymbolQualitySignalEx
[[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 @@ TEST(QualityTests, SymbolQualitySignalsS
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;
Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=344736&r1=344735&r2=344736&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Thu Oct 18 05:23:05 2018
@@ -83,6 +83,9 @@ MATCHER_P(ForCodeCompletion, IsIndexedFo
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 @@ TEST_F(SymbolCollectorTest, DeprecatedSy
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
More information about the cfe-commits
mailing list