-
Notifications
You must be signed in to change notification settings - Fork 218
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
Replace boost type_traits with stl type_traits. #1373
base: develop
Are you sure you want to change the base?
Replace boost type_traits with stl type_traits. #1373
Conversation
@@ -65,10 +64,10 @@ class serialization_storage | |||
} | |||
T * address() | |||
{ | |||
return static_cast<T*>(m_storage.address()); | |||
return static_cast<void*>(&m_storage); |
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.
@awulkiew is this fine?
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.
std::aligned_storage
is deprecated in C++23. AFAIR boost version is superior to the std one but I can't remember why besides the fact that it doesn't force us to cast address of the storage. A proposed replacement is something like this:
alignas(T) std::byte storage[sizeof(T)];
but it requires C++17 and would require careful testing. So I'd prefer to keep boost::aligned_storage
for now.
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.
Btw, here https://en.cppreference.com/w/cpp/types/aligned_storage the required use of std::launder
is mentioned. This change can introduce some unwanted effects. I guess I strongly suggest to keep the old version. :)
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.
Thanks! Looks good to me, one question to Adam
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.
Thanks, this look OK to me.
@@ -65,10 +64,10 @@ class serialization_storage | |||
} | |||
T * address() | |||
{ | |||
return static_cast<T*>(m_storage.address()); | |||
return static_cast<void*>(&m_storage); |
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.
Btw, here https://en.cppreference.com/w/cpp/types/aligned_storage the required use of std::launder
is mentioned. This change can introduce some unwanted effects. I guess I strongly suggest to keep the old version. :)
With C++14, is possible to replace Boost.TypeTraits library usages with standard
type_traits
. This PR removes direct Boost.TypeTraits dependency in Geometry module.