<div><div dir="auto">LGTM</div></div><div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Nov 28, 2019 at 6:47 AM Sergej Jaskiewicz via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">broadwaylamb added inline comments.<br>
<br>
<br>
================<br>
Comment at: libunwind/CMakeLists.txt:311<br>
+<br>
+if (LIBUNWIND_USES_ARM_EHABI AND NOT LIBUNWIND_SUPPORTS_FUNWIND_TABLES_FLAG)<br>
+  message(FATAL_ERROR "The -funwind-tables flag must be supported "<br>
----------------<br>
peter.smith wrote:<br>
> Assuming we can't set CMAKE_TRY_COMPILE_TARGET_TYPE is to do something like:<br>
> ```<br>
> if (LIBUNWIND_USES_ARM_EHABI)<br>
>   // unconditionally add -funwind-tables<br>
> ```<br>
> Anyone trying to compile libunwind without a compiler that supports -funwind-tables gets an error message at build rather than configure time, but is there anyone that wouldn't use a form of Clang or GCC?<br>
My opinion is that we want to fail as early as possible. Also, we can show a dedicated error message if we're failing at configure time, rather than leaving the user on their own with the obscure 'unsupported flag' message (why do I need this flag, the user may ask then).<br>
<br>
I too would like to avoid the `CMAKE_TRY_COMPILE_TARGET_TYPE` dance, but I'd like to hear more opinions on this from the people that understand things better than me.<br>
<br>
<br>
Repository:<br>
  rG LLVM Github Monorepo<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D70815/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D70815/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D70815" rel="noreferrer" target="_blank">https://reviews.llvm.org/D70815</a><br>
<br>
<br>
<br>
</blockquote></div></div>