[PATCH] D118486: [demangler] Improve ->* & .* demangling

[Nathan Sidwell via Phabricator via llvm-commits]

urnathan created this revision.
urnathan added reviewers: ChuanqiXu, bruno, aaron.ballman, iains.
urnathan requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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.


https://reviews.llvm.org/D118486

Files:
  libcxxabi/src/demangle/ItaniumDemangle.h
  libcxxabi/test/test_demangle.pass.cpp
  llvm/include/llvm/Demangle/ItaniumDemangle.h


Index: llvm/include/llvm/Demangle/ItaniumDemangle.h
===================================================================
--- llvm/include/llvm/Demangle/ItaniumDemangle.h
+++ llvm/include/llvm/Demangle/ItaniumDemangle.h
@@ -1803,7 +1803,12 @@
   void printLeft(OutputBuffer &OB) const override {
     LHS->print(OB);
     OB += Kind;
+    bool IsPtr = Kind.back() == '*';
+    if (IsPtr)
+      OB += '(';
     RHS->print(OB);
+    if (IsPtr)
+      OB += ')';
   }
 };
 
@@ -4856,9 +4861,16 @@
     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("+");
Index: libcxxabi/test/test_demangle.pass.cpp
===================================================================
--- libcxxabi/test/test_demangle.pass.cpp
+++ libcxxabi/test/test_demangle.pass.cpp
@@ -29481,7 +29481,7 @@
     {"_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*)"},
@@ -29658,7 +29658,7 @@
     {"_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)))"},
Index: libcxxabi/src/demangle/ItaniumDemangle.h
===================================================================
--- libcxxabi/src/demangle/ItaniumDemangle.h
+++ libcxxabi/src/demangle/ItaniumDemangle.h
@@ -1801,7 +1801,12 @@
   void printLeft(OutputBuffer &OB) const override {
     LHS->print(OB);
     OB += Kind;
+    bool IsPtr = Kind.back() == '*';
+    if (IsPtr)
+      OB += '(';
     RHS->print(OB);
+    if (IsPtr)
+      OB += ')';
   }
 };
 
@@ -4854,9 +4859,16 @@
     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("+");


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D118486.404059.patch
Type: text/x-patch
Size: 4251 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220128/ab5b11bd/attachment.bin>