Skip to content

Commit 16f3492

Browse files
authored
[libc++] restrict the expected conversion constructor not compete against copy constructor (llvm#96101)
fixes llvm#92676 So right now clang does not like ``` std::expected<std::any, int> e1; auto e2 = e1; ``` So basically when clang tries to do overload resolution of `auto e2 = e1;` It finds ``` expected(const expected&); // 1. This is OK expected(const expected<_Up, _OtherErr>&) requires __can_convert; // 2. This needs to check its constraints ``` Then in `__can_convert`, one of the check is ``` _Not<is_constructible<_Tp, expected<_Up, _OtherErr>&>> ``` which is checking ``` is_constructible<std::any, expected<_Up, _OtherErr>&> ``` Then it looks at `std::any`'s constructor ``` template < class _ValueType, class _Tp = decay_t<_ValueType>, class = enable_if_t< !is_same<_Tp, any>::value && !__is_inplace_type<_ValueType>::value && is_copy_constructible<_Tp>::value> > any(_ValueType&& __value); ``` In the above, `is_copy_constructible<_Tp>` expands to ``` is_copy_constructible<std::expected<std::any, int>> ``` And the above goes back to the original thing we asked : copy the `std::expected`, which goes to the overload resolution again. ``` expected(const expected&); expected(const expected<_Up, _OtherErr>&) requires __can_convert; ``` So the second overload results in a logical cycle. I am not a language lawyer. We could argue that clang should give up on the second overload which has logical cycle, as the first overload is a perfect match. Anyway, the fix in this patch tries to short-circuiting the second overload's constraint check: that is, if the argument matches exact same `expected<T, E>`, we give up immediately and let the copy constructor to deal with it
1 parent f782ff8 commit 16f3492

File tree

2 files changed

+23
-1
lines changed

2 files changed

+23
-1
lines changed

libcxx/include/__expected/expected.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,9 @@ class expected : private __expected_base<_Tp, _Err> {
507507
_And< is_constructible<_Tp, _UfQual>,
508508
is_constructible<_Err, _OtherErrQual>,
509509
_If<_Not<is_same<remove_cv_t<_Tp>, bool>>::value,
510-
_And< _Not<is_constructible<_Tp, expected<_Up, _OtherErr>&>>,
510+
_And<
511+
_Not<_And<is_same<_Tp, _Up>, is_same<_Err, _OtherErr>>>, // use the copy constructor instead, see #92676
512+
_Not<is_constructible<_Tp, expected<_Up, _OtherErr>&>>,
511513
_Not<is_constructible<_Tp, expected<_Up, _OtherErr>>>,
512514
_Not<is_constructible<_Tp, const expected<_Up, _OtherErr>&>>,
513515
_Not<is_constructible<_Tp, const expected<_Up, _OtherErr>>>,

libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.copy.pass.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,16 @@ static_assert(!std::is_trivially_copy_constructible_v<std::expected<CopyableNonT
6262
static_assert(!std::is_trivially_copy_constructible_v<std::expected<int, CopyableNonTrivial>>);
6363
static_assert(!std::is_trivially_copy_constructible_v<std::expected<CopyableNonTrivial, CopyableNonTrivial>>);
6464

65+
struct Any {
66+
constexpr Any() = default;
67+
constexpr Any(const Any&) = default;
68+
constexpr Any& operator=(const Any&) = default;
69+
70+
template <class T>
71+
requires(!std::is_same_v<Any, std::decay_t<T>> && std::is_copy_constructible_v<std::decay_t<T>>)
72+
constexpr Any(T&&) {}
73+
};
74+
6575
constexpr bool test() {
6676
// copy the value non-trivial
6777
{
@@ -109,6 +119,16 @@ constexpr bool test() {
109119
assert(!e2.has_value());
110120
}
111121

122+
{
123+
// TODO(LLVM 20): Remove once we drop support for Clang 17
124+
#if defined(TEST_CLANG_VER) && TEST_CLANG_VER >= 1800
125+
// https://github.com/llvm/llvm-project/issues/92676
126+
std::expected<Any, int> e1;
127+
auto e2 = e1;
128+
assert(e2.has_value());
129+
#endif
130+
}
131+
112132
return true;
113133
}
114134

0 commit comments

Comments
 (0)