[PATCH] D70972: [AIX] Make sure we use export lists for plugins

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:913
+# symbols to export from LLVM tools. This is necessary when on AIX or using MSVC
+# if you want to allow plugins, though note that the plugin has to explicitly
+# link against (exactly one) tool so we can't unilaterally turn on
----------------
Does the "fact" to note apply to AIX and MSVC equally?


================
Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:915
+# link against (exactly one) tool so we can't unilaterally turn on
+# LLVM_ENABLE_PLUGINS when it's enabled. We enable this by default on AIX.
+set(LLVM_EXPORT_SYMBOLS_FOR_PLUGINS_default OFF)
----------------
Add: "except when the LLVM libraries are set up for dynamic linking (due to incompatibility) in the initial configuration".


================
Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:917
+set(LLVM_EXPORT_SYMBOLS_FOR_PLUGINS_default OFF)
+if (${CMAKE_SYSTEM_NAME} MATCHES "AIX" AND NOT (BUILD_SHARED_LIBS OR LLVM_LINK_LLVM_DYLIB))
+  set(LLVM_EXPORT_SYMBOLS_FOR_PLUGINS_default ON)
----------------
I'm not too sure about this. Are differing defaults based on other options passed on the initial configuration a good CMake design?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70972