No symbols exported with clang-cl

[Neumann-A]

trafficstars

Due to: https://github.com/zeux/pugixml/blob/3b5c1fb022dc286b0480ccafa9b79f832deb24ce/CMakeLists.txt#L114 No symbols get exported if pugixml gets build with clang-cl instead of cl.

[zeux]

Can you elaborate on this? As in, is this a bug in clang-cl? Or is the compiler id selection not picking up MSVC-specific options for clang-cl? Do you have a suggested fix?

[Neumann-A]

The genex doesn't cover clang-cl and is only true for cl since the compiler id is Clang for clang-cl. If you would wrap the code in a simple if(WIN32) and remove the genex it would work for both compilers correctly.

[Neumann-A]

If you need a patch https://github.com/microsoft/vcpkg/blob/95c001262da51f15ce8e5668b35536184fa5105a/ports/pugixml/dllexport.patch

[zeux]

I'm wondering if there's a way to specify the expression in such a way that it works for clang-cl as well; I don't recall the exact reasons but I know when CMake files were rewritten for pugixml most conditional configuration was replaced with expressions. cc @bruxisma for insight

[bruxisma]

It is more than doable. It'll require an $<AND> and I can link to some docs. I'll do it in an hour or two as I’m currently on mobile. 😅

[bruxisma]

I was wrong, apologies. It requires an $<OR>

$<$<OR:$<CXX_COMPILER_ID:MSVC>,$<STREQUAL:${CMAKE_CXX_COMPILER_FRONTEND_VARIANT},MSVC>>:PUGIXML_API=__declspec\(dllexport\)>

Should fix it, however I'm aware this isn't as readable. Defining it via string(CONCAT) should help in that regard:

string(CONCAT pugixml.msvc $<OR:
  $<STREQUAL:${CMAKE_CXX_COMPILER_FRONTEND_VARIANT},MSVC>,
  $<CXX_COMPILER_ID:MSVC>
>)

$<${pugixml.msvc}:PUGIXML_API=__declspec\(dllexport\)>

This variable definition should be reusable elsewhere to get MSVC and clang-cl to match up.

Edit: I forgot to write the correct variable initially. 🤦 I've made the sample correct.

[Neumann-A]

This is a good example why you dont use genex everywhere. What about mingw? Does it require the export statement?

[bruxisma]

Generator expressions execute in parallel at generation time, instead of during configure time, which is single threaded. In a small project isolated it is not a performance issue. In million-line code bases it causes massive speed-ups for CMake configure time. Using genexp as much as possible is necessary to reduce reconfigure times, especially since CMake does not reuse the internal condition evaluator (cmConditionEvaluator) when operating on if() commands and instead creates it everytime a new if() is entered. This is a bison/yacc generated based parser that is constructed, and destructed everytime a condition is evaluated, even with nested branches. if() in CMake is extremely inefficient.

To also be clear, I only initially did a CMake refactor, not a rewrite (else I would have bumped the minimum version to 3.14). Otherwise, I would have just used GenerateExportHeader from CMake.

What about mingw? Does it require the export statement?

__declspec(dllexport) is an MSVC specific attribute.

MinGW supports it, but you can get by using the GCC visibility attribute instead. Additionally, having looked at the github workflow for the repository, it's not a tested platform.

[Neumann-A]

I won't fight that argument for a PRIVATE genex but for an INTERFACE/PUBLIC genex the story is a bit different if the genex leaks into the exported targets and requires re-evaluation in every downstream project.

[bruxisma]

That's the thing. It doesn't get re-evaluated. Its evaluated once at generation time, and then simply read from. It's just that the processing and evaluation is pushed into a multithreaded pool. Generation time is cheap, configure time is not.

[Neumann-A]

That's the thing. It doesn't get re-evaluated. Its evaluated once at generation time, and then simply read from. It's just that the processing and evaluation is pushed into a multithreaded pool. Generation time is cheap, configure time is not.

No you didn't understand. If the genex leaks into the exported targets every downstream project has to re-evaluate the genex. I am not speaking about a single project which uses add_subdirectory but multiple separate projects using find_package

[bruxisma]

I did understand. I'm saying that it's a negligible genex re-evaluation, even in large projects, even across multiple process. And if you don't want these to be re-evaluated, use $<LINK_ONLY>.

[Neumann-A]

negligible genex re-evaluation

but it is not zero. even if you have a factor 100x faster reconfigure time in A downstream will pay for it.

If A is a dependency in N other projects B than that time won in A is already meaningless if N is big enough. Furthermore not only direct dependencies of A have to re-evaluate but also all transient deps C which depend on B need to re-evaluate. And I am speaking from a vcpkg perspective where pugixml only gets configured once (twice for both configs) and an unknown number of other projects depend on it.

I prefer having stuff solved at configure if possible.

[bruxisma]

I do not see why the cost of configuring the project should be put on the project itself, when the actual speed at generation time is again so small that even in a million-line code base it won't be noticed. This is because when you import a target with generator expressions they are evaluated on the target before transitive linking occurs, and all targets are evaluated in parallel until a genexp dependency has been set. Even in a large project like LLVM, moving from configure based operations to generator expressions has no more cost than the cost of dependency tracking, and downstream projects will not see a performance hit.

Every downstream target would have to run the generator expression evaluator regardless of whether pugixml has genexp or not. It is still much faster than having to punish every single build system out there that uses FetchContent and the upcoming FetchContent and find_package integration. You are effectively stating that folks who use builtin CMake features should be punished at configure time (an already slow and exhausting experience in large projects) so that vcpkg, can have an advantage in some cases, while punishing other CMake packaging tools that run natively within CMake.

Lastly, I don't know about you, but I prefer my generative build system to be debuggable so that someone can at least see what they're getting. Compare the genexp pugixml-targets.cmake with the non-approach:

Genexp

set_target_properties(pugixml::static PROPERTIES
  INTERFACE_COMPILE_DEFINITIONS "\$<\$<BOOL:OFF>:PUGIXML_WCHAR_MODE>;\$<\$<BOOL:OFF>:PUGIXML_COMPACT>;\$<\$<BOOL:OFF>:PUGIXML_NO_XPATH>;\$<\$<BOOL:OFF>:PUGIXML_NO_STL>;\$<\$<BOOL:OFF>:PUGIXML_NO_EXCEPTIONS>")

---

Configure Time

At least in this case, we can see what variables evaluated to, instead of them being completely missing,

[zeux]

FWIW I'm impartial to the specific solution here, if a PR gets contributed to fix this I can merge it.