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

Chandler Carruth via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 16 15:58:43 PDT 2018


chandlerc added a comment.

In https://reviews.llvm.org/D50828#1203392, @zturner wrote:

> In https://reviews.llvm.org/D50828#1203354, @erik.pilkington wrote:
>
> > In https://reviews.llvm.org/D50828#1203312, @rsmith wrote:
> >
> > > I think the only reasonable way to handle this is for `Demangle` to move into `Support`. We could keep the headers separate but move the implementation into `lib/Support/Demangle`, similar to what we do for `AST`. Or we could also move the headers to `include/llvm/Support/Demangle`. My preference is the latter; it seems reasonable to me for `Demangle` to generally live inside `Support`.
> >
> >
> > Sure, I really don't know why we didn't just do that to start with. The original RFC[1] talked about putting the demangler into Support, but the commit that materialized it (r280732) put Demangle into it's own library. Would be nice to get @zturner's sign-off for a move too, since he owns the microsoft demangler.
> >
> > [1]: https://groups.google.com/forum/#!topic/llvm-dev/v_7OuWx8n1A
>
>
> My understanding is that `Demangle` is used by other projects like compiler-rt which can't depend on Support.  If I'm wrong, it would be great.  For some reason I can't find it when I search the archive, but if you search your inbox for "llvm r328123" you'll find a long discussion thread with various people who were dealing with this and/or may know something about it.  Anyway, I'm CC'ing them all here.
>
> Personally if we can get Demangle into support I would be very happy.


That's why all of this is behind an `#ifdef`? It seems to be trying to preserve the ability to build a stand-alone demangler that does not in fact pull in Support or anything else.


Repository:
  rL LLVM

https://reviews.llvm.org/D50828





More information about the llvm-commits mailing list