libclang: Make shared object symbol exporting by default

[cristian.adam]

Summary

https://reviews.llvm.org/D74564 enabled static building for libclang, and for non CMake consumers they had to set the CMAKE_EXPORTS define when consuming libclang.

This commit makes the non CMake users of the static building have to define CMAKE_NO_EXPORTS.

[thakis]

Thanks! Does this make it so that libclang.dll is built again with -DLLVM_ENABLE_PIC=NO on Window? From what I understand, it doesn't. Is that correct? If so, could you reinstantiate that too?

clang/include/clang-c/Platform.h

33

Why is this change necessary?

[cristian.adam]

clang/include/clang-c/Platform.h

33

I thought it would be nice to match the line 44, which deals with __attribute__((deprecated))

[cristian.adam]

In D74907#1884795, @thakis wrote:

Thanks! Does this make it so that libclang.dll is built again with -DLLVM_ENABLE_PIC=NO on Window? From what I understand, it doesn't. Is that correct? If so, could you reinstantiate that too?

It should work with -DLLVM_ENABLE_PIC=NO because now only the static targets get the CINDEX_NO_EXPORTS define.

https://reviews.llvm.org/D74564 only adds CINDEX_EXPORTS to libclang and not obj.libclang.

[cristian.adam]

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

[thakis]

In D74907#1884842, @cristian.adam wrote:

In D74907#1884795, @thakis wrote:

Thanks! Does this make it so that libclang.dll is built again with -DLLVM_ENABLE_PIC=NO on Window? From what I understand, it doesn't. Is that correct? If so, could you reinstantiate that too?

It should work with -DLLVM_ENABLE_PIC=NO because now only the static targets get the CINDEX_NO_EXPORTS define.

https://reviews.llvm.org/D74564 only adds CINDEX_EXPORTS to libclang and not obj.libclang.

