[demangler] Improve ->* & .* demangling

[urnathan]

Summary

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.

[ChuanqiXu]

libcxxabi/src/demangle/ItaniumDemangle.h

1804

Should we add a comment here? It is confusing for reader who don't have a background.

[urnathan]

clarifying comment

[bruno]

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.

Is this worth putting in a FIXME somewhere as part of this patch? Or is this being tracked elsewhere?

Other than that, LGTM.

[urnathan]

In D118486#3302766, @bruno wrote:

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.

Is this worth putting in a FIXME somewhere as part of this patch? Or is this being tracked elsewhere?

I'm actively working on it right now :)

libcxxabi/src/demangle/ItaniumDemangle.h

public:
   MemberExpr(const Node *LHS_, StringView Kind_, const Node *RHS_)
       : Node(KMemberExpr), LHS(LHS_), Kind(Kind_), RHS(RHS_) {}
 
   template<typename Fn> void match(Fn F) const { F(LHS, Kind, RHS); }
 
   void printLeft(OutputBuffer &OB) const override {
     LHS->print(OB);
     OB += Kind;
+    bool IsPtr = Kind.back() == '*';
+    if (IsPtr)
+      OB += '(';
     RHS->print(OB);
+    if (IsPtr)
+      OB += ')';
   }
 };
 
 class SubobjectExpr : public Node {
   const Node *Type;
   const Node *SubExpr;
   StringView Offset;
   NodeArray UnionSelectors;
case 'r':
       return getDerived().parseBinaryExpr("|");
     case 'R':
       First += 2;
       return getDerived().parseBinaryExpr("|=");
     }
     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("+");
     case 'L':
       First += 2;
       return getDerived().parseBinaryExpr("+=");
     case 'p': {
       First += 2;

libcxxabi/test/test_demangle.pass.cpp

     {"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXmLT_Li4EEvE4typeE", "void Casts::implicit<4u>(enable_if<(4u) *= (4), void>::type*)"},
     {"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXmmT_EvE4typeE", "void Casts::implicit<4u>(enable_if<(4u)--, void>::type*)"},
     {"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXneT_Li4EEvE4typeE", "void Casts::implicit<4u>(enable_if<(4u) != (4), void>::type*)"},
     {"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXngT_EvE4typeE", "void Casts::implicit<4u>(enable_if<-(4u), void>::type*)"},
     {"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXntT_EvE4typeE", "void Casts::implicit<4u>(enable_if<!(4u), void>::type*)"},
     {"_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*)"},
     {"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXpsT_EvE4typeE", "void Casts::implicit<4u>(enable_if<+(4u), void>::type*)"},
 //    {"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXptT_Li4EEvE4typeE", "void Casts::implicit<4u>(enable_if<(4u) -> (4), void>::type*)"},
     {"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXquT_Li4ELi5EEvE4typeE", "void Casts::implicit<4u>(enable_if<(4u) ? (4) : (5), void>::type*)"},
     {"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXrmT_Li4EEvE4typeE", "void Casts::implicit<4u>(enable_if<(4u) % (4), void>::type*)"},
     {"_ZN5Casts8implicitILj4EEEvPN9enable_ifIXrMT_Li4EEvE4typeE", "void Casts::implicit<4u>(enable_if<(4u) %= (4), void>::type*)"},
//    {"_Z1fM1XRFivEMS_OFivEMS_KOFivE", "f(int (X::*)() &, int (X::*)() &&, int (X::*)() const &&)"},
     {"_ZN5test21cIPFfvEEEvT_PFvDTclfL1p_EEE", "void test2::c<float (*)()>(float (*)(), void (*)(decltype(fp())))"},
     {"_ZN5test21dIPFfvEEEvT_PFDTclfL0p_EEvE", "void test2::d<float (*)()>(float (*)(), decltype(fp()) (*)())"},
     {"_ZN5test21eIPFfvEEEvPFDTclfp_EET_E", "void test2::e<float (*)()>(decltype(fp()) (*)(float (*)()))"},
     {"_ZN5test21fIPFfvEEEvPFvT_DTclfL0p_EEE", "void test2::f<float (*)()>(void (*)(float (*)(), decltype(fp())))"},
     {"_ZN5test21gIPFfvEEEvT_DTclfL0p_EE", "void test2::g<float (*)()>(float (*)(), decltype(fp()))"},
     {"_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)))"},
     {"_ZN5test62f2IiEEvDTcvT_dtdtL_ZNS_1zEE2ub1iE", "void test6::f2<int>(decltype((int)(test6::z.ub.i)))"},
     {"_ZN5test62f3IiEEvDTcvT_dtdtL_ZNS_1zEE1s1iE", "void test6::f3<int>(decltype((int)(test6::z.s.i)))"},
     {"_ZN5test62f4IiEEvDTcvT_dtdtL_ZNS_1zEE4uuss1iE", "void test6::f4<int>(decltype((int)(test6::z.uuss.i)))"},
     {"_ZN5test62f5IiEEvDTcvT_dtptL_ZNS_2zpEE2ua1iE", "void test6::f5<int>(decltype((int)(test6::zp->ua.i)))"},
     {"_ZN5test62f6IiEEvDTcvT_dtptL_ZNS_2zpEE2ub1iE", "void test6::f6<int>(decltype((int)(test6::zp->ub.i)))"},

llvm/include/llvm/Demangle/ItaniumDemangle.h

public:
   MemberExpr(const Node *LHS_, StringView Kind_, const Node *RHS_)
       : Node(KMemberExpr), LHS(LHS_), Kind(Kind_), RHS(RHS_) {}
 
   template<typename Fn> void match(Fn F) const { F(LHS, Kind, RHS); }
 
   void printLeft(OutputBuffer &OB) const override {
     LHS->print(OB);
     OB += Kind;
+    bool IsPtr = Kind.back() == '*';
+    if (IsPtr)
+      OB += '(';
     RHS->print(OB);
+    if (IsPtr)
+      OB += ')';
   }
 };
 
 class SubobjectExpr : public Node {
   const Node *Type;
   const Node *SubExpr;
   StringView Offset;
   NodeArray UnionSelectors;
case 'r':
       return getDerived().parseBinaryExpr("|");
     case 'R':
       First += 2;
       return getDerived().parseBinaryExpr("|=");
     }
     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("+");
     case 'L':
       First += 2;
       return getDerived().parseBinaryExpr("+=");
     case 'p': {
       First += 2;