sammccall created this revision.
sammccall added a reviewer: kbobyrev.
Herald added subscribers: dexonsmith, usaxena95, kadircet, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.
Underscore-uglified identifiers are used in standard library implementations to
guard against collisions with macros, and they hurt readability considerably.
(Consider `push_back(Tp_ &&__value)` vs `push_back(Tp value)`.
When we're describing an interface, the exact names of parameters are not
critical so we can drop these prefixes.
This patch adds a new PrintingPolicy flag that can applies this stripping
when recursively printing pieces of AST.
We set it in code completion/signature help, and in clangd's hover display.
All three features also do a bit of manual poking at names, so fix up those too.
Fixes https://github.com/clangd/clangd/issues/736
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D116387
Files:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
clang/include/clang/AST/PrettyPrinter.h
clang/include/clang/Basic/IdentifierTable.h
clang/lib/AST/DeclPrinter.cpp
clang/lib/AST/StmtPrinter.cpp
clang/lib/AST/TemplateName.cpp
clang/lib/AST/TypePrinter.cpp
clang/lib/Basic/IdentifierTable.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/test/CodeCompletion/deuglify.cpp
clang/unittests/AST/DeclPrinterTest.cpp
clang/unittests/AST/StmtPrinterTest.cpp
clang/unittests/AST/TypePrinterTest.cpp
Index: clang/unittests/AST/TypePrinterTest.cpp
===================================================================
--- clang/unittests/AST/TypePrinterTest.cpp
+++ clang/unittests/AST/TypePrinterTest.cpp
@@ -62,4 +62,21 @@
ASSERT_TRUE(PrintedTypeMatches(
Code, {}, Matcher, "const N::Type<T> &",
[](PrintingPolicy &Policy) { Policy.FullyQualifiedName = true; }));
-}
\ No newline at end of file
+}
+
+TEST(TypePrinter, ParamsUglified) {
+ llvm::StringLiteral Code = R"cpp(
+ template <typename _Tp, template <typename> class __f>
+ const __f<_Tp&> *A = nullptr;
+ )cpp";
+ auto Clean = [](PrintingPolicy &Policy) {
+ Policy.CleanUglifiedParameters = true;
+ };
+
+ ASSERT_TRUE(PrintedTypeMatches(Code, {},
+ varDecl(hasType(qualType().bind("id"))),
+ "const __f<_Tp &> *", nullptr));
+ ASSERT_TRUE(PrintedTypeMatches(Code, {},
+ varDecl(hasType(qualType().bind("id"))),
+ "const f<Tp &> *", Clean));
+}
Index: clang/unittests/AST/StmtPrinterTest.cpp
===================================================================
--- clang/unittests/AST/StmtPrinterTest.cpp
+++ clang/unittests/AST/StmtPrinterTest.cpp
@@ -263,3 +263,22 @@
[](PrintingPolicy &PP) { PP.TerseOutput = true; }));
}
+
+TEST(StmtPrinter, ParamsUglified) {
+ llvm::StringLiteral Code = R"cpp(
+ template <typename _T, int _I, template <typename> class _C>
+ auto foo(int __j) {
+ return typename _C<_T>::_F(_I, __j);
+ }
+ )cpp";
+ auto Clean = [](PrintingPolicy &Policy) {
+ Policy.CleanUglifiedParameters = true;
+ };
+
+ ASSERT_TRUE(PrintedStmtCXXMatches(StdVer::CXX14, Code,
+ returnStmt().bind("id"),
+ "return typename _C<_T>::_F(_I, __j);\n"));
+ ASSERT_TRUE(
+ PrintedStmtCXXMatches(StdVer::CXX14, Code, returnStmt().bind("id"),
+ "return typename C<T>::_F(I, j);\n", Clean));
+}
Index: clang/unittests/AST/DeclPrinterTest.cpp
===================================================================
--- clang/unittests/AST/DeclPrinterTest.cpp
+++ clang/unittests/AST/DeclPrinterTest.cpp
@@ -1336,6 +1336,41 @@
ASSERT_TRUE(PrintedDeclCXX11Matches(Code, "NT2", "int NT2 = 5"));
}
+TEST(DeclPrinter, TestFunctionParamUglified) {
+ llvm::StringLiteral Code = R"cpp(
+ class __c;
+ void _A(__c *__param);
+ )cpp";
+ auto Clean = [](PrintingPolicy &Policy) {
+ Policy.CleanUglifiedParameters = true;
+ };
+
+ ASSERT_TRUE(PrintedDeclCXX17Matches(Code, namedDecl(hasName("_A")).bind("id"),
+ "void _A(__c *__param)"));
+ ASSERT_TRUE(PrintedDeclCXX17Matches(Code, namedDecl(hasName("_A")).bind("id"),
+ "void _A(__c *param)", Clean));
+}
+
+TEST(DeclPrinter, TestTemplateParamUglified) {
+ llvm::StringLiteral Code = R"cpp(
+ template <typename _Tp, int __n, template <typename> class _Container>
+ struct _A{};
+ )cpp";
+ auto Clean = [](PrintingPolicy &Policy) {
+ Policy.CleanUglifiedParameters = true;
+ };
+
+ ASSERT_TRUE(PrintedDeclCXX17Matches(
+ Code, classTemplateDecl(hasName("_A")).bind("id"),
+ "template <typename _Tp, int __n, template <typename> class _Container> "
+ "struct _A {}"));
+ ASSERT_TRUE(PrintedDeclCXX17Matches(
+ Code, classTemplateDecl(hasName("_A")).bind("id"),
+ "template <typename Tp, int n, template <typename> class Container> "
+ "struct _A {}",
+ Clean));
+}
+
TEST(DeclPrinter, TestStaticAssert1) {
ASSERT_TRUE(PrintedDeclCXX17Matches("static_assert(true);",
staticAssertDecl().bind("id"),
Index: clang/test/CodeCompletion/deuglify.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeCompletion/deuglify.cpp
@@ -0,0 +1,25 @@
+// Fake standard library with uglified names.
+// Parameters (including template params) get ugliness stripped.
+namespace std {
+
+template <typename _Tp>
+class __vector_base {};
+
+template <typename _Tp>
+class vector : private __vector_base<_Tp> {
+public:
+ _Tp &at(unsigned __index) const;
+ int __stays_ugly();
+};
+
+} // namespace std
+
+int x = std::vector<int>{}.at(42);
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:17:14 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1: COMPLETION: __vector_base : __vector_base<<#typename Tp#>>
+// CHECK-CC1: COMPLETION: vector : vector<<#typename Tp#>>
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:17:28 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// CHECK-CC2: COMPLETION: __stays_ugly : [#int#]__stays_ugly()
+// CHECK-CC2: COMPLETION: at : [#int &#]at(<#unsigned int index#>)[# const#]
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:17:31 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// CHECK-CC3: OVERLOAD: [#int &#]at(<#unsigned int index#>)
Index: clang/lib/Sema/SemaCodeComplete.cpp
===================================================================
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -1895,6 +1895,7 @@
Policy.SuppressStrongLifetime = true;
Policy.SuppressUnwrittenScope = true;
Policy.SuppressScope = true;
+ Policy.CleanUglifiedParameters = true;
return Policy;
}
@@ -2832,7 +2833,7 @@
std::string Result;
if (Param->getIdentifier() && !ObjCMethodParam && !SuppressName)
- Result = std::string(Param->getIdentifier()->getName());
+ Result = std::string(Param->getIdentifier()->deuglifiedName());
QualType Type = Param->getType();
if (ObjCSubsts)
@@ -2843,7 +2844,7 @@
"(" + formatObjCParamQualifiers(Param->getObjCDeclQualifier(), Type);
Result += Type.getAsString(Policy) + ")";
if (Param->getIdentifier() && !SuppressName)
- Result += Param->getIdentifier()->getName();
+ Result += Param->getIdentifier()->deuglifiedName();
} else {
Type.getAsStringInternal(Result, Policy);
}
@@ -2871,7 +2872,7 @@
// for the block; just use the parameter type as a placeholder.
std::string Result;
if (!ObjCMethodParam && Param->getIdentifier())
- Result = std::string(Param->getIdentifier()->getName());
+ Result = std::string(Param->getIdentifier()->deuglifiedName());
QualType Type = Param->getType().getUnqualifiedType();
@@ -2884,7 +2885,7 @@
if (Result.back() != ')')
Result += " ";
if (Param->getIdentifier())
- Result += Param->getIdentifier()->getName();
+ Result += Param->getIdentifier()->deuglifiedName();
} else {
Type.getAsStringInternal(Result, Policy);
}
@@ -3079,14 +3080,14 @@
if (TTP->getIdentifier()) {
PlaceholderStr += ' ';
- PlaceholderStr += TTP->getIdentifier()->getName();
+ PlaceholderStr += TTP->getIdentifier()->deuglifiedName();
}
HasDefaultArg = TTP->hasDefaultArgument();
} else if (NonTypeTemplateParmDecl *NTTP =
dyn_cast<NonTypeTemplateParmDecl>(*P)) {
if (NTTP->getIdentifier())
- PlaceholderStr = std::string(NTTP->getIdentifier()->getName());
+ PlaceholderStr = std::string(NTTP->getIdentifier()->deuglifiedName());
NTTP->getType().getAsStringInternal(PlaceholderStr, Policy);
HasDefaultArg = NTTP->hasDefaultArgument();
} else {
@@ -3098,7 +3099,7 @@
PlaceholderStr = "template<...> class";
if (TTP->getIdentifier()) {
PlaceholderStr += ' ';
- PlaceholderStr += TTP->getIdentifier()->getName();
+ PlaceholderStr += TTP->getIdentifier()->deuglifiedName();
}
HasDefaultArg = TTP->hasDefaultArgument();
Index: clang/lib/Basic/IdentifierTable.cpp
===================================================================
--- clang/lib/Basic/IdentifierTable.cpp
+++ clang/lib/Basic/IdentifierTable.cpp
@@ -309,6 +309,14 @@
return ReservedIdentifierStatus::NotReserved;
}
+StringRef IdentifierInfo::deuglifiedName() const {
+ StringRef Name = getName();
+ if (Name.size() >= 2 && Name.front() == '_' &&
+ (Name[1] == '_' || (Name[1] >= 'A' && Name[1] <= 'Z')))
+ return Name.ltrim('_');
+ return Name;
+}
+
tok::PPKeywordKind IdentifierInfo::getPPKeywordID() const {
// We use a perfect hash function here involving the length of the keyword,
// the first and third character. For preprocessor ID's there are no
Index: clang/lib/AST/TypePrinter.cpp
===================================================================
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1418,7 +1418,8 @@
}
OS << "auto";
} else if (IdentifierInfo *Id = T->getIdentifier())
- OS << Id->getName();
+ OS << (Policy.CleanUglifiedParameters ? Id->deuglifiedName()
+ : Id->getName());
else
OS << "type-parameter-" << T->getDepth() << '-' << T->getIndex();
Index: clang/lib/AST/TemplateName.cpp
===================================================================
--- clang/lib/AST/TemplateName.cpp
+++ clang/lib/AST/TemplateName.cpp
@@ -223,8 +223,12 @@
void TemplateName::print(raw_ostream &OS, const PrintingPolicy &Policy,
Qualified Qual) const {
if (TemplateDecl *Template = Storage.dyn_cast<TemplateDecl *>())
- if (Qual == Qualified::Fully &&
- getDependence() != TemplateNameDependenceScope::DependentInstantiation)
+ if (Policy.CleanUglifiedParameters &&
+ isa<TemplateTemplateParmDecl>(Template) && Template->getIdentifier())
+ OS << Template->getIdentifier()->deuglifiedName();
+ else if (Qual == Qualified::Fully &&
+ getDependence() !=
+ TemplateNameDependenceScope::DependentInstantiation)
Template->printQualifiedName(OS, Policy);
else
OS << *Template;
Index: clang/lib/AST/StmtPrinter.cpp
===================================================================
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -1030,7 +1030,12 @@
Qualifier->print(OS, Policy);
if (Node->hasTemplateKeyword())
OS << "template ";
- OS << Node->getNameInfo();
+ if (Policy.CleanUglifiedParameters &&
+ isa<ParmVarDecl, NonTypeTemplateParmDecl>(Node->getDecl()) &&
+ Node->getDecl()->getIdentifier())
+ OS << Node->getDecl()->getIdentifier()->deuglifiedName();
+ else
+ Node->getNameInfo().printName(OS, Policy);
if (Node->hasExplicitTemplateArgs()) {
const TemplateParameterList *TPL = nullptr;
if (!Node->hadMultipleCandidates())
@@ -2069,7 +2074,10 @@
} else {
NeedComma = true;
}
- std::string ParamStr = P->getNameAsString();
+ std::string ParamStr =
+ (Policy.CleanUglifiedParameters && P->getIdentifier())
+ ? P->getIdentifier()->deuglifiedName().str()
+ : P->getNameAsString();
P->getOriginalType().print(OS, Policy, ParamStr);
}
if (Method->isVariadic()) {
Index: clang/lib/AST/DeclPrinter.cpp
===================================================================
--- clang/lib/AST/DeclPrinter.cpp
+++ clang/lib/AST/DeclPrinter.cpp
@@ -885,7 +885,10 @@
}
}
- printDeclType(T, D->getName());
+ printDeclType(T, (isa<ParmVarDecl>(D) && Policy.CleanUglifiedParameters &&
+ D->getIdentifier())
+ ? D->getIdentifier()->deuglifiedName()
+ : D->getName());
Expr *Init = D->getInit();
if (!Policy.SuppressInitializers && Init) {
bool ImplicitInit = false;
@@ -1131,8 +1134,13 @@
else if (TTP->getDeclName())
Out << ' ';
- if (TTP->getDeclName())
- Out << TTP->getDeclName();
+ if (TTP->getDeclName()) {
+ if (Policy.CleanUglifiedParameters && isa<TemplateTemplateParmDecl>(D) &&
+ TTP->getIdentifier())
+ Out << TTP->getIdentifier()->deuglifiedName();
+ else
+ Out << TTP->getDeclName();
+ }
} else if (auto *TD = D->getTemplatedDecl())
Visit(TD);
else if (const auto *Concept = dyn_cast<ConceptDecl>(D)) {
@@ -1742,8 +1750,12 @@
else if (TTP->getDeclName())
Out << ' ';
- if (TTP->getDeclName())
- Out << TTP->getDeclName();
+ if (TTP->getDeclName()) {
+ if (Policy.CleanUglifiedParameters && TTP->getIdentifier())
+ Out << TTP->getIdentifier()->deuglifiedName();
+ else
+ Out << TTP->getDeclName();
+ }
if (TTP->hasDefaultArgument()) {
Out << " = ";
@@ -1755,7 +1767,8 @@
const NonTypeTemplateParmDecl *NTTP) {
StringRef Name;
if (IdentifierInfo *II = NTTP->getIdentifier())
- Name = II->getName();
+ Name =
+ Policy.CleanUglifiedParameters ? II->deuglifiedName() : II->getName();
printDeclType(NTTP->getType(), Name, NTTP->isParameterPack());
if (NTTP->hasDefaultArgument()) {
Index: clang/include/clang/Basic/IdentifierTable.h
===================================================================
--- clang/include/clang/Basic/IdentifierTable.h
+++ clang/include/clang/Basic/IdentifierTable.h
@@ -458,6 +458,10 @@
/// 7.1.3, C++ [lib.global.names]).
ReservedIdentifierStatus isReserved(const LangOptions &LangOpts) const;
+ /// If the identifier is an "uglified" reserved name, return a cleaned form.
+ /// e.g. _Foo => Foo. Otherwise, just returns the name.
+ StringRef deuglifiedName() const;
+
/// Provide less than operator for lexicographical sorting.
bool operator<(const IdentifierInfo &RHS) const {
return getName() < RHS.getName();
Index: clang/include/clang/AST/PrettyPrinter.h
===================================================================
--- clang/include/clang/AST/PrettyPrinter.h
+++ clang/include/clang/AST/PrettyPrinter.h
@@ -75,7 +75,8 @@
MSVCFormatting(false), ConstantsAsWritten(false),
SuppressImplicitBase(false), FullyQualifiedName(false),
PrintCanonicalTypes(false), PrintInjectedClassNameWithArguments(true),
- UsePreferredNames(true), AlwaysIncludeTypeForTemplateArgument(false) {}
+ UsePreferredNames(true), AlwaysIncludeTypeForTemplateArgument(false),
+ CleanUglifiedParameters(false) {}
/// Adjust this printing policy for cases where it's known that we're
/// printing C++ code (for instance, if AST dumping reaches a C++-only
@@ -282,6 +283,11 @@
/// parameters.
unsigned AlwaysIncludeTypeForTemplateArgument : 1;
+ /// Whether to strip underscores when printing reserved parameter names.
+ /// e.g. std::vector<class _Tp> becomes std::vector<class Tp>.
+ /// This only affects parameter names, and so describes a compatible API.
+ unsigned CleanUglifiedParameters : 1;
+
/// Callbacks to use to allow the behavior of printing to be customized.
const PrintingCallbacks *Callbacks = nullptr;
};
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1458,8 +1458,8 @@
namespace std {
class string {};
// Move overloads have special handling.
- template <typename T> T&& move(T&&);
- template <typename I, typename O> O move(I, I, O);
+ template <typename _T> T&& move(_T&& __value);
+ template <typename _I, typename _O> _O move(_I, _I, _O);
}
)cpp",
/*Main=*/"");
@@ -1469,7 +1469,8 @@
QName("std"),
AllOf(QName("std::string"), DeclURI(TestHeaderURI),
IncludeHeader("<string>")),
- AllOf(Labeled("move(T &&)"), IncludeHeader("<utility>")),
+ // Parameter names are demangled.
+ AllOf(Labeled("move(T &&value)"), IncludeHeader("<utility>")),
AllOf(Labeled("move(I, I, O)"), IncludeHeader("<algorithm>"))));
}
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1044,6 +1044,29 @@
HI.Kind = index::SymbolKind::Field;
HI.Definition = "m_int arr[Size]";
HI.Type = {"m_int[Size]", "int[Size]"};
+ }},
+ {// Uglified standard library parameter names.
+ R"cpp(
+ template<class _Tp> class Tmpl {
+ template<int _I>
+ typename _Tp::iterator ^[[put]](_Tp __value);
+ };
+ )cpp",
+ [](HoverInfo &HI) {
+ HI.Name = "put";
+ HI.NamespaceScope = "";
+ HI.LocalScope = "Tmpl<Tp>::";
+ HI.AccessSpecifier = "private";
+ HI.Kind = index::SymbolKind::InstanceMethod;
+ HI.Definition = "template <int I> typename Tp::iterator put(Tp value)";
+ HI.ReturnType = "typename Tp::iterator";
+ HI.Type = "typename Tp::iterator (Tp)";
+ HI.Parameters = {
+ {{"Tp"}, {"value"}, llvm::None},
+ };
+ HI.TemplateParameters = {
+ {{"int"}, {"I"}, llvm::None},
+ };
}}};
for (const auto &Case : Cases) {
SCOPED_TRACE(Case.Code);
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -62,13 +62,14 @@
Base.PolishForDeclaration = true;
Base.ConstantsAsWritten = true;
Base.SuppressTemplateArgsInCXXConstructors = true;
+ Base.CleanUglifiedParameters = true;
return Base;
}
/// Given a declaration \p D, return a human-readable string representing the
/// local scope in which it is declared, i.e. class(es) and method name. Returns
/// an empty string if it is not local.
-std::string getLocalScope(const Decl *D) {
+std::string getLocalScope(const Decl *D, const PrintingPolicy &PP) {
std::vector<std::string> Scopes;
const DeclContext *DC = D->getDeclContext();
@@ -80,9 +81,9 @@
if (const ObjCContainerDecl *CD = dyn_cast<ObjCContainerDecl>(DC))
return printObjCContainer(*CD);
- auto GetName = [](const TypeDecl *D) {
+ auto GetName = [&](const TypeDecl *D) {
if (!D->getDeclName().isEmpty()) {
- PrintingPolicy Policy = D->getASTContext().getPrintingPolicy();
+ PrintingPolicy Policy = PP;
Policy.SuppressScope = true;
return declaredType(D).getAsString(Policy);
}
@@ -224,8 +225,8 @@
if (const auto *TTP = dyn_cast<TemplateTypeParmDecl>(Param)) {
P.Type = printType(TTP);
- if (!TTP->getName().empty())
- P.Name = TTP->getNameAsString();
+ if (IdentifierInfo *II = TTP->getIdentifier())
+ P.Name = II->deuglifiedName().str();
if (TTP->hasDefaultArgument())
P.Default = TTP->getDefaultArgument().getAsString(PP);
@@ -233,7 +234,7 @@
P.Type = printType(NTTP, PP);
if (IdentifierInfo *II = NTTP->getIdentifier())
- P.Name = II->getName().str();
+ P.Name = II->deuglifiedName().str();
if (NTTP->hasDefaultArgument()) {
P.Default.emplace();
@@ -243,8 +244,8 @@
} else if (const auto *TTPD = dyn_cast<TemplateTemplateParmDecl>(Param)) {
P.Type = printType(TTPD, PP);
- if (!TTPD->getName().empty())
- P.Name = TTPD->getNameAsString();
+ if (IdentifierInfo *II = TTPD->getIdentifier())
+ P.Name = II->deuglifiedName().str();
if (TTPD->hasDefaultArgument()) {
P.Default.emplace();
@@ -347,8 +348,8 @@
const PrintingPolicy &PP) {
HoverInfo::Param Out;
Out.Type = printType(PVD->getType(), PVD->getASTContext(), PP);
- if (!PVD->getName().empty())
- Out.Name = PVD->getNameAsString();
+ if (const IdentifierInfo *II = PVD->getIdentifier())
+ Out.Name = II->deuglifiedName().str();
if (const Expr *DefArg = getDefaultArg(PVD)) {
Out.Default.emplace();
llvm::raw_string_ostream OS(*Out.Default);
@@ -576,7 +577,7 @@
HI.NamespaceScope = getNamespaceScope(D);
if (!HI.NamespaceScope->empty())
HI.NamespaceScope->append("::");
- HI.LocalScope = getLocalScope(D);
+ HI.LocalScope = getLocalScope(D, PP);
if (!HI.LocalScope.empty())
HI.LocalScope.append("::");
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
