-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
[libcxx] Add LWG4135: The helper lambda of std::erase for list should specify return type as bool #128358
Conversation
@llvm/pr-subscribers-libcxx Author: None (elhewaty) ChangesFixes #118355 Full diff: https://github.com/llvm/llvm-project/pull/128358.diff 5 Files Affected:
diff --git a/libcxx/docs/Status/Cxx2cIssues.csv b/libcxx/docs/Status/Cxx2cIssues.csv
index 1ec23dfabd5ea..228d95dead807 100644
--- a/libcxx/docs/Status/Cxx2cIssues.csv
+++ b/libcxx/docs/Status/Cxx2cIssues.csv
@@ -97,7 +97,7 @@
"`LWG4124 <https://wg21.link/LWG4124>`__","Cannot format ``zoned_time`` with resolution coarser than ``seconds``","2024-11 (Wrocław)","","",""
"`LWG4126 <https://wg21.link/LWG4126>`__","Some feature-test macros for fully freestanding features are not yet marked freestanding","2024-11 (Wrocław)","","",""
"`LWG4134 <https://wg21.link/LWG4134>`__","Issue with Philox algorithm specification","2024-11 (Wrocław)","","",""
-"`LWG4135 <https://wg21.link/LWG4135>`__","The helper lambda of ``std::erase`` for list should specify return type as ``bool``","2024-11 (Wrocław)","","",""
+"`LWG4135 <https://wg21.link/LWG4135>`__","The helper lambda of ``std::erase`` for list should specify return type as ``bool``","2024-11 (Wrocław)","|Complete|","21",""
"`LWG4140 <https://wg21.link/LWG4140>`__","Useless default constructors for bit reference types","2024-11 (Wrocław)","","",""
"`LWG4141 <https://wg21.link/LWG4141>`__","Improve prohibitions on ""additional storage""","2024-11 (Wrocław)","","",""
"`LWG4142 <https://wg21.link/LWG4142>`__","``format_parse_context::check_dynamic_spec`` should require at least one type","2024-11 (Wrocław)","","",""
diff --git a/libcxx/include/forward_list b/libcxx/include/forward_list
index 4b6ca8ea8587c..8c688611d5ee2 100644
--- a/libcxx/include/forward_list
+++ b/libcxx/include/forward_list
@@ -1557,7 +1557,7 @@ erase_if(forward_list<_Tp, _Allocator>& __c, _Predicate __pred) {
template <class _Tp, class _Allocator, class _Up>
inline _LIBCPP_HIDE_FROM_ABI typename forward_list<_Tp, _Allocator>::size_type
erase(forward_list<_Tp, _Allocator>& __c, const _Up& __v) {
- return std::erase_if(__c, [&](auto& __elem) { return __elem == __v; });
+ return std::erase_if(__c, [&](const auto& __elem) -> bool { return __elem == __v; });
}
# endif
diff --git a/libcxx/include/list b/libcxx/include/list
index 3fcf796ebc03d..1285174f1c384 100644
--- a/libcxx/include/list
+++ b/libcxx/include/list
@@ -1703,7 +1703,7 @@ erase_if(list<_Tp, _Allocator>& __c, _Predicate __pred) {
template <class _Tp, class _Allocator, class _Up>
inline _LIBCPP_HIDE_FROM_ABI typename list<_Tp, _Allocator>::size_type
erase(list<_Tp, _Allocator>& __c, const _Up& __v) {
- return std::erase_if(__c, [&](auto& __elem) { return __elem == __v; });
+ return std::erase_if(__c, [&](const auto& __elem) -> bool { return __elem == __v; });
}
template <>
diff --git a/libcxx/test/libcxx/containers/sequences/forwardlist/bool-conversion.pass.cpp b/libcxx/test/libcxx/containers/sequences/forwardlist/bool-conversion.pass.cpp
new file mode 100644
index 0000000000000..402ea1e2b1880
--- /dev/null
+++ b/libcxx/test/libcxx/containers/sequences/forwardlist/bool-conversion.pass.cpp
@@ -0,0 +1,28 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <forward_list>
+
+#include <forward_list>
+struct Bool {
+ Bool() = default;
+ Bool(const Bool&) = delete;
+ operator bool() const { return true; }
+};
+
+struct Int {
+ Bool& operator==(Int) const {
+ static Bool b;
+ return b;
+ }
+};
+
+int main() {
+ std::forward_list<Int> l;
+ std::erase(l, Int{});
+}
diff --git a/libcxx/test/libcxx/containers/sequences/list/list.modifiers/bool-conversion.pass.cpp b/libcxx/test/libcxx/containers/sequences/list/list.modifiers/bool-conversion.pass.cpp
new file mode 100644
index 0000000000000..eb26e9e700f68
--- /dev/null
+++ b/libcxx/test/libcxx/containers/sequences/list/list.modifiers/bool-conversion.pass.cpp
@@ -0,0 +1,28 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <list>
+
+#include <list>
+struct Bool {
+ Bool() = default;
+ Bool(const Bool&) = delete;
+ operator bool() const { return true; }
+};
+
+struct Int {
+ Bool& operator==(Int) const {
+ static Bool b;
+ return b;
+ }
+};
+
+int main() {
+ std::list<Int> l;
+ std::erase(l, Int{});
+}
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This almost ready to go, it needs a few changes to the test.
libcxx/test/libcxx/containers/sequences/forwardlist/bool-conversion.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/libcxx/containers/sequences/forwardlist/bool-conversion.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/libcxx/containers/sequences/forwardlist/bool-conversion.pass.cpp
Show resolved
Hide resolved
6e3f411
to
9e61cf3
Compare
9e61cf3
to
62ddae3
Compare
libcxx/test/libcxx/containers/sequences/forwardlist/bool-conversion.pass.cpp
Show resolved
Hide resolved
62ddae3
to
d8d0a4f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor thing and then it's great.
libcxx/test/libcxx/containers/sequences/forwardlist/bool-conversion.pass.cpp
Outdated
Show resolved
Hide resolved
… specify return type as bool
d8d0a4f
to
ed34b2f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM!
… specify return type as bool (llvm#128358) Fixes llvm#118355
… specify return type as bool (llvm#128358) Fixes llvm#118355
… specify return type as bool (llvm#128358) Fixes llvm#118355
Fixes #118355