[Cygwin][MinGW] Internal class in explicitly-instantiation-declarated template should be instantiated #140145
Conversation
kikairoya
force-pushed
the
mingw-template-instantiation-abi
branch
3 times, most recently
from
7b6c355 to
98c7630
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
kikairoya
force-pushed
the
mingw-template-instantiation-abi
branch
3 times, most recently
from
2307fd1 to
af82a52
Compare
|
maybe try fixing the formatting complaint and see if the libc++ tests will go further? (have you tested that with the patched clang? this CI uses a pre-existing clang binary, not the one built in another workflow) |
|
I've hacked up llvm-mingw workflow to build and test toolchain and libc++ for mingw at https://github.com/jeremyd2019/llvm-mingw/actions/runs/15075388808, hopefully I got the workflow right and this will be the correct test of af82a52 |
kikairoya
force-pushed
the
mingw-template-instantiation-abi
branch
from
af82a52 to
db2e8d7
Compare
|
They've failed linking things due to missing # if defined(__MINGW32__) || defined(__CYGWIN__)
extern template class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS basic_istream<char>::sentry;
# endifrather than explicitly listing the member(s). |
|
trying to use this libc++ ci is less than helpful: the jobs keep failing for no apparent reason (cancelled? docker not running?), and whatever seems to be supposed to rerun the failed jobs is not all that reliable about doing so. Hopefully my llvm-mingw fork will show this technique works, and we can be reassured that being in an ifdef that isn't true for any other platform shouldn't break any other platform either. |
Nope, "an attribute list cannot appear here" |
kikairoya
force-pushed
the
mingw-template-instantiation-abi
branch
from
db2e8d7 to
92a3b90
Compare
|
I'm running just a libc++ test of 92a3b90 (with the toolchain built from the previous run, so it shouldn't take 7 hours this time) in https://github.com/jeremyd2019/llvm-mingw/actions/runs/15087868464 🟢 |
… template should be instantiated In-code comment says "explicit instantiation decl of the outer class doesn't affect the inner class" but this behavior seems MSVC specific, MinGW-GCC and Cygwin-GCC does not. Clang should honor gcc's behavior. This change fixes std::string compatibilty and resolves strange link error (statically linked), strange crash (dynamically linked) using libstdc++ on Cygwin.
|
This might help for Windows testing diff --git a/.github/workflows/libcxx-build-and-test.yaml b/.github/workflows/libcxx-build-and-test.yaml
index 3551fc150e..27e5bd6450 100644
--- a/.github/workflows/libcxx-build-and-test.yaml
+++ b/.github/workflows/libcxx-build-and-test.yaml
@@ -239,7 +239,7 @@ jobs:
windows:
runs-on: windows-2022
- needs: [ stage2 ]
+ #needs: [ stage2 ]
strategy:
fail-fast: false
matrix:
@@ -263,10 +263,24 @@ jobs:
if: ${{ matrix.mingw != true }}
run: |
choco install -y llvm --version=19.1.7 --allow-downgrade
+ - name: Download test llvm-mingw
+ if: ${{ matrix.mingw == true }}
+ shell: bash
+ run: |
+ ARTIFACT_URL=https://github.com/jeremyd2019/llvm-mingw/actions/runs/15080594305/artifacts/3143907630
+ case "$ARTIFACT_URL" in
+ https://github.com/*/actions/runs/[0-9]*/artifacts/[0-9]*)
+ ARTIFACT_URL="$(echo "$ARTIFACT_URL" |
+ sed 's|^\(https://\)\(github.com/\)\(.*/actions/\)runs/[0-9]*/\(artifacts/[0-9]*\)$|\1api.\2repos/\3\4/zip|')"
+ ;;
+ esac
+ curl -H "Authorization: token ${{secrets.GITHUB_TOKEN}}" \
+ -fLo artifact.zip "$ARTIFACT_URL"
+ powershell Expand-Archive artifact.zip -DestinationPath .
+ rm -f artifact.zip
- name: Install llvm-mingw
if: ${{ matrix.mingw == true }}
run: |
- curl -LO https://github.com/mstorsjo/llvm-mingw/releases/download/20250114/llvm-mingw-20250114-ucrt-x86_64.zip
powershell Expand-Archive llvm-mingw*.zip -DestinationPath .
del llvm-mingw*.zip
mv llvm-mingw* c:\llvm-mingw |
…bility with Mingw-GCC This is affected to only Clang for MinGW target. In MinGW environment, Clang handles dllexport attribute of internal class that defined in class template in different way from GCC. This incompatibility should be fixed but breaks ABI of libc++, so introduce a new keyword to keep ABI in MinGW environment with old and patched Clang and to ensure nothing affects to other environment/platforms. The new attribute keyword _LIBCPP_INNER_CLASS_IN_TEMPLATE_VIS does nothing in almost all situations except if included from client (not in building libc++ itself) by clang (not by GCC or others). If clang does include libc++, the keyword will be expanded to __attribute__((__exclude_from_explicit_instantiation__)), results attached class will be away from explicit instantiation declaration so will be instanciated implicitly as formar Clang does. Thus, it will no-op for old Clang that has incompatibility with MinGW-GCC or emulate old behavior for patched Clang. This attribute is attached only for std::basic_ostream::sentry and std::basic_istream::sentry. Other entities won't be affected by patching Clang so doesn't need to be annotate. Notably, at a time to introduce a new class as a non-template inner type of a class template, that class also needs to be attached _LIBCPP_INNER_CLASS_ININ_TEMPLATE_VIS.
kikairoya
force-pushed
the
mingw-template-instantiation-abi
branch
from
92a3b90 to
9349b45
Compare
resolves #135910