Skip to content

Conversation

ilovepi
Copy link
Contributor

@ilovepi ilovepi commented Jul 16, 2025

@ilovepi ilovepi requested a review from a team as a code owner July 16, 2025 15:45
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jul 16, 2025
Copy link
Contributor Author

ilovepi commented Jul 16, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@llvmbot
Copy link
Member

llvmbot commented Jul 16, 2025

@llvm/pr-subscribers-libcxx

Author: Paul Kirth (ilovepi)

Changes

These tests still fail on Windows with clang-22, as reported in
#70225 (comment).
The root cause seems to be the version bump in
https://llvm.googlesource.com/llvm-project/+/01f36b39bd2475a271bbeb95fb9db8ed65e2d065

Bot: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-windows-x64/b8709267330516788049/overview
Logs: https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8709267330516788049/+/u/clang/test/stdout


Full diff: https://github.com/llvm/llvm-project/pull/149124.diff

3 Files Affected:

  • (modified) libcxx/test/libcxx/fuzzing/random.pass.cpp (+1-1)
  • (modified) libcxx/test/std/depr/depr.c.headers/math_h.pass.cpp (+1-1)
  • (modified) libcxx/test/std/numerics/c.math/cmath.pass.cpp (+1-1)
diff --git a/libcxx/test/libcxx/fuzzing/random.pass.cpp b/libcxx/test/libcxx/fuzzing/random.pass.cpp
index cb074bd60fdc8..346ff47d9bb36 100644
--- a/libcxx/test/libcxx/fuzzing/random.pass.cpp
+++ b/libcxx/test/libcxx/fuzzing/random.pass.cpp
@@ -8,7 +8,7 @@
 
 // This test fails because Clang no longer enables -fdelayed-template-parsing
 // by default on Windows with C++20 (#69431).
-// XFAIL: msvc && (clang-18 || clang-19 || clang-20 || clang-21)
+// XFAIL: msvc && (clang-18 || clang-19 || clang-20 || clang-21 || clang-22)
 
 // UNSUPPORTED: c++03, c++11
 
diff --git a/libcxx/test/std/depr/depr.c.headers/math_h.pass.cpp b/libcxx/test/std/depr/depr.c.headers/math_h.pass.cpp
index 1ba0063c1dada..a6fdea524fb83 100644
--- a/libcxx/test/std/depr/depr.c.headers/math_h.pass.cpp
+++ b/libcxx/test/std/depr/depr.c.headers/math_h.pass.cpp
@@ -8,7 +8,7 @@
 
 // This test fails because Clang no longer enables -fdelayed-template-parsing
 // by default on Windows with C++20 (#69431).
-// XFAIL: msvc && (clang-18 || clang-19 || clang-20 || clang-21)
+// XFAIL: msvc && (clang-18 || clang-19 || clang-20 || clang-21 || clang-22)
 
 // <math.h>
 
diff --git a/libcxx/test/std/numerics/c.math/cmath.pass.cpp b/libcxx/test/std/numerics/c.math/cmath.pass.cpp
index 48c2918802fc3..4d1e5c329b388 100644
--- a/libcxx/test/std/numerics/c.math/cmath.pass.cpp
+++ b/libcxx/test/std/numerics/c.math/cmath.pass.cpp
@@ -8,7 +8,7 @@
 
 // This test fails because Clang no longer enables -fdelayed-template-parsing
 // by default on Windows with C++20 (#69431).
-// XFAIL: msvc && (clang-18 || clang-19 || clang-20 || clang-21)
+// XFAIL: msvc && (clang-18 || clang-19 || clang-20 || clang-21 || clang-22)
 
 // <cmath>
 

Comment on lines 9 to 11
// This test fails because Clang no longer enables -fdelayed-template-parsing
// by default on Windows with C++20 (#69431).
// XFAIL: msvc && (clang-18 || clang-19 || clang-20 || clang-21)
// XFAIL: msvc && (clang-18 || clang-19 || clang-20 || clang-21 || clang-22)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest switching to this:

// This test doesn't work on Windows because the Windows headers <EXPLAIN REASON>
// XFAIL: msvc

IIUC, the test basically doesn't work on Windows with any of our supported compilers, so it's not really useful to keep carrying the clang-XY annotations here. By the way, why does the test fail? I mean, what's special about Windows that makes this test require -fdelayed-template-parsing?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is explained in issue 70225; please add a link to that issue in the comment here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops only fixed one file. will address.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually done this time :)

@ilovepi ilovepi force-pushed the users/ilovepi/clang-version-fix-libcxx branch 2 times, most recently from 41caae6 to bd6dd16 Compare July 16, 2025 18:53
@frederick-vs-ja
Copy link
Contributor

FYI, I'm working on a "proper" workaround since C++20, see #149234.

@boomanaiden154
Copy link
Contributor

(Sorry, accidentally hit the update branch button on my phone).

@ilovepi
Copy link
Contributor Author

ilovepi commented Jul 17, 2025

FYI, I'm working on a "proper" workaround since C++20, see #149234.

That's fine, but I'd like to unblock our CI ASAP. We haven't been able to qualify a toolchain since the version bump, and we're loosing a lot of important signal. If this can land sooner I'd prefer to do that, and then there isn't any urgency to land a proper fix quickly.

@ilovepi ilovepi force-pushed the users/ilovepi/clang-version-fix-libcxx branch from 86062e2 to 8027da6 Compare July 17, 2025 17:28
@ilovepi
Copy link
Contributor Author

ilovepi commented Jul 17, 2025

All tests were passing, but I reworded the comments.

@ilovepi
Copy link
Contributor Author

ilovepi commented Jul 17, 2025

@ldionne anything left to address?

@ldionne ldionne merged commit fd12e9a into main Jul 18, 2025
77 checks passed
@ldionne ldionne deleted the users/ilovepi/clang-version-fix-libcxx branch July 18, 2025 15:03
TheBlindArchitect pushed a commit to XSLabs/integration that referenced this pull request Jul 30, 2025
…ter version bump in clang"

This reverts commit f87e801dc18890b3b1aed9f83a60049901900eec.

Reason for revert: The tests were properly marked XFAIL in
llvm/llvm-project#149124, so the
workaround is no longer required.

Original-Fixed: 432526890, 432077523
Original change's description:
> [clang] Suppress tests in libcxx after version bump in clang
>
> Skip these tests until upstream fixes land to unblock our CI.
>
> Original-Bug: 432526890, 432077523
> Change-Id: I7ae0887c88aa6bcda3d6bbea7ad123a167c69618
> Original-Reviewed-on: https://fuchsia-review.googlesource.com/c/infra/recipes/+/1324807
> Commit-Queue: Paul Kirth <[email protected]>
> Reviewed-by: Haowei Wu <[email protected]>
> Reviewed-by: Caslyn Tonelli <[email protected]>
> Fuchsia-Auto-Submit: Paul Kirth <[email protected]>

Original-Bug: 432526890, 432077523
Original-Reviewed-on: https://fuchsia-review.googlesource.com/c/infra/recipes/+/1325692
Original-Revision: 672571c8e6977612bd3795f776d472ce8435cdc7
GitOrigin-RevId: e9091f55961cb4a2e0c4798c559962b3b6df8559
Change-Id: Ia5c607fb23d4aa55d730db4df68aa07b7e5b0765
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants