-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Bugfixes for <any>
#3965
Bugfixes for <any>
#3965
Conversation
tests/std/tests/Hmm_I_dont_know_how_to_name_this_folder/test.cpp
Outdated
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
stl/inc/any
Outdated
@@ -178,7 +177,7 @@ public: | |||
int> = 0> | |||
any& operator=(_ValueType&& _Value) { | |||
// replace contained value with an object of type decay_t<_ValueType> initialized from _Value | |||
*this = any{_STD forward<_ValueType>(_Value)}; | |||
_Assign(any{_STD forward<_ValueType>(_Value)}); |
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.
This change will hopefully not affect performance of operator=(const any&)
and operator=(auto&&)
.
For operator=(any&&)
, it seems for correctness we have to make a local variable as well; otherwise the testcases introduced in 798e89c will be very hard to deal with.
STL/tests/std/tests/P0220R1_any/test.cpp Line 2051 in 60f1885
I find some tests in P0220R1_any are actually ill-formed ~ an any& is not allowed to any_cast to an rvalue reference.
Though I think this should be well-formed; |
Whether this case is ill-formed (currently) is unspecified - it's unspecified whether any violation of Mandates: is substitution failure when it can be. MSVC STL tends to make the ill-formedness SFINAE-unfriendly, which allows emitting informative message in |
@@ -78,17 +78,17 @@ struct _Any_small_RTTI { // Hand-rolled vtable for nontrivial types that can be | |||
|
|||
template <class _Ty> | |||
static void __CLRCALL_PURE_OR_CDECL _Destroy_impl(void* const _Target) noexcept { | |||
_Destroy_in_place(*static_cast<_Ty*>(_Target)); | |||
_STD _Destroy_in_place(*static_cast<_Ty*>(_Target)); |
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.
This PR is combining unrelated fixes, which makes things harder to review and harder to investigate/revert when things go wrong. They're small enough that I don't think we need to request changes, but I wanted to mention this for the future.
In general, I would say that combining unrelated cleanups is less risky than combining fixes; we want to avoid grab-bag PRs either way, but when semantic changes are involved, we should be more disciplined.
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
I had to push an additional commit to work around VSO-1659496 " |
⭐ 🐞 🛠️ |
This pr contains two fixes for
std::any
:any
's self-move-assignment (which will result in the object being destroyed).any
can holdholder<incomplete>*
etc.