[PATCH] D15883: Add ARM EHABI-related constants to unwind.h.

Logan Chien via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 17 16:54:01 PST 2016


logan added inline comments.

================
Comment at: lib/Headers/unwind.h:61
@@ +60,3 @@
+#define _UNWIND_ARM_EHABI 0
+#endif
+
----------------
compnerd wrote:
> logan wrote:
> > Since this is `unwind.h`, I feel that we can get a step further and use `__ARM_EABI_UNWINDER__` to get more compatibility to GCC's unwind.h.
> > 
> > Here's the change:
> > 
> > ```
> > #if defined(__arm__) && !defined(__USING_SJLJ_EXCEPTIONS__) && \
> >     !defined(__ARM_DWARF_EH__)
> > #define __ARM_EABI_UNWINDER__ 1
> > #endif
> > ```
> I dont know if we really need to imitate GCC's macros here.  Am I mistaken in that they assume that `__ARM_EABI_UNWINDER__` has been set to 1 externally if targeting such an environment?  I think that it is better to use the reserved namespace and intrude into libunwind's namespace as already done here.
> Am I mistaken in that they assume that `__ARM_EABI_UNWINDER__` has been set to 1 externally if targeting such an environment?

Although this is an implementation detail, it was defined by `unwind.h` in the implementation of GCC.

Remark: `__ARM_EABI_UNWINDER__` is not a pre-defined macro in GCC and Clang (can be checked with ` gcc -dM -E - < /dev/null`.)

BTW, some applications or libraries need this macro to be defined after including `<unwind.h>` (such as uclibc, boost, or libc++abi 3.0.)  I remembered that someone suggested to use `__ARM_EABI_UNWINDER__` instead of  `LIBCXXABI_ARM_EHABI` when I was fixing libc++abi several years ago.  I chose `LIBCXXABI_ARM_EHABI` simply because `__ARM_EABI_UNWINDER__` wasn't provided by clang at that time.

I am less concerned to namespace pollution, because this is already the de facto implementation in GCC world and the macro names start with underscores are reserved for compiler or standard libraries by convention.

Since this is file a public header and will be used for a long time, I personally believe that it will be better to use an existing name with the same meaning instead of introducing a new name.  In addition, this will make it easier to port the application between gcc and clang.


http://reviews.llvm.org/D15883





More information about the cfe-commits mailing list