[PATCH] D49575: Merge demangler changes over to libcxxabi

[Zachary Turner via llvm-commits]

Yea, it's fine either way.  I was just looking at it from the perspective
of minimizing differences from the master copy in LLVM.  Definitely having
headers be self-contained is worthy, but it comes at the (very small)
expense of adding an extra difference from the master copy.  That said, I
don't feel strongly, so if you want me to add it, that's fine.

On Fri, Jul 20, 2018 at 9:39 AM Erik Pilkington via Phabricator <
reviews at reviews.llvm.org> wrote:

> erik.pilkington added a comment.
>
> In https://reviews.llvm.org/D49575#1169729, @llvm-commits wrote:
>
> > Or just doing nothing. It’s a private header and is only included in 1
> TU.
> >  In that fine, cxxabi-config is already included first
>
>
> I guess we're starting to split hairs here, but I think its better to make
> headers self-contained rather then implicitly relying on #include ordering,
> even for a one-off header like this.
>
>
>
> ================
> Comment at: libcxxabi/src/demangle/Compiler.h:23
> +#ifndef NDEBUG
> +#if __has_attribute(noinline) && __has_attribute(used)
> +#define DUMP_METHOD __attribute__((noinline, used))
> ----------------
> thakis wrote:
> > erik.pilkington wrote:
> > > This depends on `__cxxabi_config.h` to make sure that
> `__has_attribute` is defined on compilers that don't support it. Please add
> the include.
> > Is there any harm to just #ifndef #def here instead of pulling in that
> whole header?
> No, that would be fine too.
>
>
> https://reviews.llvm.org/D49575
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180720/b85fcef0/attachment.html>