Skip to content

Commit

Permalink
[demangler] Improve ->* & .* demangling
Browse files Browse the repository at this point in the history
The demangler treats ->* as a BinaryExpr, but .* as a MemberExpr.
That's inconsistent.  This makes the former a MemberExpr too.
However, in order to not regress the paren output, MemberExpr::print
is modified to parenthesize the MemberExpr if the operator ends with
'*'.  Printing is affected thusly:

Before:
    obj.member
    obj->member
    obj.*member
    (obj) ->* (member)

After:
   obj.member   # Unchanged
   obj->member  # Unchanged
   obj.*(member)  # Added paren member operand
   obj->*(member) # Removed paren on object operand, less whitespace

The right solution to the paren problem is to add some notion of
precedence (and associativity) to Nodes, but that's a larger change
that would become simpler once the refactoring I'm doing is completed.

FWIW, binutils' demangler's paren algorithm has a small idea of
precedence, and will generally not emit parens when the operand is
unary.

Reviewed By: bruno

Differential Revision: https://reviews.llvm.org/D118486
  • Loading branch information
urnathan committed Feb 8, 2022
1 parent 6c174ab commit 28669bd
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 13 deletions.
17 changes: 15 additions & 2 deletions libcxxabi/src/demangle/ItaniumDemangle.h
Expand Up @@ -1787,7 +1787,13 @@ class MemberExpr : public Node {
void printLeft(OutputBuffer &OB) const override {
LHS->print(OB);
OB += Kind;
// Parenthesize pointer-to-member deference argument.
bool IsPtr = Kind.back() == '*';
if (IsPtr)
OB += '(';
RHS->print(OB);
if (IsPtr)
OB += ')';
}
};

