Skip to content

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Mar 14, 2025

Backport 672e385

Requested by: @ian-twilightcoder

@llvmbot llvmbot requested a review from a team as a code owner March 14, 2025 19:38
@llvmbot llvmbot added this to the LLVM 20.X Release milestone Mar 14, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Mar 14, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Mar 14, 2025

@philnik777 What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from philnik777 March 14, 2025 19:38
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 14, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Mar 14, 2025

@llvm/pr-subscribers-libcxx

Author: None (llvmbot)

Changes

Backport 672e385

Requested by: @ian-twilightcoder


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

1 Files Affected:

  • (modified) libcxx/include/__vector/vector_bool.h (+1)
diff --git a/libcxx/include/__vector/vector_bool.h b/libcxx/include/__vector/vector_bool.h
index 4f1c442ce0be8..feff646a35dc8 100644
--- a/libcxx/include/__vector/vector_bool.h
+++ b/libcxx/include/__vector/vector_bool.h
@@ -17,6 +17,7 @@
 #include <__bit_reference>
 #include <__config>
 #include <__functional/unary_function.h>
+#include <__fwd/bit_reference.h>
 #include <__fwd/functional.h>
 #include <__fwd/vector.h>
 #include <__iterator/distance.h>

@github-project-automation github-project-automation bot moved this from Needs Triage to Needs Merge in LLVM Release Status Mar 14, 2025
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja left a comment

Choose a reason for hiding this comment

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

LGTM. I opened #131506 for resolving CI failures with Apple Clang 17, perhaps that patch should be backported first.

@ldionne
Copy link
Member

ldionne commented Mar 17, 2025

I'd like us to validate that this is the right fix on main first. I've asked for a reproducer in #127015.

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

Now that we have a better understanding that this is a workaround for a real issue, how to reproduce the issue, etc, I am comfortable with cherry-picking this to LLVM 20. I don't think this is necessarily the workaround we'll want to keep forever, but I think this is safe and useful to cherry-pick.

See #127015 (comment) for more details.

@tstellar
Copy link
Collaborator

This still has some failing tests.

@ian-twilightcoder
Copy link
Contributor

This still has some failing tests.

I think they should pass after #131506, re-running

@ldionne
Copy link
Member

ldionne commented Mar 18, 2025

@ian-twilightcoder You'd have to rebase the PR in order for the CI issue to be fixed. I'm not concerned with the failing tests on macOS since that's indeed #131506, IMO this can be merged safely.

@ian-twilightcoder
Copy link
Contributor

@tstellar is that fine with you? I'd rebase the branch but I don't know the slash command to do that (if there even is one).

This is to fix compile error with explicit Clang modules like
```
../../third_party/libc++/src/include/__vector/vector_bool.h:85:11: error: default argument of '__bit_iterator' must be imported from module 'std.bit_reference_fwd' before it is required
   85 |   typedef __bit_iterator<vector, false> pointer;
      |           ^
../../third_party/libc++/src/include/__fwd/bit_reference.h:23:68: note: default argument declared here is not reachable
   23 | template <class _Cp, bool _IsConst, typename _Cp::__storage_type = 0>
      |                                                                    ^
```

(cherry picked from commit 672e385)
@tstellar tstellar merged commit 424c2d9 into llvm:release/20.x Mar 18, 2025
7 of 10 checks passed
@github-project-automation github-project-automation bot moved this from Needs Merge to Done in LLVM Release Status Mar 18, 2025
Copy link

@ian-twilightcoder (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

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

Development

Successfully merging this pull request may close these issues.

7 participants