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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to