[PATCH] D49575: Merge demangler changes over to libcxxabi

Erik Pilkington via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 20 09:39:53 PDT 2018


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





More information about the llvm-commits mailing list