Skip to content

[libc++] Revert removal of <movable_box.h> to fix lldb test failure #140960

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

winner245
Copy link
Contributor

@winner245 winner245 commented May 21, 2025

The removal of <__ranges/movable_box.h> from <__algorithm/for_each.h> (#134960) caused lldb test failures (#137046) due to an implicit dependency in clang's AST processing. After discussion with @dmpots, we confirmed that re-adding this header represents the simplest fix for now.

This is a temporary fix for #137046. A long-term resolution may require deeper investigation into lldb and clang behavior.

@winner245 winner245 requested a review from a team as a code owner May 21, 2025 20:42
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label May 21, 2025
@llvmbot
Copy link
Member

llvmbot commented May 21, 2025

@llvm/pr-subscribers-lldb

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

Changes

The removal of &lt;__ranges/movable_box.h&gt; from &lt;__algorithm/for_each.h&gt; (#134960) caused lldb test failures (#137046) due to an implicit dependency in clang's AST processing. After discussion with @dmpots, we confirmed that re-adding this header represents the simplest fix for now.

Note: This is a temporary solution. A long-term resolution may require deeper investigation into lldb and clang behavior.


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

1 Files Affected:

  • (modified) libcxx/include/__algorithm/for_each.h (+1)
diff --git a/libcxx/include/__algorithm/for_each.h b/libcxx/include/__algorithm/for_each.h
index b6c2c7c056edd..9bbccbdd0a5a2 100644
--- a/libcxx/include/__algorithm/for_each.h
+++ b/libcxx/include/__algorithm/for_each.h
@@ -13,6 +13,7 @@
 #include <__algorithm/for_each_segment.h>
 #include <__config>
 #include <__iterator/segmented_iterator.h>
+#include <__ranges/movable_box.h> // Re-included as a temporary fix for lldb test failure (https://github.com/llvm/llvm-project/issues/137046)
 #include <__type_traits/enable_if.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)

@ldionne
Copy link
Member

ldionne commented May 29, 2025

As said in #137046 (comment), I think we might as well mark the failing test as unsupported in LLDB. And then we remove the UNSUPPORTED annotation once the bug is fixed.

So I'm not sure we need to actually land this change.

@winner245
Copy link
Contributor Author

@ldionne I agree with your plan. This is not a bug in libc++. It is better to mark the failing test as unsupported in LLDB as a temporary fix, and then remove the UNSUPPORTED annotation once a long-term fix is available. That said, I will close this PR.

@winner245 winner245 closed this May 30, 2025
@Michael137
Copy link
Member

This is not a bug in libc++. It is better to mark the failing test as unsupported in LLDB

FWIW, I don't think this is the correct stance to take in general with these types of issues. If something breaks in a different subproject, reverting/waiting for the subproject to figure out a fix is what we typically would/should do in LLVM. But since this has been sitting for a while now and this part of LLDB is rather experimental, I agree that's what we should do in this case

@winner245 winner245 deleted the re-add-movable_box branch May 30, 2025 10:51
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. lldb
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants