[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

1790

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;
+    // Parenthesize pointer-to-member deference argument.
+    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

 //===--- ItaniumDemangle.h -----------*- mode:c++;eval:(read-only-mode) -*-===//
 //       Do not edit! See README.txt.
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Generic itanium demangler library.
 // There are two copies of this file in the source tree.  The one under
 // libcxxabi is the original and the one under llvm is the copy.  Use
 // cp-to-llvm.sh to update the copy.  See README.txt for more details.
 //
 //===----------------------------------------------------------------------===//
 
-#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:
 //   - C++ modules TS
 
 #include "DemangleConfig.h"
 #include "StringView.h"
 #include "Utility.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;
+    // Parenthesize pointer-to-member deference argument.
+    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;

 template <typename Alloc>
 struct ManglingParser : AbstractManglingParser<ManglingParser<Alloc>, Alloc> {
   using AbstractManglingParser<ManglingParser<Alloc>,
                                Alloc>::AbstractManglingParser;
 };
 
 DEMANGLE_NAMESPACE_END
 
-#endif // LLVM_DEMANGLE_ITANIUMDEMANGLE_H
+#endif // DEMANGLE_ITANIUMDEMANGLE_H

llvm/include/llvm/Demangle/StringView.h

 //===--- StringView.h ----------------*- mode:c++;eval:(read-only-mode) -*-===//
 //       Do not edit! See README.txt.
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // FIXME: Use std::string_view instead when we support C++17.
 // There are two copies of this file in the source tree.  The one under
 // libcxxabi is the original and the one under llvm is the copy.  Use
 // cp-to-llvm.sh to update the copy.  See README.txt for more details.
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_DEMANGLE_STRINGVIEW_H
-#define LLVM_DEMANGLE_STRINGVIEW_H
+#ifndef DEMANGLE_STRINGVIEW_H
+#define DEMANGLE_STRINGVIEW_H
 
 #include "DemangleConfig.h"
 #include <cassert>
 #include <cstring>
 
 DEMANGLE_NAMESPACE_BEGIN
 
 class StringView {

llvm/include/llvm/Demangle/Utility.h

 //===--- Utility.h -------------------*- mode:c++;eval:(read-only-mode) -*-===//
 //       Do not edit! See README.txt.
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Provide some utility classes for use in the demangler.
 // There are two copies of this file in the source tree.  The one in libcxxabi
 // is the original and the one in llvm is the copy.  Use cp-to-llvm.sh to update
 // the copy.  See README.txt for more details.
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_DEMANGLE_UTILITY_H
-#define LLVM_DEMANGLE_UTILITY_H
+#ifndef DEMANGLE_UTILITY_H
+#define DEMANGLE_UTILITY_H
 
 #include "StringView.h"
 #include <array>
 #include <cstdint>
 #include <cstdlib>
 #include <cstring>
 #include <exception>
 #include <limits>