libclang: Add static build support for Windows

[cristian.adam]

Summary

On Linux and macOS it was possible to build libclang statically by configuring CMake with:

-D LIBCLANG_BUILD_STATIC=ON
-D LLVM_ENABLE_PIC=OFF

On Windows was not possible. This commit fixes this.

[cristian.adam]

Build with this patch being built on GitHub Actions on Windows MSVC, Windows MinGW, Linux, macOS:
https://github.com/cristianadam/llvm-project/actions/runs/38838989

[yvvan]

Please, upload patches with context (-U9999).

clang/include/clang-c/Platform.h

31

Is it different from just leaving CINDEX_LINKAGE empty?

[cristian.adam]

clang/include/clang-c/Platform.h

31

On Windows the default symbol visibility is hidden and with CINDEX_LINKAGE we make the symbols visible.

On Linux it's the other way around, everything is visible, but
with CMAKE_CXX_VISIBILITY_PRESET=hidden we will have the Windows behavior and then we need to make CINDEX_LINKAGE point to something that will make the symbols visible.

[yvvan]

LGTM

clang/include/clang-c/Platform.h

31

but this one happens only in non-Windows case (#elif) so it should not change anything I guess

[cristian.adam]

I do not have commit rights, I need help to submit this patch.

[cristian.adam]

@mstorsjo can you help me with pushing this commit? Thank you in advance!

[thakis]

clang/tools/libclang/CMakeLists.txt

117

Is this enough? Every target that depends on libclang now needs to define CINDEX_EXPORTS – is that already the case?

[cristian.adam]

clang/tools/libclang/CMakeLists.txt

117

The targets that depend on libclang should get the CINDEX_EXPORTS from libclang due to the PUBLIC usage.

[thakis]

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.

clang/tools/libclang/CMakeLists.txt

117

Cool, thanks.

[cristian.adam]

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.

I'll try the other way around in a subsequent review. I agree that the existing code shouldn't be changed, and only the users that want a static libclang should have to do changes.

[thakis]

Great, thanks. Since we're currently broken due to this change, any ETA for the follow-up?

[cristian.adam]

In D74564#1884675, @thakis wrote:

Great, thanks. Since we're currently broken due to this change, any ETA for the follow-up?

I'll submit it today.

[cristian.adam]

I'll submit it today.

Done, see: https://reviews.llvm.org/D74907

clang/include/clang-c/Platform.h

 
 #ifndef LLVM_CLANG_C_PLATFORM_H
 #define LLVM_CLANG_C_PLATFORM_H
 
 #include "clang-c/ExternC.h"
 
 LLVM_CLANG_C_EXTERN_C_BEGIN
 
-/* MSVC DLL import/export. */
-#ifdef _MSC_VER
+/* Windows DLL import/export. */
+#ifdef _WIN32
+  #ifdef CINDEX_EXPORTS
     #ifdef _CINDEX_LIB_
       #define CINDEX_LINKAGE __declspec(dllexport)
     #else
       #define CINDEX_LINKAGE __declspec(dllimport)
     #endif
-#else
+  #endif
+#elif defined(CINDEX_EXPORTS)
+  #define CINDEX_LINKAGE __attribute__((visibility("default")))
+#endif
+
+#ifndef CINDEX_LINKAGE
   #define CINDEX_LINKAGE
 #endif
 
 #ifdef __GNUC__
   #define CINDEX_DEPRECATED __attribute__((deprecated))
 #else
   #ifdef _MSC_VER
     #define CINDEX_DEPRECATED __declspec(deprecated)
   #else
     #define CINDEX_DEPRECATED
   #endif
 #endif
 
 LLVM_CLANG_C_EXTERN_C_END
 
 #endif

clang/tools/libclang/CMakeLists.txt

 
 if(MSVC)
   # Avoid LNK4197 by not specifying libclang.exports here.
   # Each functions is exported as "dllexport" in include/clang-c.
   # KB835326
   set(LLVM_EXPORTED_SYMBOL_FILE)
 endif()
 
-if(LLVM_ENABLE_PIC OR WIN32)
+if(LLVM_ENABLE_PIC)
   set(ENABLE_SHARED SHARED)
 endif()
 
-if((NOT LLVM_ENABLE_PIC OR LIBCLANG_BUILD_STATIC) AND NOT WIN32)
+if(NOT LLVM_ENABLE_PIC OR LIBCLANG_BUILD_STATIC)
   set(ENABLE_STATIC STATIC)
 endif()
 
 if(WIN32)
   set(output_name "libclang")
 else()
   set(output_name "clang")
 endif()
add_clang_library(libclang ${ENABLE_SHARED} ${ENABLE_STATIC} INSTALL_WITH_TOOLCHAIN
 
   LINK_COMPONENTS
   ${LLVM_TARGETS_TO_BUILD}
   Core
   Support
   )
 
 if(ENABLE_SHARED)
+  target_compile_definitions(libclang PUBLIC CINDEX_EXPORTS)
   if(WIN32)
     set_target_properties(libclang
       PROPERTIES
       VERSION ${LIBCLANG_LIBRARY_VERSION}
       DEFINE_SYMBOL _CINDEX_LIB_)
   elseif(APPLE)
     set(LIBCLANG_LINK_FLAGS " -Wl,-compatibility_version -Wl,1")
     set(LIBCLANG_LINK_FLAGS "${LIBCLANG_LINK_FLAGS} -Wl,-current_version -Wl,${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}")