[clang] 33c3ef2 - [CodeCompletion][clangd] Clean __uglified parameter names in completion & hover
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 26 06:51:27 PST 2022
Author: Sam McCall
Date: 2022-01-26T15:51:17+01:00
New Revision: 33c3ef2fbeec4ede5fc303b09cdca99ae2c0522a
URL: https://github.com/llvm/llvm-project/commit/33c3ef2fbeec4ede5fc303b09cdca99ae2c0522a
DIFF: https://github.com/llvm/llvm-project/commit/33c3ef2fbeec4ede5fc303b09cdca99ae2c0522a.diff
LOG: [CodeCompletion][clangd] Clean __uglified parameter names in completion & hover
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
Differential Revision: https://reviews.llvm.org/D116387
Added:
clang/test/CodeCompletion/deuglify.cpp
Modified:
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/unittests/AST/DeclPrinterTest.cpp
clang/unittests/AST/StmtPrinterTest.cpp
clang/unittests/AST/TypePrinterTest.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 24cd8b7313bd3..a21c085299b2f 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1457,8 +1457,8 @@ TEST_F(SymbolCollectorTest, CanonicalSTLHeader) {
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=*/"");
@@ -1468,7 +1468,8 @@ TEST_F(SymbolCollectorTest, CanonicalSTLHeader) {
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>"))));
}
diff --git a/clang/include/clang/AST/PrettyPrinter.h b/clang/include/clang/AST/PrettyPrinter.h
index 01dc8e0002703..fd40328d8dcf7 100644
--- a/clang/include/clang/AST/PrettyPrinter.h
+++ b/clang/include/clang/AST/PrettyPrinter.h
@@ -73,7 +73,8 @@ struct PrintingPolicy {
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
@@ -280,6 +281,11 @@ struct PrintingPolicy {
/// 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;
};
diff --git a/clang/include/clang/Basic/IdentifierTable.h b/clang/include/clang/Basic/IdentifierTable.h
index 19c967efcc424..aaf1297c1a648 100644
--- a/clang/include/clang/Basic/IdentifierTable.h
+++ b/clang/include/clang/Basic/IdentifierTable.h
@@ -458,6 +458,10 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
/// 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();
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 044eb8f8f8e55..698b1c3ffd348 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -885,7 +885,10 @@ void DeclPrinter::VisitVarDecl(VarDecl *D) {
}
}
- 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,12 @@ void DeclPrinter::VisitTemplateDecl(const TemplateDecl *D) {
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();
+ }
} else if (auto *TD = D->getTemplatedDecl())
Visit(TD);
else if (const auto *Concept = dyn_cast<ConceptDecl>(D)) {
@@ -1742,8 +1749,12 @@ void DeclPrinter::VisitTemplateTypeParmDecl(const TemplateTypeParmDecl *TTP) {
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 +1766,8 @@ void DeclPrinter::VisitNonTypeTemplateParmDecl(
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()) {
diff --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index b65a38d1e5665..adc0720fe0008 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -1030,7 +1030,12 @@ void StmtPrinter::VisitDeclRefExpr(DeclRefExpr *Node) {
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 @@ void StmtPrinter::VisitLambdaExpr(LambdaExpr *Node) {
} 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()) {
diff --git a/clang/lib/AST/TemplateName.cpp b/clang/lib/AST/TemplateName.cpp
index c8bd74f0b5bb4..05d7d58b71c4d 100644
--- a/clang/lib/AST/TemplateName.cpp
+++ b/clang/lib/AST/TemplateName.cpp
@@ -223,8 +223,12 @@ bool TemplateName::containsUnexpandedParameterPack() const {
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;
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index cf520fcb037ed..bba323f651aad 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -1418,7 +1418,8 @@ void TypePrinter::printTemplateTypeParmBefore(const TemplateTypeParmType *T,
}
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();
diff --git a/clang/lib/Basic/IdentifierTable.cpp b/clang/lib/Basic/IdentifierTable.cpp
index d811aeec84a0f..b86cb7af69bdf 100644
--- a/clang/lib/Basic/IdentifierTable.cpp
+++ b/clang/lib/Basic/IdentifierTable.cpp
@@ -309,6 +309,14 @@ IdentifierInfo::isReserved(const LangOptions &LangOpts) const {
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
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index b86bfe869c69d..01fdf51c60c38 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -1896,6 +1896,7 @@ static PrintingPolicy getCompletionPrintingPolicy(const ASTContext &Context,
Policy.SuppressStrongLifetime = true;
Policy.SuppressUnwrittenScope = true;
Policy.SuppressScope = true;
+ Policy.CleanUglifiedParameters = true;
return Policy;
}
@@ -2837,7 +2838,7 @@ FormatFunctionParameter(const PrintingPolicy &Policy,
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)
@@ -2847,7 +2848,7 @@ FormatFunctionParameter(const PrintingPolicy &Policy,
Result = "(" + formatObjCParamQualifiers(ObjCQual, Type);
Result += Type.getAsString(Policy) + ")";
if (Param->getIdentifier() && !SuppressName)
- Result += Param->getIdentifier()->getName();
+ Result += Param->getIdentifier()->deuglifiedName();
} else {
Type.getAsStringInternal(Result, Policy);
}
@@ -2875,7 +2876,7 @@ FormatFunctionParameter(const PrintingPolicy &Policy,
// 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();
@@ -2887,7 +2888,7 @@ FormatFunctionParameter(const PrintingPolicy &Policy,
if (Result.back() != ')')
Result += " ";
if (Param->getIdentifier())
- Result += Param->getIdentifier()->getName();
+ Result += Param->getIdentifier()->deuglifiedName();
} else {
Type.getAsStringInternal(Result, Policy);
}
@@ -3082,14 +3083,14 @@ static void AddTemplateParameterChunks(
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 {
@@ -3101,7 +3102,7 @@ static void AddTemplateParameterChunks(
PlaceholderStr = "template<...> class";
if (TTP->getIdentifier()) {
PlaceholderStr += ' ';
- PlaceholderStr += TTP->getIdentifier()->getName();
+ PlaceholderStr += TTP->getIdentifier()->deuglifiedName();
}
HasDefaultArg = TTP->hasDefaultArgument();
diff --git a/clang/test/CodeCompletion/deuglify.cpp b/clang/test/CodeCompletion/deuglify.cpp
new file mode 100644
index 0000000000000..c5f801736376c
--- /dev/null
+++ b/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#>)
diff --git a/clang/unittests/AST/DeclPrinterTest.cpp b/clang/unittests/AST/DeclPrinterTest.cpp
index bdc23f33f39b0..0b579565f5b4a 100644
--- a/clang/unittests/AST/DeclPrinterTest.cpp
+++ b/clang/unittests/AST/DeclPrinterTest.cpp
@@ -1336,6 +1336,41 @@ TEST(DeclPrinter, TestTemplateArgumentList16) {
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"),
diff --git a/clang/unittests/AST/StmtPrinterTest.cpp b/clang/unittests/AST/StmtPrinterTest.cpp
index 65dfec4cc5b4a..f3ea74d65696f 100644
--- a/clang/unittests/AST/StmtPrinterTest.cpp
+++ b/clang/unittests/AST/StmtPrinterTest.cpp
@@ -263,3 +263,22 @@ TEST(StmtPrinter, TerseOutputWithLambdas) {
[](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));
+}
diff --git a/clang/unittests/AST/TypePrinterTest.cpp b/clang/unittests/AST/TypePrinterTest.cpp
index 07dc21a88fba1..8500d518d25fe 100644
--- a/clang/unittests/AST/TypePrinterTest.cpp
+++ b/clang/unittests/AST/TypePrinterTest.cpp
@@ -62,4 +62,21 @@ TEST(TypePrinter, TemplateId) {
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));
+}
More information about the cfe-commits
mailing list