Skip to content

Don't include <xmemory> in <optional> and <variant> #3624

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.

Sign up for GitHub

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

Merged
StephanTLavavej merged 4 commits into from
Apr 7, 2023
Merged

Don't include <xmemory> in <optional> and <variant> #3624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3f6d939
Don't include `<xmemory>` in `<optional>` and `<variant>`
frederick-vs-ja Apr 5, 2023
1e04d12
Fix destructor calls
frederick-vs-ja Apr 5, 2023
8647ef5
Reduce `numeric_limits` dependency for `variant`
frederick-vs-ja Apr 5, 2023
2fabe95
Address @strega-nil-ms's review comments
frederick-vs-ja Apr 5, 2023
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
6 changes: 3 additions & 3 deletions stl/inc/optional
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ _EMIT_STL_WARNING(STL4038, "The contents of <optional> are available only with C
#include <initializer_list>
#include <type_traits>
#include <utility>
#include <xmemory>
#include <xsmf_control.h>
#include <xutility>

#pragma pack(push, _CRT_PACKING)
#pragma warning(push, _STL_WARNING_LEVEL)
Expand Down Expand Up @@ -104,7 +104,7 @@ struct _Optional_destruct_base<_Ty, false> { // either contains a value of _Ty o

_CONSTEXPR20 ~_Optional_destruct_base() noexcept {
if (_Has_value) {
_Destroy_in_place(_Value);
_Value.~_Ty();
Copy link
Member

@StephanTLavavej StephanTLavavej Apr 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change requested: I observe that we're storing remove_cv_t<_Ty> _Value; and saying only ~_Ty(). This seems to be acceptable according to the Standardese, and all compilers accept, so I don't think we need to add remove_cv_t here.

}
}

Expand All @@ -129,7 +129,7 @@ struct _Optional_destruct_base<_Ty, false> { // either contains a value of _Ty o

_CONSTEXPR20 void reset() noexcept {
if (_Has_value) {
_Destroy_in_place(_Value);
_Value.~_Ty();
_Has_value = false;
}
}
Expand Down
20 changes: 13 additions & 7 deletions stl/inc/variant
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@
#if !_HAS_CXX17
_EMIT_STL_WARNING(STL4038, "The contents of <variant> are available only with C++17 or later.");
#else // ^^^ !_HAS_CXX17 / _HAS_CXX17 vvv
#if _HAS_CXX20
#include <compare>
#endif // _HAS_CXX20
#include <exception>
#include <initializer_list>
#include <type_traits>
#include <utility>
#include <xmemory>
#include <xsmf_control.h>
#include <xstddef>
#include <xutility>

#pragma pack(push, _CRT_PACKING)
#pragma warning(push, _STL_WARNING_LEVEL)
Expand Down Expand Up @@ -693,10 +695,12 @@ _NODISCARD constexpr _Variant_raw_visit_t<_Fn, _Storage> _Variant_raw_visit(
template <class...>
class _Variant_base;

inline constexpr size_t _Schar_max_as_size = static_cast<unsigned char>(-1) / 2;
inline constexpr size_t _Short_max_as_size = static_cast<unsigned short>(-1) / 2;

template <size_t _Count>
using _Variant_index_t = // signed so that conversion of -1 to size_t can cheaply sign extend
conditional_t<(_Count < static_cast<size_t>((numeric_limits<signed char>::max)())), signed char,
conditional_t<(_Count < static_cast<size_t>((numeric_limits<short>::max)())), short, int>>;
conditional_t<(_Count < _Schar_max_as_size), signed char, conditional_t<(_Count < _Short_max_as_size), short, int>>;

template <class... _Types>
struct _Variant_construct_visitor { // visitor that constructs the same alternative in a target _Variant_base as is
Expand Down Expand Up @@ -808,16 +812,18 @@ public:
_CONSTEXPR20 void _Destroy() noexcept {
// destroy the contained value
// pre: _Idx == index()
if constexpr (_Idx != variant_npos && !is_trivially_destructible_v<_Meta_at_c<variant<_Types...>, _Idx>>) {
_STD _Destroy_in_place(_STD _Variant_raw_get<_Idx>(_Storage()));
using _Indexed_value_type = remove_cv_t<_Meta_at_c<variant<_Types...>, _Idx>>;
if constexpr (_Idx != variant_npos && !is_trivially_destructible_v<_Indexed_value_type>) {
_STD _Variant_raw_get<_Idx>(_Storage()).~_Indexed_value_type();
}
}

_CONSTEXPR20 void _Destroy() noexcept { // destroy the contained value, if any
if constexpr (!conjunction_v<is_trivially_destructible<_Types>...>) {
_STD _Variant_raw_visit(index(), _Storage(), [](auto _Ref) noexcept {
if constexpr (decltype(_Ref)::_Idx != variant_npos) {
_STD _Destroy_in_place(_Ref._Val);
using _Indexed_value_type = _Remove_cvref_t<decltype(_Ref._Val)>;
_Ref._Val.~_Indexed_value_type();
}
});
}
Expand Down