Don't include <xmemory> in <optional> and <variant>#3624
Don't include <xmemory> in <optional> and <variant>#3624StephanTLavavej merged 4 commits intomicrosoft:mainfrom
<xmemory> in <optional> and <variant>#3624Conversation
strega-nil-ms
left a comment
There was a problem hiding this comment.
Seems reasonable, but I don't like removing explicit dependencies, personally (since the throughput gains are negligible).
| _CONSTEXPR20 ~_Optional_destruct_base() noexcept { | ||
| if (_Has_value) { | ||
| _Destroy_in_place(_Value); | ||
| _Value.~_Ty(); |
There was a problem hiding this comment.
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.
|
Looks good to me, thanks! I think that the breaking change impact here is limited - |
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
|
Thanks for finding and implementing this throughput improvement! 🚀 🐇 🐆 |
Separated from #3623.
std::variantandstd::optionalforbid using an array type as component, so it's unnecessary to include<xmemory>to obtain_Destroy_in_place.I think it's OK to writeEdit: dropping this due to review comments.<xutility>for short. Several other headers, including conditionally included<compare>, are already handled in<xutility>.Edit: I also need to reduce the
numeric_limitsdepedency (needed for EDG?). I expect that the current approach is transient.