[clang-tools-extra] 6407aa9 - [clangd] Add access specifier information to hover contents
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Wed May 27 10:36:59 PDT 2020
Author: Daniel Martín
Date: 2020-05-27T19:36:46+02:00
New Revision: 6407aa9d2e0e225bc81d3b2602d6e6ed79912ec2
URL: https://github.com/llvm/llvm-project/commit/6407aa9d2e0e225bc81d3b2602d6e6ed79912ec2
DIFF: https://github.com/llvm/llvm-project/commit/6407aa9d2e0e225bc81d3b2602d6e6ed79912ec2.diff
LOG: [clangd] Add access specifier information to hover contents
Summary:
For https://github.com/clangd/clangd/issues/382
This commit adds access specifier information to the hover
contents. For example, the hover information of a class field or
member function will now indicate if the field or member is private,
public, or protected. This can be particularly useful when a developer
is in the implementation file and wants to know if a particular member
definition is public or private.
Reviewers: kadircet
Reviewed By: kadircet
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D80472
Added:
Modified:
clang-tools-extra/clang-doc/Generators.cpp
clang-tools-extra/clang-doc/Generators.h
clang-tools-extra/clang-doc/HTMLGenerator.cpp
clang-tools-extra/clang-doc/MDGenerator.cpp
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Hover.h
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang/include/clang/Basic/Specifiers.h
clang/lib/AST/DeclPrinter.cpp
clang/lib/AST/JSONNodeDumper.cpp
clang/lib/AST/TextNodeDumper.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-doc/Generators.cpp b/clang-tools-extra/clang-doc/Generators.cpp
index ec7133466f2e..3b7dcf93411a 100644
--- a/clang-tools-extra/clang-doc/Generators.cpp
+++ b/clang-tools-extra/clang-doc/Generators.cpp
@@ -27,20 +27,6 @@ findGeneratorByName(llvm::StringRef Format) {
// Enum conversion
-std::string getAccess(AccessSpecifier AS) {
- switch (AS) {
- case AccessSpecifier::AS_public:
- return "public";
- case AccessSpecifier::AS_protected:
- return "protected";
- case AccessSpecifier::AS_private:
- return "private";
- case AccessSpecifier::AS_none:
- return {};
- }
- llvm_unreachable("Unknown AccessSpecifier");
-}
-
std::string getTagType(TagTypeKind AS) {
switch (AS) {
case TagTypeKind::TTK_Class:
diff --git a/clang-tools-extra/clang-doc/Generators.h b/clang-tools-extra/clang-doc/Generators.h
index 799d503b1023..89c6b34c4384 100644
--- a/clang-tools-extra/clang-doc/Generators.h
+++ b/clang-tools-extra/clang-doc/Generators.h
@@ -42,8 +42,6 @@ typedef llvm::Registry<Generator> GeneratorRegistry;
llvm::Expected<std::unique_ptr<Generator>>
findGeneratorByName(llvm::StringRef Format);
-std::string getAccess(AccessSpecifier AS);
-
std::string getTagType(TagTypeKind AS);
} // namespace doc
diff --git a/clang-tools-extra/clang-doc/HTMLGenerator.cpp b/clang-tools-extra/clang-doc/HTMLGenerator.cpp
index dc569e2a482c..49ff36a02be7 100644
--- a/clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ b/clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -402,7 +402,7 @@ genRecordMembersBlock(const llvm::SmallVector<MemberTypeInfo, 4> &Members,
Out.emplace_back(std::make_unique<TagNode>(HTMLTag::TAG_UL));
auto &ULBody = Out.back();
for (const auto &M : Members) {
- std::string Access = getAccess(M.Access);
+ std::string Access = getAccessSpelling(M.Access).str();
if (Access != "")
Access = Access + " ";
auto LIBody = std::make_unique<TagNode>(HTMLTag::TAG_LI);
@@ -679,7 +679,7 @@ genHTML(const FunctionInfo &I, const ClangDocContext &CDCtx,
Out.emplace_back(std::make_unique<TagNode>(HTMLTag::TAG_P));
auto &FunctionHeader = Out.back();
- std::string Access = getAccess(I.Access);
+ std::string Access = getAccessSpelling(I.Access).str();
if (Access != "")
FunctionHeader->Children.emplace_back(
std::make_unique<TextNode>(Access + " "));
diff --git a/clang-tools-extra/clang-doc/MDGenerator.cpp b/clang-tools-extra/clang-doc/MDGenerator.cpp
index 9ad71e435a70..58c2de96b298 100644
--- a/clang-tools-extra/clang-doc/MDGenerator.cpp
+++ b/clang-tools-extra/clang-doc/MDGenerator.cpp
@@ -157,7 +157,7 @@ static void genMarkdown(const ClangDocContext &CDCtx, const FunctionInfo &I,
First = false;
}
writeHeader(I.Name, 3, OS);
- std::string Access = getAccess(I.Access);
+ std::string Access = getAccessSpelling(I.Access).str();
if (Access != "")
writeLine(genItalic(Access + " " + I.ReturnType.Type.Name + " " + I.Name +
"(" + Stream.str() + ")"),
@@ -250,7 +250,7 @@ static void genMarkdown(const ClangDocContext &CDCtx, const RecordInfo &I,
if (!I.Members.empty()) {
writeHeader("Members", 2, OS);
for (const auto &Member : I.Members) {
- std::string Access = getAccess(Member.Access);
+ std::string Access = getAccessSpelling(Member.Access).str();
if (Access != "")
writeLine(Access + " " + Member.Type.Name + " " + Member.Name, OS);
else
diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index 3d0430b57931..e2a3a0dd62f5 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -468,6 +468,7 @@ HoverInfo getHoverContents(const NamedDecl *D, const SymbolIndex *Index) {
HoverInfo HI;
const ASTContext &Ctx = D->getASTContext();
+ HI.AccessSpecifier = getAccessSpelling(D->getAccess()).str();
HI.NamespaceScope = getNamespaceScope(D);
if (!HI.NamespaceScope->empty())
HI.NamespaceScope->append("::");
@@ -835,9 +836,12 @@ markup::Document HoverInfo::present() const {
ScopeComment = "// In namespace " +
llvm::StringRef(*NamespaceScope).rtrim(':').str() + '\n';
}
+ std::string DefinitionWithAccess = !AccessSpecifier.empty()
+ ? AccessSpecifier + ": " + Definition
+ : Definition;
// Note that we don't print anything for global namespace, to not annoy
// non-c++ projects or projects that are not making use of namespaces.
- Output.addCodeBlock(ScopeComment + Definition);
+ Output.addCodeBlock(ScopeComment + DefinitionWithAccess);
}
return Output;
}
diff --git a/clang-tools-extra/clangd/Hover.h b/clang-tools-extra/clangd/Hover.h
index 931e1c2363a4..b712d844e33d 100644
--- a/clang-tools-extra/clangd/Hover.h
+++ b/clang-tools-extra/clangd/Hover.h
@@ -59,6 +59,9 @@ struct HoverInfo {
/// Source code containing the definition of the symbol.
std::string Definition;
+ /// Access specifier for declarations inside class/struct/unions, empty for
+ /// others.
+ std::string AccessSpecifier;
/// Pretty-printed variable type.
/// Set only for variables.
llvm::Optional<std::string> Type;
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index e5ff0ee364d8..dc818ea66193 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -12,6 +12,7 @@
#include "TestIndex.h"
#include "TestTU.h"
#include "index/MemIndex.h"
+#include "clang/Basic/Specifiers.h"
#include "clang/Index/IndexSymbol.h"
#include "llvm/ADT/None.h"
#include "llvm/ADT/StringRef.h"
@@ -79,6 +80,7 @@ TEST(Hover, Structured) {
HI.Type = "char";
HI.Offset = 0;
HI.Size = 1;
+ HI.AccessSpecifier = "public";
}},
// Local to class method.
{R"cpp(
@@ -115,6 +117,7 @@ TEST(Hover, Structured) {
HI.Type = "char";
HI.Offset = 0;
HI.Size = 1;
+ HI.AccessSpecifier = "public";
}},
// Struct definition shows size.
{R"cpp(
@@ -344,6 +347,7 @@ class Foo {})cpp";
HI.Kind = index::SymbolKind::Constructor;
HI.Definition = "X()";
HI.Parameters.emplace();
+ HI.AccessSpecifier = "public";
}},
{"class X { [[^~]]X(); };", // FIXME: Should be [[~X]]()
[](HoverInfo &HI) {
@@ -353,6 +357,7 @@ class Foo {})cpp";
HI.Kind = index::SymbolKind::Destructor;
HI.Definition = "~X()";
HI.Parameters.emplace();
+ HI.AccessSpecifier = "private";
}},
{"class X { [[op^erator]] int(); };",
[](HoverInfo &HI) {
@@ -362,6 +367,7 @@ class Foo {})cpp";
HI.Kind = index::SymbolKind::ConversionFunction;
HI.Definition = "operator int()";
HI.Parameters.emplace();
+ HI.AccessSpecifier = "private";
}},
{"class X { operator [[^X]](); };",
[](HoverInfo &HI) {
@@ -494,6 +500,7 @@ class Foo {})cpp";
HI.NamespaceScope = "";
HI.LocalScope = "Add<1, 2>::";
HI.Value = "3";
+ HI.AccessSpecifier = "public";
}},
{R"cpp(
constexpr int answer() { return 40 + 2; }
@@ -606,6 +613,7 @@ class Foo {})cpp";
HI.Definition = "typename T = int";
HI.LocalScope = "foo::";
HI.Type = "typename";
+ HI.AccessSpecifier = "public";
}},
{// TemplateTemplate Type Parameter
R"cpp(
@@ -618,6 +626,7 @@ class Foo {})cpp";
HI.Definition = "template <typename> class T";
HI.LocalScope = "foo::";
HI.Type = "template <typename> class";
+ HI.AccessSpecifier = "public";
}},
{// NonType Template Parameter
R"cpp(
@@ -630,6 +639,7 @@ class Foo {})cpp";
HI.Definition = "int T = 5";
HI.LocalScope = "foo::";
HI.Type = "int";
+ HI.AccessSpecifier = "public";
}},
{// Getter
@@ -646,6 +656,7 @@ class Foo {})cpp";
HI.Type = "float ()";
HI.ReturnType = "float";
HI.Parameters.emplace();
+ HI.AccessSpecifier = "public";
}},
{// Setter
R"cpp(
@@ -664,6 +675,7 @@ class Foo {})cpp";
HI.Parameters->emplace_back();
HI.Parameters->back().Type = "float";
HI.Parameters->back().Name = "v";
+ HI.AccessSpecifier = "public";
}},
{// Setter (builder)
R"cpp(
@@ -682,6 +694,7 @@ class Foo {})cpp";
HI.Parameters->emplace_back();
HI.Parameters->back().Type = "float";
HI.Parameters->back().Name = "v";
+ HI.AccessSpecifier = "public";
}},
};
for (const auto &Case : Cases) {
@@ -715,6 +728,7 @@ class Foo {})cpp";
EXPECT_EQ(H->Value, Expected.Value);
EXPECT_EQ(H->Size, Expected.Size);
EXPECT_EQ(H->Offset, Expected.Offset);
+ EXPECT_EQ(H->AccessSpecifier, Expected.AccessSpecifier);
}
}
@@ -1964,7 +1978,51 @@ Size: 4 bytes
// In test::Bar
def)",
},
- };
+ {
+ [](HoverInfo &HI) {
+ HI.Kind = index::SymbolKind::Field;
+ HI.AccessSpecifier = "public";
+ HI.Name = "foo";
+ HI.LocalScope = "test::Bar::";
+ HI.Definition = "def";
+ },
+ R"(field foo
+
+// In test::Bar
+public: def)",
+ },
+ {
+ [](HoverInfo &HI) {
+ HI.Definition = "int method()";
+ HI.AccessSpecifier = "protected";
+ HI.Kind = index::SymbolKind::InstanceMethod;
+ HI.NamespaceScope = "";
+ HI.LocalScope = "cls<int>::";
+ HI.Name = "method";
+ HI.Parameters.emplace();
+ HI.ReturnType = "int";
+ HI.Type = "int ()";
+ },
+ R"(instance-method method
+
+→ int
+
+// In cls<int>
+protected: int method())",
+ },
+ {
+ [](HoverInfo &HI) {
+ HI.Kind = index::SymbolKind::Union;
+ HI.AccessSpecifier = "private";
+ HI.Name = "foo";
+ HI.NamespaceScope = "ns1::";
+ HI.Definition = "union foo {}";
+ },
+ R"(union foo
+
+// In namespace ns1
+private: union foo {})",
+ }};
for (const auto &C : Cases) {
HoverInfo HI;
diff --git a/clang/include/clang/Basic/Specifiers.h b/clang/include/clang/Basic/Specifiers.h
index e6c2cb39566c..2c80dd4fa810 100644
--- a/clang/include/clang/Basic/Specifiers.h
+++ b/clang/include/clang/Basic/Specifiers.h
@@ -365,6 +365,20 @@ namespace clang {
};
llvm::StringRef getParameterABISpelling(ParameterABI kind);
+
+ inline llvm::StringRef getAccessSpelling(AccessSpecifier AS) {
+ switch (AS) {
+ case AccessSpecifier::AS_public:
+ return "public";
+ case AccessSpecifier::AS_protected:
+ return "protected";
+ case AccessSpecifier::AS_private:
+ return "private";
+ case AccessSpecifier::AS_none:
+ return {};
+ }
+ llvm_unreachable("Unknown AccessSpecifier");
+ }
} // end namespace clang
#endif // LLVM_CLANG_BASIC_SPECIFIERS_H
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index fc2a166e11b4..4df6512e6c76 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -289,12 +289,10 @@ void DeclPrinter::ProcessDeclGroup(SmallVectorImpl<Decl*>& Decls) {
}
void DeclPrinter::Print(AccessSpecifier AS) {
- switch(AS) {
- case AS_none: llvm_unreachable("No access specifier!");
- case AS_public: Out << "public"; break;
- case AS_protected: Out << "protected"; break;
- case AS_private: Out << "private"; break;
- }
+ const auto AccessSpelling = getAccessSpelling(AS);
+ if (AccessSpelling.empty())
+ llvm_unreachable("No access specifier!");
+ Out << AccessSpelling;
}
void DeclPrinter::PrintConstructorInitializers(CXXConstructorDecl *CDecl,
diff --git a/clang/lib/AST/JSONNodeDumper.cpp b/clang/lib/AST/JSONNodeDumper.cpp
index 91281fb44bfa..8edfed673ce2 100644
--- a/clang/lib/AST/JSONNodeDumper.cpp
+++ b/clang/lib/AST/JSONNodeDumper.cpp
@@ -1,5 +1,6 @@
#include "clang/AST/JSONNodeDumper.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
#include "clang/Lex/Lexer.h"
#include "llvm/ADT/StringSwitch.h"
@@ -465,13 +466,10 @@ JSONNodeDumper::createCXXRecordDefinitionData(const CXXRecordDecl *RD) {
#undef FIELD2
std::string JSONNodeDumper::createAccessSpecifier(AccessSpecifier AS) {
- switch (AS) {
- case AS_none: return "none";
- case AS_private: return "private";
- case AS_protected: return "protected";
- case AS_public: return "public";
- }
- llvm_unreachable("Unknown access specifier");
+ const auto AccessSpelling = getAccessSpelling(AS);
+ if (AccessSpelling.empty())
+ return "none";
+ return AccessSpelling.str();
}
llvm::json::Object
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index 9dbe55707539..1b640a8cbe40 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -17,6 +17,7 @@
#include "clang/AST/LocInfoType.h"
#include "clang/Basic/Module.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
using namespace clang;
@@ -436,19 +437,10 @@ void TextNodeDumper::dumpName(const NamedDecl *ND) {
}
void TextNodeDumper::dumpAccessSpecifier(AccessSpecifier AS) {
- switch (AS) {
- case AS_none:
- break;
- case AS_public:
- OS << "public";
- break;
- case AS_protected:
- OS << "protected";
- break;
- case AS_private:
- OS << "private";
- break;
- }
+ const auto AccessSpelling = getAccessSpelling(AS);
+ if (AccessSpelling.empty())
+ return;
+ OS << AccessSpelling;
}
void TextNodeDumper::dumpCleanupObject(
More information about the cfe-commits
mailing list