In D119199#3337699, @vtjnash wrote:

Is that a bug in clang or in MSVC2019? We expected that CLANG_PLUGIN_SUPPORT=ON can expose problems with the Windows linker, and we add new tests here for verifying that configuration is functioning as expected. You might need to disable CLANG_PLUGIN_SUPPORT on that target?

release/14.x now configures but fails to build on MSVC2019 with -D LLVM_EXPORT_SYMBOLS_FOR_PLUGINS=ON like at https://reviews.llvm.org/D111100

In D119199#3309117, @vtjnash wrote:

@cristian.adam Is this good now? You are blocking merging this, but I think it is ready to land, and I would like to not hold it up for other people if it is fixing their issues (and the issues you discovered too)

This patch is having problems on Windows with MSVC 2019 and -D LLVM_EXPORT_SYMBOLS_FOR_PLUGINS=ON and -D "LLVM_ENABLE_PROJECTS=clang;clang-tools-extra".

I had on Windows two uses cases that failed due to this patch.

Qt Creator hit the same issue as others have had, 0 bytes clang-cl.exe due to packaging with 7zip.

@thakis has a use case at https://reviews.llvm.org/D74907 building a SHARED libclang with LLVM_ENABLE_PIC set to OFF.

In D75068#2004072, @davezarzycki wrote:

Rather than wholly reverting this, I committed a fix: 665471907a5c072c6653a38c35f35e5d54cef220

The problem is that "NOT LIBCLANG_BUILD_STATIC" does not imply PIC, so there is no point in trying to build the SHARED version. Please let me know if I somehow missed something. Thanks!

@mstorsjo can you merge (again) this on my behalf? Thank you in advance!

Since I do not have commit rights, I require help to get this merged.

@thakis ping?

ping

Ping

Ping.

ping?

The pre-merge check complains with clang-tidy not being able to locate windows.h and io.h. Is there anything I can do?

ping?

In D74599#1893597, @rnk wrote:

lgtm

Do you need someone to commit this? Thanks for the portability fix!

Use WindowsSupport.h in llvm-ar, which sets _WIN32_WINNT to 0x0601

I've tested this patch with:

Thank you!

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.

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

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

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?

I'll submit it today.

In D74564#1884675, @thakis wrote:

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

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.

Added _WIN32_WINNT to have the same value as WINVER

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

I have a bug report regarding this review at: https://bugs.llvm.org/show_bug.cgi?id=44907

Updated revision with a -U9999 diff.

This could also be cherry-picked for release/10.x. Would close https://bugs.llvm.org/show_bug.cgi?id=44906

In D74561#1880310, @mstorsjo wrote:

I can try to get it committed soon. What form do you want for the author line, "Cristian Adam <cristian.adam@gmail.com>"?

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

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

Updated from Windows Vista (0x600) to Windows 7 (0x601)

I'm new to Phabricator. I don't feel like installing php on my Windows machine. I wish llvm would have used github as review. But this won't happen anytime soon. Oh well.

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

BUILD_SHARED_LIBS does create lots of shared objects, and LLVM_BUILD_LLVM_DYLIB should be the right solution for Qt Creator where we have a bit of big binaries.

In D63881#1571286, @beanz wrote:

This is definitely not the right fix. We don’t want to set everything as Public dependencies. Our build system’s model for dependencies is that each component specified what it directly uses which is its Private dependencies. If you are having issues that probably means missing dependencies.

In D63881#1561072, @compnerd wrote:

I don't understand the change for the PRIVATE to PUBLIC. That doesn't make a different to the import library handling, which is baked right into CMake, so that will always be handled properly irrespective of the link dependency's visibility. I think the only thing that may be needed is the Core dependency on the TestPlugin.