-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Description
In #3965, I tried to fix std::any
's self-assignment. The obvious way is to compare the address first, but I chose a different design (by always making a copy first) to support the if(1)
case below.
The implementation cannot be as efficient as address comparison. And worse, I find that any::swap
is implemented based on std::exchange
, which calls operator=(any&&)
inside. As a result, that PR made any::swap(any&)
a lot less efficient.
// What's the precondition of any-assignment and swap?
void scenarios() {
std::any a = std::make_any<std::any>(42);
std::any& a_o = *std::any_cast<std::any>(&a);
if (1) { // Should this be supported?
a = std::move(a_o);
assert(std::any_cast<int>(a) == 42);
} else { // And what about this?
a_o = std::move(a);
}
// And these?
// a.swap(a_o);
// a_o.swap(a);
}
So the question is, what's the precondition for std::any
's swap and assignment operators? Should we really support those use cases, i,e. can we just compare the address directly?
Even if we cannot compare the address, I think we should still try to improve any::swap
. By inlining the code, it turns out at least one copy is unnecessary.
Related:
The test case added in the original PR, and another PR trying to fix self-assignment:
#5413
https://github.com/microsoft/STL/pull/3965/files#diff-a0f3b054031c8ff90d8a2ad3fc1903f67bb1477d1a1fbf4217c02b55d2301dbe
The current implementation of operator=
and swap
:
Lines 183 to 186 in 37d575e
any& operator=(any&& _That) noexcept { | |
_Assign(_STD move(_That)); | |
return *this; | |
} |
Lines 232 to 234 in 37d575e
void swap(any& _That) noexcept { | |
_That = _STD exchange(*this, _STD move(_That)); | |
} |
https://eel.is/c++draft/any.assign#4.sentence-1
any& operator=(any&& rhs) noexcept;
Effects: As if by any(std::move(rhs)).swap(*this)[.](https://eel.is/c++draft/any.assign#4.sentence-1)
https://eel.is/c++draft/any.modifiers#19.sentence-1
void swap(any& rhs) noexcept;
Effects: Exchanges the states of *this and rhs[.](https://eel.is/c++draft/any.modifiers#19.sentence-1)