Skip to content
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

Merged
merged 23 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions stl/inc/any
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Copy link
Member

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.

}

template <class _Ty>
static void __CLRCALL_PURE_OR_CDECL _Copy_impl(void* const _Target, const void* const _Source) {
_Construct_in_place(*static_cast<_Ty*>(_Target), *static_cast<const _Ty*>(_Source));
_STD _Construct_in_place(*static_cast<_Ty*>(_Target), *static_cast<const _Ty*>(_Source));
}

template <class _Ty>
static void __CLRCALL_PURE_OR_CDECL _Move_impl(void* const _Target, void* const _Source) noexcept {
_Construct_in_place(*static_cast<_Ty*>(_Target), _STD move(*static_cast<_Ty*>(_Source)));
_STD _Construct_in_place(*static_cast<_Ty*>(_Target), _STD move(*static_cast<_Ty*>(_Source)));
}

_Destroy_fn* _Destroy;
Expand Down Expand Up @@ -163,13 +163,12 @@ public:

// Assignment [any.assign]
any& operator=(const any& _That) {
*this = any{_That};
_Assign(_That);
return *this;
}

any& operator=(any&& _That) noexcept {
reset();
_Move_from(_That);
_Assign(_STD move(_That));
return *this;
}

Expand All @@ -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(_STD forward<_ValueType>(_Value));
return *this;
}

Expand Down Expand Up @@ -290,19 +289,24 @@ private:
}
}

void _Assign(any _That) noexcept { // intentionally pass by value
reset();
_Move_from(_That);
}

template <class _Decayed, class... _Types>
_Decayed& _Emplace(_Types&&... _Args) { // emplace construct _Decayed
if constexpr (_Any_is_trivial<_Decayed>) {
// using the _Trivial representation
auto& _Obj = reinterpret_cast<_Decayed&>(_Storage._TrivialData);
_Construct_in_place(_Obj, _STD forward<_Types>(_Args)...);
_STD _Construct_in_place(_Obj, _STD forward<_Types>(_Args)...);
_Storage._TypeData =
reinterpret_cast<uintptr_t>(&typeid(_Decayed)) | static_cast<uintptr_t>(_Any_representation::_Trivial);
return _Obj;
} else if constexpr (_Any_is_small<_Decayed>) {
// using the _Small representation
auto& _Obj = reinterpret_cast<_Decayed&>(_Storage._SmallStorage._Data);
_Construct_in_place(_Obj, _STD forward<_Types>(_Args)...);
_STD _Construct_in_place(_Obj, _STD forward<_Types>(_Args)...);
_Storage._SmallStorage._RTTI = &_Any_small_RTTI_obj<_Decayed>;
_Storage._TypeData =
reinterpret_cast<uintptr_t>(&typeid(_Decayed)) | static_cast<uintptr_t>(_Any_representation::_Small);
Expand Down
53 changes: 53 additions & 0 deletions tests/std/tests/P0220R1_any/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2570,21 +2570,41 @@ namespace msvc {
// empty
any a;
a = std::move(a);
assertEmpty(a);

a = std::make_any<any>();
a = any_cast<any&&>(std::move(a)); // extract inner any
assertEmpty(a);
}
{
// small
any a{small{42}};
a = std::move(a);
assertContains<small>(a, 42);

a = std::make_any<any>(small{42});
a = any_cast<any&&>(std::move(a)); // extract inner any
assertContains<small>(a, 42);
}
{
// large
any a{large{42}};
a = std::move(a);
assertContains<large>(a, 42);

a = std::make_any<any>(large{42});
a = any_cast<any&&>(std::move(a)); // extract inner any
assertContains<large>(a, 42);
}
{
// trivial
any a{int{42}};
a = std::move(a);
assertContains<int>(a, 42);

a = std::make_any<any>(int{42});
a = any_cast<any&&>(std::move(a)); // extract inner any
assertContains<int>(a, 42);
}
}
#ifdef __clang__
Expand Down Expand Up @@ -3160,6 +3180,38 @@ namespace msvc {
}
#pragma warning(pop)
} // namespace trivial

namespace gh_140_robust_against_adl {
struct incomplete;

template <class T>
struct wrapper {
T t;
};

template <class Type>
void test_for() {
any a;
a = any{Type()};
a = any{std::in_place_type<Type>};
a = Type();
a = std::make_any<Type>();
a.emplace<Type>();
assert(any_cast<Type>(&a) != nullptr);
}

void run_test() {
using _trivial = wrapper<incomplete>*;
using _small = std::pair<_trivial, small>;
using _large = std::pair<_trivial, large>;

globalMemCounter.disable_allocations = true;
test_for<_trivial>();
test_for<_small>();
globalMemCounter.disable_allocations = false;
test_for<_large>();
}
} // namespace gh_140_robust_against_adl
} // namespace msvc

int main() {
Expand Down Expand Up @@ -3196,4 +3248,5 @@ int main() {
msvc::size_and_alignment::run_test();
msvc::small_type::run_test();
msvc::trivial::run_test();
msvc::gh_140_robust_against_adl::run_test();
achabense marked this conversation as resolved.
Show resolved Hide resolved
}