AK: Fix "assignment from temporary" check of Optional::operator=

There was an existing check to ensure that `U` was an lvalue reference,
but when this check fails, overload resolution will just move right on
to the copy asignment operator, which will cause the temporary to be
assigned anyway.

Disallowing `Optional<T&>`s to be created from temporaries entirely
would be undesired, since existing code has valid reasons for creating
`Optional<T&>`s from temporaries, such as for function call arguments.

This fix explicitly deletes the `Optional::operator=(U&&)` operator,
so overload resolution stops.
This commit is contained in:
Jonne Ransijn 2024-11-25 00:25:57 +01:00 committed by Andrew Kaster
parent 84e73c7f5f
commit 75b482bbb9
Notes: github-actions[bot] 2024-11-25 06:05:37 +00:00
2 changed files with 37 additions and 3 deletions

View file

@ -446,16 +446,22 @@ public:
return *this;
}
// Note: Disallows assignment from a temporary as this does not do any lifetime extension.
template<typename U>
requires(!IsSame<OptionalNone, RemoveCVReference<U>>)
ALWAYS_INLINE Optional& operator=(U&& value)
requires(CanBePlacedInOptional<U> && IsLvalueReference<U>)
ALWAYS_INLINE Optional& operator=(U& value)
requires(CanBePlacedInOptional<U>)
{
m_pointer = &value;
return *this;
}
// Note: Disallows assignment from a temporary as this does not do any lifetime extension.
template<typename U>
requires(!IsSame<OptionalNone, RemoveCVReference<U>>)
ALWAYS_INLINE consteval Optional& operator=(RemoveReference<U> const&& value)
requires(CanBePlacedInOptional<U>)
= delete;
ALWAYS_INLINE void clear()
{
m_pointer = nullptr;

View file

@ -272,6 +272,34 @@ TEST_CASE(comparison_reference)
EXPECT_NE(opt1, opt3);
}
template<typename To, typename From>
struct CheckAssignments;
template<typename To, typename From>
requires(requires { declval<To>() = declval<From>(); })
struct CheckAssignments<To, From> {
static constexpr bool allowed = true;
};
template<typename To, typename From>
requires(!requires { declval<To>() = declval<From>(); })
struct CheckAssignments<To, From> {
static constexpr bool allowed = false;
};
static_assert(CheckAssignments<Optional<int>, int>::allowed);
static_assert(!CheckAssignments<Optional<int*>, double*>::allowed);
static_assert(CheckAssignments<Optional<int&>, int&>::allowed);
static_assert(!CheckAssignments<Optional<int&>, int const&>::allowed);
static_assert(!CheckAssignments<Optional<int&>, int&&>::allowed);
static_assert(!CheckAssignments<Optional<int&>, int const&&>::allowed);
static_assert(CheckAssignments<Optional<int const&>, int&>::allowed);
static_assert(CheckAssignments<Optional<int const&>, int const&>::allowed);
static_assert(CheckAssignments<Optional<int const&>, int&&>::allowed); // Lifetime extension
static_assert(CheckAssignments<Optional<int const&>, int const&&>::allowed); // Lifetime extension
TEST_CASE(string_specialization)
{
EXPECT_EQ(sizeof(Optional<String>), sizeof(String));