[PATCH] D41272: Don't try to run MCJIT/OrcJIT EH tests when C++ library is statically linked

Chris Bieneman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 18 16:39:38 PST 2017


beanz added a comment.

A few inline comments. Overall I like the direction.



================
Comment at: cmake/modules/CheckCXXSharedLibrary.cmake:60
+  execute_process(
+    COMMAND ${CMAKE_OBJDUMP} -p "${TARGET_NAME}/CheckCXXSharedLibrary"
+    RESULT_VARIABLE CHECK_RESULT
----------------
Based on our IRC conversation I ran off and read CMake's sources. Looks like `CMAKE_OBJDUMP` is unset on Windows, and only set elsewhere if `objdump` is found by CMake.

Might be worth adding an early out at the beginning of this function to mark the check as failed if `CMAKE_OBJDUMP` is unset. In that case you should also probably log that the check failed to the CMake status, and log why in the error log.


================
Comment at: cmake/modules/CheckCXXSharedLibrary.cmake:65
+  )
+
+  if(CHECK_OUTPUT MATCHES "lib(std)?c\\+\\+")
----------------
`CHECK_RESULT` and `CHECK_ERROR` are unused here, as well as `LINK_RESULT`, `LINK_OUTPUT` and `LINK_ERROR`.

Probably makes sense to write the value of all those variables out to log files in the event the link or `objdump` commands fail.


Repository:
  rL LLVM

https://reviews.llvm.org/D41272





More information about the llvm-commits mailing list