[PATCH] D120905: [demangler] Add operator precedence

Mon Mar 28 14:54:11 PDT 2022

dblaikie added a comment.

In D120905#3412571 <https://reviews.llvm.org/D120905#3412571>, @urnathan wrote:

> - F22582853: 0001-WIP-match.patch <https://reviews.llvm.org/F22582853>

Ah, thanks for posting - I committed an incomplete fix (didn't touch all the other ctors) in 1d1cf9b6c42c820f38eb776cb7504564441a23ee <https://reviews.llvm.org/rG1d1cf9b6c42c820f38eb776cb7504564441a23ee> - I think the right thing is probably to do-nothing in `print(Node::Prec prec)` - since I assume/my understanding is that the printing already accounts for precedence by adding parentheses etc where needed?

================
Comment at: llvm/include/llvm/Demangle/ItaniumDemangle.h:1785

   template<typename Fn> void match(Fn F) const { F(LHS, InfixOperator, RHS); }

----------------
ldionne wrote:
> goncharov wrote:
> > I am not familiar with the code at all, but it seems to be an issue. We have downstream code that relies on the ability to match mode from ctor, but this "match" and other definitions have not been updated along with ctors. Do you mind reverting the patch for now?
> > 
> Can you give a bit more context regarding what issue you're encountering? Since this was landed some time ago, I think it would be best to fix-forward if we can.
4 hours isn't an especially large window for a revert, but yeah - some more context would be good. I'm working on providing that now/too.

But, yeah, it looks like, judging by other "match" functions it should have the same arg list as the ctor.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120905/new/

https://reviews.llvm.org/D120905