The problem is (I believe) that add_clang_library(libclang is passed ${ENABLE_SHARED} and that used to be true on Windows but no longer is after your change (due to the change in line 80 in your original change), so libclang.dll isn't built at all any longer. If it was still built, the _CINDEX_LIB_ logic around line 128 wouldn't be passed for the same reason.

[cristian.adam]

-DLLVM_ENABLE_PIC=OFF on Windows if LIBCLANG_BUILD_STATIC is not set to ON will generate a SHARED libclang.dll

[cristian.adam]

This comment was removed by cristian.adam.

[cristian.adam]

The problem is (I believe) that add_clang_library(libclang is passed ${ENABLE_SHARED} and that used to be true on Windows but no longer is after your change (due to the change in line 80 in your original change), so libclang.dll isn't built at all any longer. If it was still built, the _CINDEX_LIB_ logic around line 128 wouldn't be passed for the same reason.

Right, I've uploaded a new patch which should take care of it.

[thakis]

Thanks! Let me try landing this and see what happens :)

[cristian.adam]

Thank you!

A build with -DLIBCLANG_BUILD_STATIC=ON and -DLLVM_ENABLE_PIC=OFF is green at:
https://github.com/cristianadam/llvm-project/actions/runs/42545217

The default build with -DLIBCLANG_BUILD_STATIC=OFF and -DLLVM_ENABLE_PIC=ON is green at:
https://github.com/cristianadam/llvm-project/actions/runs/42516378

[thakis]

Looks like things are still broken in several ways even with this in:

[2600/2950] Linking CXX executable bin\c-index-test.exe
 FAILED: bin/c-index-test.exe 
 cmd.exe /C "cd . && C:\b\s\w\ir\cache\builder\src\third_party\llvm-build-tools\cmake-3.12.1-win32-x86\bin\cmake.exe -E vs_link_exe --intdir=tools\clang\tools\c-index-test\CMakeFiles\c-index-test.dir --manifests  -- C:\b\s\w\ir\cache\builder\src\third_party\depot_tools\win_toolchain\vs_files\9ff60e43ba91947baca460d0ca3b1b980c3a2c23\VC\Tools\MSVC\14.23.28105\bin\Hostx64\x64\link.exe /nologo tools\clang\tools\c-index-test\CMakeFiles\c-index-test.dir\c-index-test.c.obj tools\clang\tools\c-index-test\CMakeFiles\c-index-test.dir\core_main.cpp.obj tools\clang\tools\c-index-test\CMakeFiles\c-index-test.dir\C_\b\s\w\ir\cache\builder\src\third_party\llvm\llvm\resources\windows_version_resource.rc.res  /out:bin\c-index-test.exe /implib:lib\c-index-test.lib /pdb:bin\c-index-test.pdb /version:0.0  /machine:x64 /STACK:10000000 /INCREMENTAL:NO /subsystem:console  lib\LLVMSupport.lib lib\libclang.lib lib\clangAST.lib lib\clangBasic.lib lib\clangCodeGen.lib lib\clangFrontend.lib lib\clangIndex.lib lib\clangSerialization.lib lib\LLVMCoverage.lib lib\LLVMLTO.lib lib\LLVMObjCARCOpts.lib lib\LLVMPasses.lib lib\LLVMCoroutines.lib lib\LLVMipo.lib lib\LLVMIRReader.lib lib\LLVMAsmParser.lib lib\LLVMLinker.lib lib\LLVMInstrumentation.lib lib\LLVMVectorize.lib lib\LLVMCodeGen.lib lib\LLVMBitWriter.lib lib\LLVMScalarOpts.lib lib\LLVMAggressiveInstCombine.lib lib\LLVMInstCombine.lib lib\LLVMTarget.lib lib\clangFrontend.lib lib\clangDriver.lib version.lib lib\clangParse.lib lib\LLVMOption.lib lib\clangSerialization.lib lib\clangSema.lib lib\clangAnalysis.lib lib\clangASTMatchers.lib lib\clangEdit.lib lib\clangFormat.lib lib\clangToolingInclusions.lib lib\clangToolingCore.lib lib\clangAST.lib lib\LLVMFrontendOpenMP.lib lib\LLVMTransformUtils.lib lib\LLVMAnalysis.lib lib\LLVMObject.lib lib\LLVMBitReader.lib lib\LLVMMCParser.lib lib\LLVMTextAPI.lib lib\LLVMProfileData.lib lib\clangRewrite.lib lib\clangLex.lib lib\clangBasic.lib lib\LLVMCore.lib lib\LLVMRemarks.lib lib\LLVMBitstreamReader.lib lib\LLVMMC.lib lib\LLVMBinaryFormat.lib lib\LLVMDebugInfoCodeView.lib lib\LLVMDebugInfoMSF.lib lib\LLVMSupport.lib C:\b\s\w\ir\cache\builder\src\third_party\llvm-build-tools\zlib-1.2.11\zlib.lib psapi.lib shell32.lib ole32.lib uuid.lib advapi32.lib delayimp.lib -delayload:shell32.dll -delayload:ole32.dll lib\LLVMDemangle.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cd ."
 LINK: command "C:\b\s\w\ir\cache\builder\src\third_party\depot_tools\win_toolchain\vs_files\9ff60e43ba91947baca460d0ca3b1b980c3a2c23\VC\Tools\MSVC\14.23.28105\bin\Hostx64\x64\link.exe /nologo tools\clang\tools\c-index-test\CMakeFiles\c-index-test.dir\c-index-test.c.obj tools\clang\tools\c-index-test\CMakeFiles\c-index-test.dir\core_main.cpp.obj tools\clang\tools\c-index-test\CMakeFiles\c-index-test.dir\C_\b\s\w\ir\cache\builder\src\third_party\llvm\llvm\resources\windows_version_resource.rc.res /out:bin\c-index-test.exe /implib:lib\c-index-test.lib /pdb:bin\c-index-test.pdb /version:0.0 /machine:x64 /STACK:10000000 /INCREMENTAL:NO /subsystem:console lib\LLVMSupport.lib lib\libclang.lib lib\clangAST.lib lib\clangBasic.lib lib\clangCodeGen.lib lib\clangFrontend.lib lib\clangIndex.lib lib\clangSerialization.lib lib\LLVMCoverage.lib lib\LLVMLTO.lib lib\LLVMObjCARCOpts.lib lib\LLVMPasses.lib lib\LLVMCoroutines.lib lib\LLVMipo.lib lib\LLVMIRReader.lib lib\LLVMAsmParser.lib lib\LLVMLinker.lib lib\LLVMInstrumentation.lib lib\LLVMVectorize.lib lib\LLVMCodeGen.lib lib\LLVMBitWriter.lib lib\LLVMScalarOpts.lib lib\LLVMAggressiveInstCombine.lib lib\LLVMInstCombine.lib lib\LLVMTarget.lib lib\clangFrontend.lib lib\clangDriver.lib version.lib lib\clangParse.lib lib\LLVMOption.lib lib\clangSerialization.lib lib\clangSema.lib lib\clangAnalysis.lib lib\clangASTMatchers.lib lib\clangEdit.lib lib\clangFormat.lib lib\clangToolingInclusions.lib lib\clangToolingCore.lib lib\clangAST.lib lib\LLVMFrontendOpenMP.lib lib\LLVMTransformUtils.lib lib\LLVMAnalysis.lib lib\LLVMObject.lib lib\LLVMBitReader.lib lib\LLVMMCParser.lib lib\LLVMTextAPI.lib lib\LLVMProfileData.lib lib\clangRewrite.lib lib\clangLex.lib lib\clangBasic.lib lib\LLVMCore.lib lib\LLVMRemarks.lib lib\LLVMBitstreamReader.lib lib\LLVMMC.lib lib\LLVMBinaryFormat.lib lib\LLVMDebugInfoCodeView.lib lib\LLVMDebugInfoMSF.lib lib\LLVMSupport.lib C:\b\s\w\ir\cache\builder\src\third_party\llvm-build-tools\zlib-1.2.11\zlib.lib psapi.lib shell32.lib ole32.lib uuid.lib advapi32.lib delayimp.lib -delayload:shell32.dll -delayload:ole32.dll lib\LLVMDemangle.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST /MANIFESTFILE:bin\c-index-test.exe.manifest" failed (exit code 1120) with the following output:
 c-index-test.c.obj : error LNK2019: unresolved external symbol clang_getCString referenced in function index_diagnostic
(and many other symbols)

and also

ninja: warning: multiple rules generate lib/libclang.lib. builds involving this target will not be correct; continuing anyway [-w dupbuild=warn]

(which is a hard error with newer ninjas).

I'll revert for now to unbreak things. Happy to test new attempts before they go in :)

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
 
 /* Windows DLL import/export. */
+#ifndef CINDEX_NO_EXPORTS
+  #define CINDEX_EXPORTS
+#endif
 #ifdef _WIN32
   #ifdef CINDEX_EXPORTS
     #ifdef _CINDEX_LIB_
       #define CINDEX_LINKAGE __declspec(dllexport)
     #else
       #define CINDEX_LINKAGE __declspec(dllimport)
     #endif
   #endif
-#elif defined(CINDEX_EXPORTS)
+#elif defined(CINDEX_EXPORTS) && defined(__GNUC__)
   #define CINDEX_LINKAGE __attribute__((visibility("default")))
 #endif
 
 #ifndef CINDEX_LINKAGE
   #define CINDEX_LINKAGE
 #endif
 
 #ifdef __GNUC__

clang/tools/libclang/CMakeLists.txt

add_clang_library(libclang ${ENABLE_SHARED} ${ENABLE_STATIC} INSTALL_WITH_TOOLCHAIN
   ${LIBS}
 
   LINK_COMPONENTS
   ${LLVM_TARGETS_TO_BUILD}
   Core
   Support
   )
 
+if(ENABLE_STATIC)
+  foreach(name libclang obj.libclang libclang_static)
+    if (TARGET ${name})
+      target_compile_definitions(${name} PUBLIC CINDEX_NO_EXPORTS)
+    endif()
+  endforeach()
+endif()
+
 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}")