[PATCH] D61356: [compiler-rt] Rework the object build support

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added a comment.

This is really nasty, but I'm not sure if there's a better way to do this.



================
Comment at: compiler-rt/cmake/Modules/AddCompilerRT.cmake:278
+      # TODO: CMake 3.4 doesn't allow installing library objects so we have to build them manually.
+      # When LLVM migrates to LLVM 3.9, we can use add_library just like for other library types.
+      if(CMAKE_C_COMPILER_ID MATCHES Clang AND CMAKE_C_COMPILER_TARGET)
----------------
I think you mean "CMake 3.9" and not "LLVM 3.9" :)


================
Comment at: compiler-rt/cmake/Modules/AddCompilerRT.cmake:278
-      string(TOUPPER ${CMAKE_BUILD_TYPE} config)
-      get_property(cflags SOURCE ${sources_${libname}} PROPERTY COMPILE_FLAGS)
-      separate_arguments(cflags)
----------------
smeenai wrote:
> I think you mean "CMake 3.9" and not "LLVM 3.9" :)
Is it okay to drop this part? I don't know how common it is to set `COMPILE_FLAGS` on sources.


================
Comment at: compiler-rt/cmake/Modules/AddCompilerRT.cmake:304
+      endforeach()
+      string(REPLACE " " ";" compile_command_${libname} "${compile_command_${libname}}")
       add_custom_command(
----------------
Would `separate_arguments` be better here, to handle quoted arguments etc. correctly? (Though I guess the `string(REPLACE)` on line 282 is potentially screwing up quoted arguments anyway.)


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D61356