[libcxx-commits] [PATCH] D60370: [gn] Support for building libunwind

[Nico Weber via Phabricator via libcxx-commits]

thakis added inline comments.


================
Comment at: llvm/utils/gn/secondary/libunwind/src/BUILD.gn:51
+  cflags_c = [ "-std=c99" ]
+  cflags_cc = [ "-fno-rtti" ]
+  include_dirs = [ "//libunwind/include" ]
----------------
phosek wrote:
> thakis wrote:
> > why do you remove config_nortti and then add -fno-rtti back in here? Can't you just not remove it in the first place?
> In CMake build, it's possible to re-enable RTTI using the `LLVM_ENABLE_RTTI` that some users actually use, especially when linking LLVM into other projects. Do we plan on providing the same flag in LLVM build? If yes, that would be an argument for setting `-fno-rtti` here explicitly because it should be independent of the global LLVM setup.
Not unless someone needs it. I'd default to not adding things until they're needed.


================
Comment at: llvm/utils/gn/secondary/libunwind/src/BUILD.gn:77
+    sources = unwind_sources
+    # TODO: sync_source_lists_from_cmake.py doesn't handle these.
+    #public = unwind_headers
----------------
phosek wrote:
> thakis wrote:
> > It looks for .h files, no matter if in a target or not. Does not adding this to public help the script? That'd surprise me.
> I think the problem is that the CMake build uses `${CMAKE_CURRENT_SOURCE_DIR}/../include/libunwind.h` while I tried using `../include/libunwind.h`, the script doesn't seem to handle variables or `..`. Any suggestions how to work around it?
Is the `${CMAKE_CURRENT_SOURCE_DIR}` needed on the cmake side? Some places don't use it:

$ git grep '\.\..*\.h' 'clang*CMake*'
clang/tools/libclang/CMakeLists.txt:  ../../include/clang-c/Index.h



Repository:
  rUNW libunwind

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60370/new/

https://reviews.llvm.org/D60370