danielmartin updated this revision to Diff 265942.
danielmartin added a comment.
Move clang::getAccess to Specifiers.h and refactor logic in clang-doc
to use that function instead of its own.
Also changes where "public", "private" etc. is shown in the hover
contents. Now it's shown at the bottom.
Strengthens an existing unit test to also check for access specifiers.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80472/new/
https://reviews.llvm.org/D80472
Files:
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
Index: clang/include/clang/Basic/Specifiers.h
===================================================================
--- clang/include/clang/Basic/Specifiers.h
+++ clang/include/clang/Basic/Specifiers.h
@@ -365,6 +365,20 @@
};
llvm::StringRef getParameterABISpelling(ParameterABI kind);
+
+ inline llvm::StringRef 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");
+ }
} // end namespace clang
#endif // LLVM_CLANG_BASIC_SPECIFIERS_H
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -80,6 +80,7 @@
HI.Type = "char";
HI.Offset = 0;
HI.Size = 1;
+ HI.AccessSpecifier = "public";
}},
// Local to class method.
{R"cpp(
@@ -116,6 +117,7 @@
HI.Type = "char";
HI.Offset = 0;
HI.Size = 1;
+ HI.AccessSpecifier = "public";
}},
// Struct definition shows size.
{R"cpp(
@@ -345,6 +347,7 @@
HI.Kind = index::SymbolKind::Constructor;
HI.Definition = "X()";
HI.Parameters.emplace();
+ HI.AccessSpecifier = "public";
}},
{"class X { [[^~]]X(); };", // FIXME: Should be [[~X]]()
[](HoverInfo &HI) {
@@ -354,6 +357,7 @@
HI.Kind = index::SymbolKind::Destructor;
HI.Definition = "~X()";
HI.Parameters.emplace();
+ HI.AccessSpecifier = "private";
}},
{"class X { [[op^erator]] int(); };",
[](HoverInfo &HI) {
@@ -363,6 +367,7 @@
HI.Kind = index::SymbolKind::ConversionFunction;
HI.Definition = "operator int()";
HI.Parameters.emplace();
+ HI.AccessSpecifier = "private";
}},
{"class X { operator [[^X]](); };",
[](HoverInfo &HI) {
@@ -495,6 +500,7 @@
HI.NamespaceScope = "";
HI.LocalScope = "Add<1, 2>::";
HI.Value = "3";
+ HI.AccessSpecifier = "public";
}},
{R"cpp(
constexpr int answer() { return 40 + 2; }
@@ -607,6 +613,7 @@
HI.Definition = "typename T = int";
HI.LocalScope = "foo::";
HI.Type = "typename";
+ HI.AccessSpecifier = "public";
}},
{// TemplateTemplate Type Parameter
R"cpp(
@@ -619,6 +626,7 @@
HI.Definition = "template <typename> class T";
HI.LocalScope = "foo::";
HI.Type = "template <typename> class";
+ HI.AccessSpecifier = "public";
}},
{// NonType Template Parameter
R"cpp(
@@ -631,6 +639,7 @@
HI.Definition = "int T = 5";
HI.LocalScope = "foo::";
HI.Type = "int";
+ HI.AccessSpecifier = "public";
}},
{// Getter
@@ -647,6 +656,7 @@
HI.Type = "float ()";
HI.ReturnType = "float";
HI.Parameters.emplace();
+ HI.AccessSpecifier = "public";
}},
{// Setter
R"cpp(
@@ -665,6 +675,7 @@
HI.Parameters->emplace_back();
HI.Parameters->back().Type = "float";
HI.Parameters->back().Name = "v";
+ HI.AccessSpecifier = "public";
}},
{// Setter (builder)
R"cpp(
@@ -683,6 +694,7 @@
HI.Parameters->emplace_back();
HI.Parameters->back().Type = "float";
HI.Parameters->back().Name = "v";
+ HI.AccessSpecifier = "public";
}},
};
for (const auto &Case : Cases) {
@@ -716,6 +728,7 @@
EXPECT_EQ(H->Value, Expected.Value);
EXPECT_EQ(H->Size, Expected.Size);
EXPECT_EQ(H->Offset, Expected.Offset);
+ EXPECT_EQ(H->AccessSpecifier, Expected.AccessSpecifier);
}
}
@@ -1968,20 +1981,20 @@
{
[](HoverInfo &HI) {
HI.Kind = index::SymbolKind::Field;
- HI.AccessSpecifier = AccessSpecifier::AS_public;
+ HI.AccessSpecifier = "public";
HI.Name = "foo";
HI.LocalScope = "test::Bar::";
HI.Definition = "def";
},
- R"(public field foo
+ R"(field foo
// In test::Bar
-def)",
+public: def)",
},
{
[](HoverInfo &HI) {
HI.Definition = "int method()";
- HI.AccessSpecifier = AccessSpecifier::AS_protected;
+ HI.AccessSpecifier = "protected";
HI.Kind = index::SymbolKind::InstanceMethod;
HI.NamespaceScope = "";
HI.LocalScope = "cls<int>::";
@@ -1990,25 +2003,25 @@
HI.ReturnType = "int";
HI.Type = "int ()";
},
- R"(protected instance-method method
+ R"(instance-method method
→ int
// In cls<int>
-int method())",
+protected: int method())",
},
{
[](HoverInfo &HI) {
HI.Kind = index::SymbolKind::Union;
- HI.AccessSpecifier = AccessSpecifier::AS_private;
+ HI.AccessSpecifier = "private";
HI.Name = "foo";
HI.NamespaceScope = "ns1::";
HI.Definition = "union foo {}";
},
- R"(private union foo
+ R"(union foo
// In namespace ns1
-union foo {})",
+private: union foo {})",
}};
for (const auto &C : Cases) {
Index: clang-tools-extra/clangd/Hover.h
===================================================================
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -59,8 +59,9 @@
/// Source code containing the definition of the symbol.
std::string Definition;
- /// Access specifier. Applies to members of class/structs or unions.
- AccessSpecifier AccessSpecifier = AccessSpecifier::AS_none;
+ /// 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;
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -468,7 +468,7 @@
HoverInfo HI;
const ASTContext &Ctx = D->getASTContext();
- HI.AccessSpecifier = D->getAccess();
+ HI.AccessSpecifier = std::string(getAccess(D->getAccess()));
HI.NamespaceScope = getNamespaceScope(D);
if (!HI.NamespaceScope->empty())
HI.NamespaceScope->append("::");
@@ -677,20 +677,6 @@
}
}
-StringRef getAccessString(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");
-}
-
} // namespace
llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
@@ -789,8 +775,6 @@
// level 1 and 2 headers in a huge font, see
// https://github.com/microsoft/vscode/issues/88417 for details.
markup::Paragraph &Header = Output.addHeading(3);
- if (AccessSpecifier != AccessSpecifier::AS_none)
- Header.appendText(getAccessString(AccessSpecifier)).appendSpace();
if (Kind != index::SymbolKind::Unknown)
Header.appendText(index::getSymbolKindString(Kind)).appendSpace();
assert(!Name.empty() && "hover triggered on a nameless symbol");
@@ -852,9 +836,12 @@
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;
}
Index: clang-tools-extra/clang-doc/MDGenerator.cpp
===================================================================
--- clang-tools-extra/clang-doc/MDGenerator.cpp
+++ clang-tools-extra/clang-doc/MDGenerator.cpp
@@ -157,7 +157,7 @@
First = false;
}
writeHeader(I.Name, 3, OS);
- std::string Access = getAccess(I.Access);
+ std::string Access = std::string(getAccess(I.Access));
if (Access != "")
writeLine(genItalic(Access + " " + I.ReturnType.Type.Name + " " + I.Name +
"(" + Stream.str() + ")"),
@@ -250,7 +250,7 @@
if (!I.Members.empty()) {
writeHeader("Members", 2, OS);
for (const auto &Member : I.Members) {
- std::string Access = getAccess(Member.Access);
+ std::string Access = std::string(getAccess(Member.Access));
if (Access != "")
writeLine(Access + " " + Member.Type.Name + " " + Member.Name, OS);
else
Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===================================================================
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -402,7 +402,7 @@
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 = std::string(getAccess(M.Access));
if (Access != "")
Access = Access + " ";
auto LIBody = std::make_unique<TagNode>(HTMLTag::TAG_LI);
@@ -679,7 +679,7 @@
Out.emplace_back(std::make_unique<TagNode>(HTMLTag::TAG_P));
auto &FunctionHeader = Out.back();
- std::string Access = getAccess(I.Access);
+ std::string Access = std::string(getAccess(I.Access));
if (Access != "")
FunctionHeader->Children.emplace_back(
std::make_unique<TextNode>(Access + " "));
Index: clang-tools-extra/clang-doc/Generators.h
===================================================================
--- clang-tools-extra/clang-doc/Generators.h
+++ clang-tools-extra/clang-doc/Generators.h
@@ -42,8 +42,6 @@
llvm::Expected<std::unique_ptr<Generator>>
findGeneratorByName(llvm::StringRef Format);
-std::string getAccess(AccessSpecifier AS);
-
std::string getTagType(TagTypeKind AS);
} // namespace doc
Index: clang-tools-extra/clang-doc/Generators.cpp
===================================================================
--- clang-tools-extra/clang-doc/Generators.cpp
+++ clang-tools-extra/clang-doc/Generators.cpp
@@ -27,20 +27,6 @@
// 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:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
