[PATCH] D50828: Add profiling and canonicalization support to demangler nodes. No functionality change intended.

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 16 18:04:03 PDT 2018


But do you actually need those changes that pavel is working on? That looks
like a whole bunch of code that someone who just wants to print a demangled
name won’t care about. How involved are bugs usually, because I’m imagining
they’re a) usually just a couple lines change and b) pretty infrequent.
Once the relicense happens, you just change the include path and include
the real one.

I mean I agree it’s a bit ugly, but I don’t think it’s any more ugly than
moving it to support and hacking up the file with ifdefs so that it both is
and isn’t part of support at the same time. After all, we already have two
vaguely similar demanglers in the tree. One in llvm and one in libcxxabi.
So that doesn’t actually get any worse.

I pretty strongly dislike the alternative of moving demangler to support
and “lying” about it being part of support :-:
On Thu, Aug 16, 2018 at 5:53 PM Erik Pilkington via Phabricator <
reviews at reviews.llvm.org> wrote:

> erik.pilkington added a comment.
>
> In https://reviews.llvm.org/D50828#1203493, @zturner wrote:
>
> > Another option is to simply move it into support and say that going
> forward
> >  merges to libcxxabi might get more difficult. Perhaps this isn’t so bad
> >  though. Isn’t the itanium demangler mostly complete? If the potential
> for
> >  future changes to it is low, maybe this is an acceptable tradeoff.
>
>
> It's pretty much complete, but there is still going to be a steady stream
> of updates. Each release of C++ is likely to add a handful more constructs
> that need manglings, and often new attributes/extensions get custom
> manglings that need to be handled as well.
>
> > Then we
> >  could just move it to support, use it and evolve it any way we want
> with no
> >  restrictions, and when new functionality gets added to the itanium
> mangler
> >  that needs to be port to libcxxabi, it’s a little more effort than a
> simple
> >  copy paste because you have to think about it some.
> >
> > Thoughts?
>
> -1 from me, large-scale refactorings such as this, or Pavel's plans here:
> https://reviews.llvm.org/D50599 would make updating/fixing bugs in the
> demangler a nightmare. I really don't want to have to implement every
> feature twice. Not to mention its just plain ugly to have two vaguely
> similar demanglers in the source tree.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D50828
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180816/34e82087/attachment.html>


More information about the llvm-commits mailing list