ioeric updated this revision to Diff 147032.
ioeric marked an inline comment as done.
ioeric added a comment.
- Address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46751
Files:
clangd/ClangdLSPServer.cpp
clangd/SourceCode.cpp
clangd/SourceCode.h
clangd/index/SymbolCollector.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -697,6 +697,40 @@
AllOf(QName("pörk"), DeclRange(Header.range()))));
}
+TEST_F(SymbolCollectorTest, FilterPrivateProtoSymbols) {
+ TestHeaderName = testPath("x.proto.h");
+ const std::string Header =
+ R"(// Generated by the protocol buffer compiler. DO NOT EDIT!
+ namespace nx {
+ class Top_Level {};
+ class TopLevel {};
+ enum Kind {
+ KIND_OK,
+ Kind_Not_Ok,
+ };
+ bool operator<(int x, int y);
+ })";
+ runSymbolCollector(Header, /*Main=*/"");
+ EXPECT_THAT(Symbols, UnorderedElementsAre(QName("nx"), QName("nx::TopLevel"),
+ QName("nx::Kind"),
+ QName("nx::KIND_OK")));
+}
+
+TEST_F(SymbolCollectorTest, DoubleCheckProtoHeaderComment) {
+ TestHeaderName = testPath("x.proto.h");
+ const std::string Header = R"(
+ namespace nx {
+ class Top_Level {};
+ enum Kind {
+ Kind_Fine
+ };
+ }
+ )";
+ runSymbolCollector(Header, /*Main=*/"");
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("nx"), QName("nx::Top_Level"),
+ QName("nx::Kind"), QName("nx::Kind_Fine")));
+}
} // namespace
} // namespace clangd
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -90,6 +90,44 @@
return llvm::None;
}
+// All proto generated headers should start with this line.
+static const char *PROTO_HEADER_COMMENT =
+ "// Generated by the protocol buffer compiler. DO NOT EDIT!";
+
+// Checks whether the decl is a private symbol in a header generated by
+// protobuf compiler.
+// To identify whether a proto header is actually generated by proto compiler,
+// we check whether it starts with PROTO_HEADER_COMMENT.
+// FIXME: make filtering extensible when there are more use cases for symbol
+// filters.
+bool isPrivateProtoDecl(const NamedDecl &ND) {
+ const auto &SM = ND.getASTContext().getSourceManager();
+ auto Loc = findNameLoc(&ND);
+ auto FileName = SM.getFilename(Loc);
+ if (!FileName.endswith(".proto.h") && !FileName.endswith(".pb.h"))
+ return false;
+ auto FID = SM.getFileID(Loc);
+ // Double check that this is an actual protobuf header.
+ if (!SM.getBufferData(FID).startswith(PROTO_HEADER_COMMENT))
+ return false;
+
+ // If ND does not have an identifier/name, it must be private.
+ if (ND.getIdentifier() == nullptr)
+ return true;
+ auto Name = ND.getIdentifier()->getName();
+ if (!Name.contains('_'))
+ return false;
+ // Nested proto entities (e.g. Message::Nested) have top-level decls
+ // that shouldn't be used (Message_Nested). Ignore them completely.
+ // The nested entities are dangling type aliases, we may want to reconsider
+ // including them in the future.
+ // For enum constants, SOME_ENUM_CONSTANT is not private and should be
+ // indexed. Outer_INNER is private. This heuristic relies on naming style, it
+ // will include OUTER_INNER and exclude some_enum_constant.
+ return (ND.getKind() != Decl::EnumConstant) ||
+ std::any_of(Name.begin(), Name.end(), islower);
+}
+
bool shouldFilterDecl(const NamedDecl *ND, ASTContext *ASTCtx,
const SymbolCollector::Options &Opts) {
using namespace clang::ast_matchers;
@@ -130,6 +168,9 @@
.empty())
return true;
+ // Avoid indexing internal symbols in protobuf generated headers.
+ if (isPrivateProtoDecl(*ND))
+ return true;
return false;
}
Index: clangd/SourceCode.h
===================================================================
--- clangd/SourceCode.h
+++ clangd/SourceCode.h
@@ -15,6 +15,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
#include "Protocol.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Core/Replacement.h"
namespace clang {
class SourceManager;
@@ -55,6 +56,11 @@
std::pair<llvm::StringRef, llvm::StringRef>
splitQualifiedName(llvm::StringRef QName);
+TextEdit replacementToEdit(StringRef Code, const tooling::Replacement &R);
+
+std::vector<TextEdit> replacementsToEdits(StringRef Code,
+ const tooling::Replacements &Repls);
+
} // namespace clangd
} // namespace clang
#endif
Index: clangd/SourceCode.cpp
===================================================================
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -166,5 +166,20 @@
return {QName.substr(0, Pos + 2), QName.substr(Pos + 2)};
}
+TextEdit replacementToEdit(StringRef Code, const tooling::Replacement &R) {
+ Range ReplacementRange = {
+ offsetToPosition(Code, R.getOffset()),
+ offsetToPosition(Code, R.getOffset() + R.getLength())};
+ return {ReplacementRange, R.getReplacementText()};
+}
+
+std::vector<TextEdit> replacementsToEdits(StringRef Code,
+ const tooling::Replacements &Repls) {
+ std::vector<TextEdit> Edits;
+ for (const auto &R : Repls)
+ Edits.push_back(replacementToEdit(Code, R));
+ return Edits;
+}
+
} // namespace clangd
} // namespace clang
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -60,32 +60,6 @@
static URISchemeRegistry::Add<TestScheme>
X("test", "Test scheme for clangd lit tests.");
-TextEdit replacementToEdit(StringRef Code, const tooling::Replacement &R) {
- Range ReplacementRange = {
- offsetToPosition(Code, R.getOffset()),
- offsetToPosition(Code, R.getOffset() + R.getLength())};
- return {ReplacementRange, R.getReplacementText()};
-}
-
-std::vector<TextEdit>
-replacementsToEdits(StringRef Code,
- const std::vector<tooling::Replacement> &Replacements) {
- // Turn the replacements into the format specified by the Language Server
- // Protocol. Fuse them into one big JSON array.
- std::vector<TextEdit> Edits;
- for (const auto &R : Replacements)
- Edits.push_back(replacementToEdit(Code, R));
- return Edits;
-}
-
-std::vector<TextEdit> replacementsToEdits(StringRef Code,
- const tooling::Replacements &Repls) {
- std::vector<TextEdit> Edits;
- for (const auto &R : Repls)
- Edits.push_back(replacementToEdit(Code, R));
- return Edits;
-}
-
SymbolKindBitset defaultSymbolKinds() {
SymbolKindBitset Defaults;
for (size_t I = SymbolKindMin; I <= static_cast<size_t>(SymbolKind::Array);
@@ -291,7 +265,11 @@
return replyError(ErrorCode::InternalError,
llvm::toString(Replacements.takeError()));
- std::vector<TextEdit> Edits = replacementsToEdits(*Code, *Replacements);
+ // Turn the replacements into the format specified by the Language
+ // Server Protocol. Fuse them into one big JSON array.
+ std::vector<TextEdit> Edits;
+ for (const auto &R : *Replacements)
+ Edits.push_back(replacementToEdit(*Code, R));
WorkspaceEdit WE;
WE.changes = {{Params.textDocument.uri.uri(), Edits}};
reply(WE);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
