[libcxx][fstream][NFC] Make __failed helper lambda a member function #149390
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-libcxx Author: Michael Buch (Michael137) ChangesThis patch makes the The expression evaluator is asserting in the Clang parser: Ideally we'd figure out why LLDB is falling over on this lambda. But to unblock CI for now, make this a member function. In the long run we should figure out the LLDB bug here so libc++ doesn't need to care about whether it uses lambdas like this or not. Full diff: https://github.com/llvm/llvm-project/pull/149390.diff 1 Files Affected:
diff --git a/libcxx/include/fstream b/libcxx/include/fstream
index dc5c47304f014..cf6d5779d1de5 100644
--- a/libcxx/include/fstream
+++ b/libcxx/include/fstream
@@ -401,6 +401,14 @@ private:
}
}
}
+
+ typename traits_type::int_type __overflow_failed() {
+ if (this->pptr() == this->epptr() + 1) {
+ this->pbump(-1); // lose the character we overflowed above -- we don't really have a
+ // choice since we couldn't commit the contents of the put area
+ }
+ return traits_type::eof();
+ }
};
template <class _CharT, class _Traits>
@@ -821,14 +829,6 @@ typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits>
template <class _CharT, class _Traits>
typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits>::overflow(int_type __c) {
- auto __failed = [this]() {
- if (this->pptr() == this->epptr() + 1) {
- this->pbump(-1); // lose the character we overflowed above -- we don't really have a
- // choice since we couldn't commit the contents of the put area
- }
- return traits_type::eof();
- };
-
if (__file_ == nullptr)
return traits_type::eof();
__write_mode();
@@ -850,7 +850,7 @@ typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits>
if (__always_noconv_) {
size_t __n = static_cast<size_t>(this->pptr() - this->pbase());
if (std::fwrite(this->pbase(), sizeof(char_type), __n, __file_) != __n) {
- return __failed();
+ return __overflow_failed();
}
} else {
if (!__cv_)
@@ -864,14 +864,14 @@ typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits>
do {
codecvt_base::result __r = __cv_->out(__st_, __b, __p, __end, __extbuf_, __extbuf_ + __ebs_, __extbuf_end);
if (__end == __b) {
- return __failed();
+ return __overflow_failed();
}
// No conversion needed: output characters directly to the file, done.
if (__r == codecvt_base::noconv) {
size_t __n = static_cast<size_t>(__p - __b);
if (std::fwrite(__b, 1, __n, __file_) != __n) {
- return __failed();
+ return __overflow_failed();
}
break;
@@ -879,7 +879,7 @@ typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits>
} else if (__r == codecvt_base::ok) {
size_t __n = static_cast<size_t>(__extbuf_end - __extbuf_);
if (std::fwrite(__extbuf_, 1, __n, __file_) != __n) {
- return __failed();
+ return __overflow_failed();
}
break;
@@ -888,13 +888,13 @@ typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits>
} else if (__r == codecvt_base::partial) {
size_t __n = static_cast<size_t>(__extbuf_end - __extbuf_);
if (std::fwrite(__extbuf_, 1, __n, __file_) != __n) {
- return __failed();
+ return __overflow_failed();
}
__b = const_cast<char_type*>(__end);
continue;
} else {
- return __failed();
+ return __overflow_failed();
}
} while (true);
}
|
ldionne
approved these changes
Jul 17, 2025
There was a problem hiding this comment.
LGTM with green CI. Can you file a Github issue to figure out the issue and we can add it in a comment here. That way we can go back to the much more readable variant once the bug is fixed.
But in the meantime, we should make sure that LLDB works properly with the version of libc++ we'll include in LLVM 21, so I think it makes sense to land this change and cherry-pick it.
|
Tests pass. Code formatter github action keeps failing with: Seems unrelated so going to merge this. I ran |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/55/builds/14409 Here is the relevant piece of the build log for the reference |
This was referenced Jul 23, 2025
|
/cherry-pick 8f4deff |
|
Failed to cherry-pick: 8f4deff https://github.com/llvm/llvm-project/actions/runs/17307221349 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch makes the
__failedlambda a member function onfstream. This fixes two LLDB expression evaluation test failures that got introduced with #147389:The expression evaluator is asserting in the Clang parser:
Ideally we'd figure out why LLDB is falling over on this lambda. But to unblock CI for now, make this a member function.
In the long run we should figure out the LLDB bug here so libc++ doesn't need to care about whether it uses lambdas like this or not.