[PATCH] D74564: libclang: Add static build support for Windows

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Another question: We have a bot that builds LLVM with this cmake config:

'cmake', '-GNinja', '-DCMAKE_BUILD_TYPE=Release', '-DLLVM_ENABLE_ASSERTIONS=OFF', '-DLLVM_ENABLE_PROJECTS=clang;compiler-rt;lld;chrometools;clang-tools-extra', '-DLLVM_TARGETS_TO_BUILD=AArch64;ARM;Mips;PowerPC;SystemZ;WebAssembly;X86', '-DLLVM_ENABLE_PIC=OFF', '-DLLVM_ENABLE_UNWIND_TABLES=OFF', '-DLLVM_ENABLE_TERMINFO=OFF', '-DCLANG_PLUGIN_SUPPORT=OFF', '-DCLANG_ENABLE_STATIC_ANALYZER=OFF', '-DCLANG_ENABLE_ARCMT=OFF', '-DBUG_REPORT_URL=https://crbug.com and run tools/clang/scripts/process_crashreports.py (only works inside Google) which will upload a report', '-DCOMPILER_RT_USE_LIBCXX=NO', '-DLLVM_INCLUDE_GO_TESTS=OFF', '-DCLANG_SPAWN_CC1=ON', '-DLLVM_USE_CRT_RELEASE=MT', '-DLLVM_ENABLE_ZLIB=FORCE_ON', '-DLLVM_ENABLE_THREADS=OFF', '-DLLVM_ENABLE_PROJECTS=clang', '-DCMAKE_C_FLAGS=-IC:\\b\\s\\w\\ir\\cache\\builder\\src\\third_party\\llvm-build-tools\\zlib-1.2.11', '-DCMAKE_CXX_FLAGS=-IC:\\b\\s\\w\\ir\\cache\\builder\\src\\third_party\\llvm-build-tools\\zlib-1.2.11', '-DLLVM_BUILD_INSTRUMENTED=IR', '-DCMAKE_C_COMPILER=C:/b/s/w/ir/cache/builder/src/third_party/llvm-bootstrap-install/bin/clang-cl.exe', '-DCMAKE_CXX_COMPILER=C:/b/s/w/ir/cache/builder/src/third_party/llvm-bootstrap-install/bin/clang-cl.exe', '-DCMAKE_LINKER=C:/b/s/w/ir/cache/builder/src/third_party/llvm-bootstrap-install/bin/lld-link.exe', 'C:\\b\\s\\w\\ir\\cache\\builder\\src\\third_party\\llvm\\llvm']

It then packaged libclang.dll, which after this change is no longer produced. That's because we pass -DDLLVM_ENABLE_PIC=OFF. That used to not affect Windows, which intuitively makes sense since fPIC are more gcc style flags, and position independent code and shared libraries work fairly differently on Windows vs elsewhere.

Given that this is currently a breaking change: Does it make sense to re-use LLVM_ENABLE_PIC for this, which currently doesn't have an effect on Windows anywhere? Maybe there should be a dedicated "I want libclang to be a static library" opt-in? And maybe the Platform.h should default to the dll setup and require a define to not use it, instead of the other way round?  That seems safer for embedders.



================
Comment at: clang/tools/libclang/CMakeLists.txt:117
 if(ENABLE_SHARED)
+  target_compile_definitions(libclang PUBLIC CINDEX_EXPORTS)
   if(WIN32)
----------------
cristian.adam wrote:
> thakis wrote:
> > Is this enough? Every target that depends on libclang now needs to define CINDEX_EXPORTS – is that already the case?
> The targets that depend on libclang should get the `CINDEX_EXPORTS` from libclang due to the `PUBLIC` usage.
> 
> 
Cool, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74564