Skip to content

[Clang] Mangling of pack indexing type and expression for itanium #123513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[Clang] Mangling of pack indexing type and expression for itanium #123513

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca55a3f
[Clang] Mangling of pack indexing type and expression for itanium
cor3ntin Jan 19, 2025
f17310f
Fix test file
cor3ntin Jan 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1003,6 +1003,7 @@ Bug Fixes to C++ Support
lambda functions or inline friend functions defined inside templates (#GH122493).
- Clang now rejects declaring an alias template with the same name as its template parameter. (#GH123423)
- Correctly determine the implicit constexprness of lambdas in dependent contexts. (#GH97958) (#GH114234)
- Implement Itanium mangling for pack indexing. (#GH112003)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should live in C++2c support section, as this isn't strictly a bug.


Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
35 changes: 19 additions & 16 deletions clang/lib/AST/ItaniumMangle.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,7 @@ class CXXNameMangler {
void mangleInitListElements(const InitListExpr *InitList);
void mangleRequirement(SourceLocation RequiresExprLoc,
const concepts::Requirement *Req);
void mangleReferenceToPack(const NamedDecl *ND);
void mangleExpression(const Expr *E, unsigned Arity = UnknownArity,
bool AsTemplateArg = false);
void mangleCXXCtorType(CXXCtorType T, const CXXRecordDecl *InheritedFrom);
Expand Down Expand Up @@ -4348,10 +4349,10 @@ void CXXNameMangler::mangleType(const PackExpansionType *T) {
}

void CXXNameMangler::mangleType(const PackIndexingType *T) {
if (!T->hasSelectedType())
mangleType(T->getPattern());
else
mangleType(T->getSelectedType());
// <type> ::= Dy <type> <expression> # pack indexing type (C++23)
Out << "Dy";
mangleType(T->getPattern());
mangleExpression(T->getIndexExpr());
}

void CXXNameMangler::mangleType(const ObjCInterfaceType *T) {
Expand Down Expand Up @@ -4787,6 +4788,7 @@ void CXXNameMangler::mangleRequirement(SourceLocation RequiresExprLoc,

void CXXNameMangler::mangleExpression(const Expr *E, unsigned Arity,
bool AsTemplateArg) {
// clang-format off
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Why turning it off?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-format initially completely destroy the comment below, which has been neatly aligned

// <expression> ::= <unary operator-name> <expression>
// ::= <binary operator-name> <expression> <expression>
// ::= <trinary operator-name> <expression> <expression> <expression>
Expand All @@ -4806,6 +4808,8 @@ void CXXNameMangler::mangleExpression(const Expr *E, unsigned Arity,
// ::= ds <expression> <expression> # expr.*expr
// ::= sZ <template-param> # size of a parameter pack
// ::= sZ <function-param> # size of a function parameter pack
// ::= sy <template-param> <expression> # pack indexing expression
// ::= sy <function-param> <expression> # pack indexing expression
// ::= u <source-name> <template-arg>* E # vendor extended expression
// ::= <expr-primary>
// <expr-primary> ::= L <type> <value number> E # integer literal
Expand All @@ -4815,6 +4819,7 @@ void CXXNameMangler::mangleExpression(const Expr *E, unsigned Arity,
// ::= L <pointer type> 0 E # null pointer template argument
// ::= L <type> <real-part float> _ <imag-part float> E # complex floating point literal (C99); not used by clang
// ::= L <mangled-name> E # external name
// clang-format on
QualType ImplicitlyConvertedToType;

// A top-level expression that's not <expr-primary> needs to be wrapped in
Expand Down Expand Up @@ -4886,7 +4891,6 @@ void CXXNameMangler::mangleExpression(const Expr *E, unsigned Arity,
case Expr::OMPIteratorExprClass:
case Expr::CXXInheritedCtorInitExprClass:
case Expr::CXXParenListInitExprClass:
case Expr::PackIndexingExprClass:
llvm_unreachable("unexpected statement kind");

case Expr::ConstantExprClass:
Expand Down Expand Up @@ -5788,17 +5792,7 @@ void CXXNameMangler::mangleExpression(const Expr *E, unsigned Arity,
}

Out << "sZ";
const NamedDecl *Pack = SPE->getPack();
if (const TemplateTypeParmDecl *TTP = dyn_cast<TemplateTypeParmDecl>(Pack))
mangleTemplateParameter(TTP->getDepth(), TTP->getIndex());
else if (const NonTypeTemplateParmDecl *NTTP
= dyn_cast<NonTypeTemplateParmDecl>(Pack))
mangleTemplateParameter(NTTP->getDepth(), NTTP->getIndex());
else if (const TemplateTemplateParmDecl *TempTP
= dyn_cast<TemplateTemplateParmDecl>(Pack))
mangleTemplateParameter(TempTP->getDepth(), TempTP->getIndex());
else
mangleFunctionParam(cast<ParmVarDecl>(Pack));
mangleReferenceToPack(SPE->getPack());
break;
}

Expand Down Expand Up @@ -5828,6 +5822,15 @@ void CXXNameMangler::mangleExpression(const Expr *E, unsigned Arity,
break;
}

case Expr::PackIndexingExprClass: {
auto *PE = cast<PackIndexingExpr>(E);
NotPrimaryExpr();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am looking at the test cases and I don't see the X, can we make sure we cover that branch in NotPrimaryExpr e.g. if (AsTemplateArg && IsPrimaryExpr). We should make sure we cover that in the demangling as well.

Out << "sy";
mangleReferenceToPack(PE->getPackDecl());
mangleExpression(PE->getIndexExpr());
break;
}

case Expr::CXXThisExprClass:
NotPrimaryExpr();
Out << "fpT";
Expand Down
11 changes: 11 additions & 0 deletions clang/test/CodeGenCXX/mangle-cxx2c.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-linux-gnu -std=c++2a | FileCheck %s

namespace spaceship {
struct X {};
struct Y {};
int operator<=>(X, Y);

// CHECK-LABEL: define {{.*}} @_ZN9spaceship1fIiEEvDTcmltcvNS_1YE_EcvNS_1XE_EcvT__EE
template<typename T> void f(decltype(Y() < X(), T()) x) {}
template void f<int>(int);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't related to what this patch proposes or am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oups

43 changes: 43 additions & 0 deletions libcxxabi/src/demangle/ItaniumDemangle.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1521,6 +1521,27 @@ class ParameterPackExpansion final : public Node {
}
};

class PackIndexing final : public Node {
const Node *Pattern;
const Node *Index;

public:
PackIndexing(const Node *Pattern_, const Node *Index_)
: Node(KPackIndexing), Pattern(Pattern_), Index(Index_) {}

template <typename Fn> void match(Fn F) const { F(Pattern, Index); }

void printLeft(OutputBuffer &OB) const override {
OB.printOpen('(');
ParameterPackExpansion PPE(Pattern);
PPE.printLeft(OB);
OB.printClose(')');
OB.printOpen('[');
Index->printLeft(OB);
OB.printClose(']');
}
};

class TemplateArgs final : public Node {
NodeArray Params;
Node *Requires;
Expand Down Expand Up @@ -4510,6 +4531,18 @@ Node *AbstractManglingParser<Derived, Alloc>::parseType() {
Result = make<ParameterPackExpansion>(Child);
break;
}
// ::= Dy <type> <expression> # pack indexing (C++26)
case 'y': {
First += 2;
Node *Pattern = getDerived().parseType();
if (!Pattern)
return nullptr;
Node *Index = getDerived().parseExpr();
if (!Index)
return nullptr;
Result = make<PackIndexing>(Pattern, Index);
break;
}
// Exception specifier on a function type.
case 'o':
case 'O':
Expand Down Expand Up @@ -5354,6 +5387,16 @@ Node *AbstractManglingParser<Derived, Alloc>::parseExpr() {
return nullptr;
return make<ParameterPackExpansion>(Child);
}
if (consumeIf("sy")) {
Node *Pattern = look() == 'T' ? getDerived().parseTemplateParam()
: getDerived().parseFunctionParam();
if (Pattern == nullptr)
return nullptr;
Node *Index = getDerived().parseExpr();
if (Index == nullptr)
return nullptr;
return make<PackIndexing>(Pattern, Index);
}
if (consumeIf("sZ")) {
if (look() == 'T') {
Node *R = getDerived().parseTemplateParam();
Expand Down
2 changes: 1 addition & 1 deletion libcxxabi/src/demangle/ItaniumNodes.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,5 +100,5 @@ NODE(ExprRequirement)
NODE(TypeRequirement)
NODE(NestedRequirement)
NODE(ExplicitObjectParameter)

NODE(PackIndexing)
#undef NODE
5 changes: 5 additions & 0 deletions libcxxabi/test/test_demangle.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30220,6 +30220,11 @@ const char* cases[][2] = {
{"_ZZNH3Foo3fooES_iENK4Foo24foo2Ev", "Foo::foo(this Foo, int)::Foo2::foo2() const" },
{"_ZNH3FooclERKS_", "Foo::operator()(this Foo const&)"},


// C++26 pack indexing
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add invalid cases as well see invalid_cases and make sure we cover each case that can return a nullptr in AbstractManglingParser (I think)

{"_Z3fooILi0ETpTnDaJLi1ELi2EEEDTsyT0_T_Ev", "decltype((1, 2...)[0]) foo<0, 1, 2>()"},
{"_Z1gILi0EJciEEDyT0_T_v", "(char, int)[0] g<0, char, int>()"},

// fixed-point types as defined in the N1169 draft of ISO/IEC DTR 18037
{"_Z1fDAs", "f(short _Accum)"},
{"_Z1fDAt", "f(unsigned short _Accum)"},
Expand Down
43 changes: 43 additions & 0 deletions llvm/include/llvm/Demangle/ItaniumDemangle.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1521,6 +1521,27 @@ class ParameterPackExpansion final : public Node {
}
};

class PackIndexing final : public Node {
const Node *Pattern;
const Node *Index;

public:
PackIndexing(const Node *Pattern_, const Node *Index_)
: Node(KPackIndexing), Pattern(Pattern_), Index(Index_) {}

template <typename Fn> void match(Fn F) const { F(Pattern, Index); }

void printLeft(OutputBuffer &OB) const override {
OB.printOpen('(');
ParameterPackExpansion PPE(Pattern);
PPE.printLeft(OB);
OB.printClose(')');
OB.printOpen('[');
Index->printLeft(OB);
OB.printClose(']');
}
};

class TemplateArgs final : public Node {
NodeArray Params;
Node *Requires;
Expand Down Expand Up @@ -4510,6 +4531,18 @@ Node *AbstractManglingParser<Derived, Alloc>::parseType() {
Result = make<ParameterPackExpansion>(Child);
break;
}
// ::= Dy <type> <expression> # pack indexing (C++26)
case 'y': {
First += 2;
Node *Pattern = getDerived().parseType();
if (!Pattern)
return nullptr;
Node *Index = getDerived().parseExpr();
if (!Index)
return nullptr;
Result = make<PackIndexing>(Pattern, Index);
break;
}
// Exception specifier on a function type.
case 'o':
case 'O':
Expand Down Expand Up @@ -5354,6 +5387,16 @@ Node *AbstractManglingParser<Derived, Alloc>::parseExpr() {
return nullptr;
return make<ParameterPackExpansion>(Child);
}
if (consumeIf("sy")) {
Node *Pattern = look() == 'T' ? getDerived().parseTemplateParam()
: getDerived().parseFunctionParam();
if (Pattern == nullptr)
return nullptr;
Node *Index = getDerived().parseExpr();
if (Index == nullptr)
return nullptr;
return make<PackIndexing>(Pattern, Index);
}
if (consumeIf("sZ")) {
if (look() == 'T') {
Node *R = getDerived().parseTemplateParam();
Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm/Demangle/ItaniumNodes.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,5 +100,5 @@ NODE(ExprRequirement)
NODE(TypeRequirement)
NODE(NestedRequirement)
NODE(ExplicitObjectParameter)

NODE(PackIndexing)
#undef NODE
Loading