[libc++] Resolve LWG4366: Heterogeneous comparison of expected may be ill-formed #185342
Open
smallp-o-p wants to merge 4 commits intollvm:mainfrom
Open
[libc++] Resolve LWG4366: Heterogeneous comparison of expected may be ill-formed #185342smallp-o-p wants to merge 4 commits intollvm:mainfrom
expected may be ill-formed #185342smallp-o-p wants to merge 4 commits intollvm:mainfrom
Conversation
llvmbot
added
the
libc++
Member
|
@llvm/pr-subscribers-libcxx Author: William Tran-Viet (smallp-o-p) Changes
Full diff: https://github.com/llvm/llvm-project/pull/185342.diff 5 Files Affected:
diff --git a/libcxx/docs/Status/Cxx2cIssues.csv b/libcxx/docs/Status/Cxx2cIssues.csv
index 60b1bd6ff70da..008c594d8dd1d 100644
--- a/libcxx/docs/Status/Cxx2cIssues.csv
+++ b/libcxx/docs/Status/Cxx2cIssues.csv
@@ -201,7 +201,7 @@
"`LWG4351 <https://wg21.link/LWG4351>`__","``integral-constant-like`` needs more ``remove_cvref_t``","2025-11 (Kona)","","","`#171359 <https://github.com/llvm/llvm-project/issues/171359>`__",""
"`LWG4358 <https://wg21.link/LWG4358>`__","§[exec.as.awaitable] is using ""Preconditions:"" when it should probably be described in the constraint","2025-11 (Kona)","","","`#171360 <https://github.com/llvm/llvm-project/issues/171360>`__",""
"`LWG4360 <https://wg21.link/LWG4360>`__","``awaitable-sender`` concept should qualify use of ``awaitable-receiver`` type","2025-11 (Kona)","","","`#171361 <https://github.com/llvm/llvm-project/issues/171361>`__",""
-"`LWG4366 <https://wg21.link/LWG4366>`__","Heterogeneous comparison of ``expected`` may be ill-formed","2025-11 (Kona)","","","`#171362 <https://github.com/llvm/llvm-project/issues/171362>`__",""
+"`LWG4366 <https://wg21.link/LWG4366>`__","Heterogeneous comparison of ``expected`` may be ill-formed","2025-11 (Kona)","|Complete|","22","`#171362 <https://github.com/llvm/llvm-project/issues/171362>`__",""
"`LWG4369 <https://wg21.link/LWG4369>`__","``check-types`` function for ``upon_error`` and ``upon_stopped`` is wrong","2025-11 (Kona)","","","`#171363 <https://github.com/llvm/llvm-project/issues/171363>`__",""
"`LWG4370 <https://wg21.link/LWG4370>`__","Comparison of ``optional<T>`` to ``T`` may be ill-formed","2025-11 (Kona)","|Complete|","22","`#171364 <https://github.com/llvm/llvm-project/issues/171364>`__",""
"`LWG4372 <https://wg21.link/LWG4372>`__","Weaken *Mandates:* for dynamic padding values in padded layouts","2025-11 (Kona)","","","`#171365 <https://github.com/llvm/llvm-project/issues/171365>`__",""
diff --git a/libcxx/include/__expected/expected.h b/libcxx/include/__expected/expected.h
index 24ae33d4e3af8..fc451b6511200 100644
--- a/libcxx/include/__expected/expected.h
+++ b/libcxx/include/__expected/expected.h
@@ -1169,7 +1169,9 @@ class expected : private __expected_base<_Tp, _Err> {
}
# endif
{
- return __x.__has_val() && static_cast<bool>(__x.__val() == __v);
+ if (__x.__has_val())
+ return __x.__val() == __v;
+ return false;
}
template <class _E2>
@@ -1180,7 +1182,9 @@ class expected : private __expected_base<_Tp, _Err> {
}
# endif
{
- return !__x.__has_val() && static_cast<bool>(__x.__unex() == __e.error());
+ if (!__x.__has_val())
+ return __x.__unex() == __e.error();
+ return false;
}
};
@@ -1881,8 +1885,10 @@ class expected<_Tp, _Err> : private __expected_void_base<_Err> {
{
if (__x.__has_val() != __y.__has_val()) {
return false;
+ } else if (__x.__has_val()) {
+ return true;
} else {
- return __x.__has_val() || static_cast<bool>(__x.__unex() == __y.__unex());
+ return __x.__unex() == __y.__unex();
}
}
@@ -1894,7 +1900,9 @@ class expected<_Tp, _Err> : private __expected_void_base<_Err> {
}
# endif
{
- return !__x.__has_val() && static_cast<bool>(__x.__unex() == __y.error());
+ if (!__x.__has_val())
+ return __x.__unex() == __y.error();
+ return false;
}
};
diff --git a/libcxx/test/std/utilities/expected/expected.expected/equality/equality.T2.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/equality/equality.T2.pass.cpp
index 16c6986ae670e..ebd8ebabbd2c8 100644
--- a/libcxx/test/std/utilities/expected/expected.expected/equality/equality.T2.pass.cpp
+++ b/libcxx/test/std/utilities/expected/expected.expected/equality/equality.T2.pass.cpp
@@ -45,6 +45,19 @@ constexpr bool test() {
assert(e1 != i3);
}
+ // LWG4366
+ { // x.has_value()
+ const std::expected<ImplicitBool::E1, int> e1(ImplicitBool::E1{1});
+ assert(e1 == ImplicitBool::E2{1});
+ assert(e1 != ImplicitBool::E2{3});
+ }
+
+ { // !x.has_value()
+ const std::expected<ImplicitBool::E1, int> e1(std::unexpect, 1);
+ assert(e1 != ImplicitBool::E2{1});
+ assert(e1 != ImplicitBool::E2{2});
+ }
+
return true;
}
diff --git a/libcxx/test/std/utilities/expected/expected.expected/equality/equality.unexpected.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/equality/equality.unexpected.pass.cpp
index 153cbbddf3062..24a6f3d0e906e 100644
--- a/libcxx/test/std/utilities/expected/expected.expected/equality/equality.unexpected.pass.cpp
+++ b/libcxx/test/std/utilities/expected/expected.expected/equality/equality.unexpected.pass.cpp
@@ -45,6 +45,26 @@ constexpr bool test() {
assert(e1 == un3);
}
+ // LWG4366
+ { // x.has_value()
+ const std::expected<int, ImplicitBool::E1> e1(std::in_place, 1);
+ const std::unexpected<ImplicitBool::E2> u2{ImplicitBool::E2{1}};
+ const std::unexpected<ImplicitBool::E2> u3{ImplicitBool::E2{2}};
+
+ assert(e1 != u2);
+ assert(e1 != u3);
+ }
+
+ { // !x.has_value()
+ const std::unexpected<ImplicitBool::E1> u1{ImplicitBool::E1{1}};
+ const std::unexpected<ImplicitBool::E2> u2{ImplicitBool::E2{1}};
+ const std::unexpected<ImplicitBool::E2> u3{ImplicitBool::E2{2}};
+
+ const std::expected<int, ImplicitBool::E1> e1(u1);
+ assert(e1 == u2);
+ assert(e1 != u3);
+ }
+
return true;
}
diff --git a/libcxx/test/support/test_comparisons.h b/libcxx/test/support/test_comparisons.h
index e37ab44828c70..9d8bf910a2ebc 100644
--- a/libcxx/test/support/test_comparisons.h
+++ b/libcxx/test/support/test_comparisons.h
@@ -332,6 +332,23 @@ static_assert(HasOperatorLessThanEqual<ThreeWayComparable>);
static_assert(HasOperatorNotEqual<ThreeWayComparable>);
static_assert(HasOperatorSpaceship<ThreeWayComparable>);
+// LWG4366: Heterogeneous comparison of expected may be ill-formed
+struct ImplicitBool {
+ bool val;
+ constexpr operator bool() const { return val; };
+ constexpr explicit operator bool() = delete;
+
+ struct E1 {
+ int x;
+ };
+
+ struct E2 {
+ int y;
+ };
+};
+
+constexpr ImplicitBool operator==(ImplicitBool::E1 lhs, ImplicitBool::E2 rhs) { return {lhs.x == rhs.y}; }
+
#endif // TEST_STD_VER >= 20
#endif // TEST_COMPARISONS_H
|
philnik777
reviewed
Mar 9, 2026
Co-authored-by: A. Jiang <de34@live.cn>
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions cpp,h -- libcxx/include/__expected/expected.h libcxx/test/std/utilities/expected/expected.expected/equality/equality.T2.pass.cpp libcxx/test/std/utilities/expected/expected.expected/equality/equality.unexpected.pass.cpp libcxx/test/support/test_comparisons.h --diff_from_common_commit
View the diff from clang-format here.diff --git a/libcxx/include/__expected/expected.h b/libcxx/include/__expected/expected.h
index c4b1a37c7..80132f442 100644
--- a/libcxx/include/__expected/expected.h
+++ b/libcxx/include/__expected/expected.h
@@ -449,9 +449,7 @@ private:
// Helper to handle cases for LWG4366 Heterogeneous comparison of ``expected`` may be ill-formed
// Where the comparison may produce a value that is a type that is not bool, and is implicitly
// convertible to bool, but not explicitly.
-constexpr bool __into_bool(bool __b) noexcept {
- return __b;
-}
+constexpr bool __into_bool(bool __b) noexcept { return __b; }
template <class _Tp, class _Err>
class expected : private __expected_base<_Tp, _Err> {
|
| # endif | ||
| { | ||
| return __x.__has_val() && static_cast<bool>(__x.__val() == __v); | ||
| return __x.__has_val() && __into_bool(__x.__val() == __v); |
Contributor
There was a problem hiding this comment.
I think we should use std:: if we decide to invent the __into_bool function, ditto below.
Suggested change
| return __x.__has_val() && __into_bool(__x.__val() == __v); | |
| return __x.__has_val() && std::__into_bool(__x.__val() == __v); |
Pedantically, the return type of operator== is allowed to be ADL-unfriendly. E.g.
template <class T>
struct holder { T t; };
struct incomplete;
template <class T>
struct convsrc {
bool b_;
constexpr operator bool() const { return b_; }
};
using testing_convsrc = convsrc<holder<incomplete>>;
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Resolves #171362
operator==code to be more in line with the standard as the current way was making an explicitbool()conversion in thex.meow() == y.meow()cases