Expand Down Expand Up @@ -4847,9 +4853,16 @@ Node *AbstractManglingParser<Derived, Alloc>::parseExpr() {
return nullptr;
case 'p':
switch (First[1]) {
case 'm':
case 'm': {
First += 2;
return getDerived().parseBinaryExpr("->*");
Node *LHS = getDerived().parseExpr();
if (LHS == nullptr)
return LHS;
Node *RHS = getDerived().parseExpr();
if (RHS == nullptr)
return nullptr;
return make<MemberExpr>(LHS, "->*", RHS);
}
case 'l':
First += 2;
return getDerived().parseBinaryExpr("+");
Expand Down
4 changes: 2 additions & 2 deletions libcxxabi/test/test_demangle.pass.cpp
Expand Up @@ -29481,7 +29481,7 @@ const char* cases[][2] =
{"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXooT_Li4EEvE4typeE", "void Casts::implicit<4u>(enable_if<(4u) || (4), void>::type*)"},
{"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXorT_Li4EEvE4typeE", "void Casts::implicit<4u>(enable_if<(4u) | (4), void>::type*)"},
{"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXoRT_Li4EEvE4typeE", "void Casts::implicit<4u>(enable_if<(4u) |= (4), void>::type*)"},
{"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXpmT_Li4EEvE4typeE", "void Casts::implicit<4u>(enable_if<(4u) ->* (4), void>::type*)"},
{"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXpmT_Li4EEvE4typeE", "void Casts::implicit<4u>(enable_if<4u->*(4), void>::type*)"},
{"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXplT_Li4EEvE4typeE", "void Casts::implicit<4u>(enable_if<(4u) + (4), void>::type*)"},
{"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXpLT_Li4EEvE4typeE", "void Casts::implicit<4u>(enable_if<(4u) += (4), void>::type*)"},
{"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXppT_EvE4typeE", "void Casts::implicit<4u>(enable_if<(4u)++, void>::type*)"},
Expand Down Expand Up @@ -29658,7 +29658,7 @@ const char* cases[][2] =
{"_ZN5test21hIPFfvEEEvT_DTcvPFDTclfL0p_EEvELi0EE", "void test2::h<float (*)()>(float (*)(), decltype((decltype(fp()) (*)())(0)))"},
{"_ZN5test21iIPFfvEEEvDTcvPFDTclfp_EET_ELi0EE", "void test2::i<float (*)()>(decltype((decltype(fp()) (*)(float (*)()))(0)))"},
{"_ZZN5test21gIPFfvEEEvT_DTclfL0p_EEE8variable", "void test2::g<float (*)()>(float (*)(), decltype(fp()))::variable"},
{"_ZN5test31aINS_1XEMS1_PiEEvT_T0_DTdsfL0p_fL0p0_E", "void test3::a<test3::X, int* test3::X::*>(test3::X, int* test3::X::*, decltype(fp.*fp0))"},
{"_ZN5test31aINS_1XEMS1_PiEEvT_T0_DTdsfL0p_fL0p0_E", "void test3::a<test3::X, int* test3::X::*>(test3::X, int* test3::X::*, decltype(fp.*(fp0)))"},
{"_ZN5test43tf1INS_1XEEEvDTnw_T_piLi1EEE", "void test4::tf1<test4::X>(decltype(new test4::X(1)))"},
{"_ZN5test51aIiEEvDTnxcvT__EE", "void test5::a<int>(decltype(noexcept ((int)())))"},
{"_ZN5test62f1IiEEvDTcvT_dtdtL_ZNS_1zEE2ua1iE", "void test6::f1<int>(decltype((int)(test6::z.ua.i)))"},
Expand Down
23 changes: 18 additions & 5 deletions llvm/include/llvm/Demangle/ItaniumDemangle.h
Expand Up @@ -13,8 +13,8 @@
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_DEMANGLE_ITANIUMDEMANGLE_H
#define LLVM_DEMANGLE_ITANIUMDEMANGLE_H
#ifndef DEMANGLE_ITANIUMDEMANGLE_H
#define DEMANGLE_ITANIUMDEMANGLE_H

// FIXME: (possibly) incomplete list of features that clang mangles that this
// file does not yet support:
Expand Down Expand Up @@ -1787,7 +1787,13 @@ class MemberExpr : public Node {
void printLeft(OutputBuffer &OB) const override {
LHS->print(OB);
OB += Kind;
// Parenthesize pointer-to-member deference argument.
bool IsPtr = Kind.back() == '*';
if (IsPtr)
OB += '(';
RHS->print(OB);
if (IsPtr)
OB += ')';
}
};

Expand Down Expand Up @@ -4847,9 +4853,16 @@ Node *AbstractManglingParser<Derived, Alloc>::parseExpr() {
return nullptr;
case 'p':
switch (First[1]) {
case 'm':
case 'm': {
First += 2;
return getDerived().parseBinaryExpr("->*");
Node *LHS = getDerived().parseExpr();
if (LHS == nullptr)
return LHS;
Node *RHS = getDerived().parseExpr();
if (RHS == nullptr)
return nullptr;
return make<MemberExpr>(LHS, "->*", RHS);
}
case 'l':
First += 2;
return getDerived().parseBinaryExpr("+");
Expand Down Expand Up @@ -5738,4 +5751,4 @@ struct ManglingParser : AbstractManglingParser<ManglingParser<Alloc>, Alloc> {

DEMANGLE_NAMESPACE_END

#endif // LLVM_DEMANGLE_ITANIUMDEMANGLE_H
#endif // DEMANGLE_ITANIUMDEMANGLE_H
4 changes: 2 additions & 2 deletions llvm/include/llvm/Demangle/StringView.h
Expand Up @@ -13,8 +13,8 @@
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_DEMANGLE_STRINGVIEW_H
#define LLVM_DEMANGLE_STRINGVIEW_H
#ifndef DEMANGLE_STRINGVIEW_H
#define DEMANGLE_STRINGVIEW_H

#include "DemangleConfig.h"
#include <cassert>
Expand Down
4 changes: 2 additions & 2 deletions llvm/include/llvm/Demangle/Utility.h
Expand Up @@ -13,8 +13,8 @@
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_DEMANGLE_UTILITY_H
#define LLVM_DEMANGLE_UTILITY_H
#ifndef DEMANGLE_UTILITY_H
#define DEMANGLE_UTILITY_H

#include "StringView.h"
#include <array>
Expand Down

0 comments on commit 28669bd

Please sign in to comment.