[clang-tools-extra] 055d809 - [clangd] Don't index __reserved_names in headers.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 5 07:34:15 PST 2022
Author: Sam McCall
Date: 2022-01-05T16:34:04+01:00
New Revision: 055d8090d1d5137dab88533995e0c5d9b5390c28
URL: https://github.com/llvm/llvm-project/commit/055d8090d1d5137dab88533995e0c5d9b5390c28
DIFF: https://github.com/llvm/llvm-project/commit/055d8090d1d5137dab88533995e0c5d9b5390c28.diff
LOG: [clangd] Don't index __reserved_names in headers.
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.
Differential Revision: https://reviews.llvm.org/D115301
Added:
Modified:
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/AST.h
clang-tools-extra/clangd/Quality.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/index/SymbolCollector.h
clang-tools-extra/clangd/unittests/ASTTests.cpp
clang-tools-extra/clangd/unittests/QualityTests.cpp
clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp
index 3b5956037746..53b55e1579ec 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -376,6 +376,24 @@ std::string printType(const QualType QT, const DeclContext &CurContext) {
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())
diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h
index 53d9f16a4262..afd591f0f4b4 100644
--- a/clang-tools-extra/clangd/AST.h
+++ b/clang-tools-extra/clangd/AST.h
@@ -74,6 +74,12 @@ std::string printObjCMethod(const ObjCMethodDecl &Method);
// `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/Quality.cpp b/clang-tools-extra/clangd/Quality.cpp
index dc9c75b57670..900db4913f80 100644
--- 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 @@ void SymbolQualitySignals::merge(const CodeCompletionResult &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) {
@@ -198,7 +193,7 @@ void SymbolQualitySignals::merge(const Symbol &IndexResult) {
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
index 315d79a932df..faed27d7c8c4 100644
--- 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 <string>
namespace clang {
@@ -330,6 +330,13 @@ bool isProtoFile(SourceLocation Loc, const SourceManager &SourceMgr);
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
index 8960602a0190..fa58c6ff6fb1 100644
--- a/clang-tools-extra/clangd/index/FileIndex.cpp
+++ b/clang-tools-extra/clangd/index/FileIndex.cpp
@@ -56,6 +56,9 @@ SlabTuple indexSymbols(ASTContext &AST, Preprocessor &PP,
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.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 639003d667f4..cccc2c4e8f3d 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -356,6 +356,10 @@ bool SymbolCollector::shouldCollectSymbol(const NamedDecl &ND,
// 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/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h
index a451a81ed29c..0be6d2ad6c5b 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.h
+++ b/clang-tools-extra/clangd/index/SymbolCollector.h
@@ -81,6 +81,9 @@ class SymbolCollector : public index::IndexDataConsumer {
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/unittests/ASTTests.cpp b/clang-tools-extra/clangd/unittests/ASTTests.cpp
index 44dc8f0c69cf..53d399eed72a 100644
--- a/clang-tools-extra/clangd/unittests/ASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ASTTests.cpp
@@ -427,6 +427,29 @@ TEST(ClangdAST, GetAttributes) {
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
index f5fcb0e8d04e..2992f694637d 100644
--- a/clang-tools-extra/clangd/unittests/QualityTests.cpp
+++ b/clang-tools-extra/clangd/unittests/QualityTests.cpp
@@ -54,13 +54,6 @@ TEST(QualityTests, SymbolQualitySignalExtraction) {
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 @@ TEST(QualityTests, SymbolQualitySignalExtraction) {
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
index bfe0e430b775..fdcee9feffb2 100644
--- a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -315,6 +315,16 @@ TEST(SourceCodeTests, SourceLocationInMainFile) {
}
}
+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
index 5afaa83bfdcb..24cd8b7313bd 100644
--- 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 @@ TEST_F(SymbolCollectorTest, NoCrashOnObjCMethodCStyleParam) {
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
More information about the cfe-commits
mailing list