Improve compatibility with mingw-w64 in generated headers #1200
Conversation
…TY_BASES The __declspec(empty_bases) attribute is only supported by MSVC or Clang using the MSVC ABI to enable Empty Base Class Optimization (EBCO). GCC or Clang using the MinGW ABI already perform EBCO and do not support this attribute.
|
@microsoft-github-policy-service agree |
|
These changes look fairly reasonable to me, though without some basic test coverage it's not impossible for it to break in the future. Unfortunately the CI is out of our control so nothing you can do (a cppwinrt maintainer has to update them). Ideally the CI would eventually be migrated to a yml that's committed here so we can add basic coverage for clang-cl and mingw Clang. |
I'd love to add a supplemental CI build using yml and GitHub Actions so that it is completely transparent - would appreciate such a contribution. The Azure build will probably stick around as its too complex to port to yml. |
|
Thanks for the quick responses. I would love to have a CI build to check the llvm-mingw configuration, but first we need to add the missing stuff to mingw-w64 before we can try to implement the CI checks. |
Well a failing CI build is a great way to validate that the changes are even necessary. I'm not doubting they are, but it provides a clean way to validate those additions. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
strings/base_includes.h
Outdated
| #include <intrin.h> | ||
|
|
There was a problem hiding this comment.
| #include <intrin.h> | |
| #include <intrin.h> |
alvinhochun
force-pushed
the
alvin/mingw-w64
branch
from
2648b53
to
9b056a9
Compare
|
The build failed to compile all projects. Did you test this locally? |
The attribute __declspec(novtable) is only supported by MSVC or Clang using the MSVC ABI. Since this is only an optimization, we can leave it out for MinGW ABI to allow it to compile.
This uses weak alias symbols to emulate the MSVC-specific `/alternatename` linker feature.
alvinhochun
force-pushed
the
alvin/mingw-w64
branch
from
9b056a9
to
559c680
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Sorry about that, I only tested the "test" project before so I did not realize how it breaks (Full disclosure: I have to downgrade the projects to toolset v142 because I don't have VS2022 installed, and it fails to build "test_cpp20" and "test_old", so I still haven't been able to run these two tests.) |
This aims to make the generated headers usable by a Clang-based mingw-w64 toolchain. See mstorsjo/llvm-mingw#307
Note that, even with these changes, the headers does not work with llvm-mingw 15 out of the box, because there are some functions and definitions missing in mingw-w64's version of
intrin.h. We will consider fixing this upstream.