Skip to content

Commit 8c1958c

Browse files
tope99tope99
committed
[#681] Make InplaceOptionalHolder nothrow_move_constructible
1 parent 6a93351 commit 8c1958c

3 files changed

Lines changed: 47 additions & 14 deletions

File tree

‎compiler/extensions/cpp/runtime/ClangTidySuppressions.txt‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,16 +73,16 @@ cppcoreguidelines-pro-bounds-pointer-arithmetic:src/zserio/BitBuffer.h:274
7373
# This is false positive, the member is initialized.
7474
cppcoreguidelines-pro-type-member-init:src/zserio/BitStreamWriter.h:70
7575
# This is aligned storage which we want to leave uninitialized.
76-
cppcoreguidelines-pro-type-member-init:src/zserio/OptionalHolder.h:693
76+
cppcoreguidelines-pro-type-member-init:src/zserio/OptionalHolder.h:694
7777

7878
# This is necessary for implementation of low level implementation of AnyHolder and OptionalHolder to mimic
7979
# standard C++17 'any' and 'optional' abstractions.
8080
cppcoreguidelines-pro-type-reinterpret-cast:src/zserio/AnyHolder.h:832
8181
cppcoreguidelines-pro-type-reinterpret-cast:src/zserio/AnyHolder.h:838
8282
cppcoreguidelines-pro-type-reinterpret-cast:src/zserio/AnyHolder.h:868
8383
cppcoreguidelines-pro-type-reinterpret-cast:src/zserio/AnyHolder.h:875
84-
cppcoreguidelines-pro-type-reinterpret-cast:src/zserio/OptionalHolder.h:657
85-
cppcoreguidelines-pro-type-reinterpret-cast:src/zserio/OptionalHolder.h:667
84+
cppcoreguidelines-pro-type-reinterpret-cast:src/zserio/OptionalHolder.h:658
85+
cppcoreguidelines-pro-type-reinterpret-cast:src/zserio/OptionalHolder.h:668
8686
# This is necessary for implementation of reading and writing to the file.
8787
cppcoreguidelines-pro-type-reinterpret-cast:src/zserio/FileUtil.cpp:19
8888
cppcoreguidelines-pro-type-reinterpret-cast:src/zserio/FileUtil.cpp:49
@@ -100,9 +100,9 @@ google-explicit-constructor:src/zserio/BitStreamReader.h:43
100100
google-explicit-constructor:src/zserio/OptionalHolder.h:231
101101
google-explicit-constructor:src/zserio/OptionalHolder.h:241
102102
google-explicit-constructor:src/zserio/OptionalHolder.h:251
103-
google-explicit-constructor:src/zserio/OptionalHolder.h:698
104-
google-explicit-constructor:src/zserio/OptionalHolder.h:706
105-
google-explicit-constructor:src/zserio/OptionalHolder.h:717
103+
google-explicit-constructor:src/zserio/OptionalHolder.h:699
104+
google-explicit-constructor:src/zserio/OptionalHolder.h:707
105+
google-explicit-constructor:src/zserio/OptionalHolder.h:718
106106
google-explicit-constructor:src/zserio/pmr/PolymorphicAllocator.h:46
107107
google-explicit-constructor:src/zserio/pmr/PolymorphicAllocator.h:71
108108
google-explicit-constructor:src/zserio/Span.h:114
@@ -164,6 +164,10 @@ cppcoreguidelines-pro-bounds-pointer-arithmetic:test/zserio/StringViewTest.cpp:2
164164
# Intentional tests.
165165
cppcoreguidelines-pro-type-reinterpret-cast:test/zserio/SqliteConnectionTest.cpp:320
166166

167+
# Just a test for move ctor should not define all other ctors
168+
cppcoreguidelines-special-member-functions:test/zserio/InplaceOptionalHolderTest.cpp:403
169+
cppcoreguidelines-special-member-functions:test/zserio/InplaceOptionalHolderTest.cpp:410
170+
167171
# Intentional tests. It is necessary for readability.
168172
google-build-using-namespace:test/zserio/ReflectableTest.cpp:22
169173

‎compiler/extensions/cpp/runtime/src/zserio/OptionalHolder.h‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ class in_place_storage
614614
*
615615
* \return Reference to the current storage.
616616
*/
617-
in_place_storage& operator=(in_place_storage&& other)
617+
in_place_storage& operator=(in_place_storage&& other) noexcept(std::is_nothrow_move_constructible<T>::value)
618618
{
619619
new (&m_inPlace) T(std::move(*other.getObject()));
620620
other.getObject()->~T(); // ensure that destructor of object in original storage is called
@@ -629,7 +629,8 @@ class in_place_storage
629629
*
630630
* \return Reference to the current storage.
631631
*/
632-
in_place_storage& assign(NoInitT, in_place_storage&& other)
632+
in_place_storage& assign(NoInitT, in_place_storage&& other) noexcept(
633+
std::is_nothrow_move_constructible<T>::value)
633634
{
634635
new (&m_inPlace) T(NoInit, std::move(*other.getObject()));
635636
other.getObject()->~T(); // ensure that destructor of object in original storage is called
@@ -714,7 +715,7 @@ class inplace_optional_holder : public optional_holder_base<T, inplace_optional_
714715
*
715716
* \param val Value to store in the holder.
716717
*/
717-
inplace_optional_holder(T&& val)
718+
inplace_optional_holder(T&& val) noexcept(std::is_nothrow_constructible<T>::value)
718719
{
719720
new (m_storage.getStorage()) T(std::move(val));
720721
m_hasValue = true;
@@ -728,7 +729,7 @@ class inplace_optional_holder : public optional_holder_base<T, inplace_optional_
728729
*/
729730
template <typename U = T,
730731
typename std::enable_if<std::is_constructible<U, NoInitT, U>::value, int>::type = 0>
731-
inplace_optional_holder(NoInitT, T&& val)
732+
inplace_optional_holder(NoInitT, T&& val) noexcept(std::is_nothrow_move_constructible<T>::value)
732733
{
733734
new (m_storage.getStorage()) T(NoInit, std::move(val));
734735
m_hasValue = true;
@@ -770,7 +771,7 @@ class inplace_optional_holder : public optional_holder_base<T, inplace_optional_
770771
* \param other Other holder to move.
771772
*/
772773
inplace_optional_holder(inplace_optional_holder&& other) noexcept(
773-
std::is_nothrow_move_constructible<in_place_storage<T>>::value)
774+
std::is_nothrow_move_assignable<in_place_storage<T>>::value)
774775
{
775776
if (other.hasValue())
776777
{
@@ -788,7 +789,7 @@ class inplace_optional_holder : public optional_holder_base<T, inplace_optional_
788789
template <typename U = T,
789790
typename std::enable_if<std::is_constructible<U, NoInitT, U>::value, int>::type = 0>
790791
inplace_optional_holder(NoInitT, inplace_optional_holder&& other) noexcept(
791-
std::is_nothrow_move_constructible<in_place_storage<T>>::value)
792+
std::is_nothrow_move_assignable<in_place_storage<T>>::value)
792793
{
793794
if (other.hasValue())
794795
{
@@ -871,7 +872,8 @@ class inplace_optional_holder : public optional_holder_base<T, inplace_optional_
871872
*
872873
* \return Reference to the current holder.
873874
*/
874-
inplace_optional_holder& operator=(inplace_optional_holder&& other)
875+
inplace_optional_holder& operator=(inplace_optional_holder&& other) noexcept(
876+
std::is_nothrow_move_assignable<in_place_storage<T>>::value)
875877
{
876878
if (this != &other)
877879
{
@@ -896,7 +898,8 @@ class inplace_optional_holder : public optional_holder_base<T, inplace_optional_
896898
*/
897899
template <typename U = T,
898900
typename std::enable_if<std::is_constructible<U, NoInitT, U>::value, int>::type = 0>
899-
inplace_optional_holder& assign(NoInitT, inplace_optional_holder&& other)
901+
inplace_optional_holder& assign(NoInitT, inplace_optional_holder&& other) noexcept(
902+
std::is_nothrow_move_assignable<in_place_storage<T>>::value)
900903
{
901904
if (this != &other)
902905
{

‎compiler/extensions/cpp/runtime/test/zserio/InplaceOptionalHolderTest.cpp‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,4 +400,30 @@ TEST_F(InplaceOptionalHolderTest, constGet)
400400
ASSERT_EQ(intValue, optionalObject->getValue());
401401
}
402402

403+
struct NothrowMoveCtor
404+
{
405+
NothrowMoveCtor() = default;
406+
NothrowMoveCtor(NothrowMoveCtor&&) noexcept
407+
{}
408+
};
409+
410+
struct ThrowMoveCtor
411+
{
412+
ThrowMoveCtor() = default;
413+
ThrowMoveCtor(ThrowMoveCtor&&)
414+
{}
415+
};
416+
417+
TEST_F(InplaceOptionalHolderTest, nothrowMoveConstructible)
418+
{
419+
static_assert(std::is_nothrow_move_constructible<zserio::InplaceOptionalHolder<NothrowMoveCtor>>::value,
420+
"IsNothrowMove");
421+
static_assert(std::is_nothrow_move_assignable<zserio::InplaceOptionalHolder<NothrowMoveCtor>>::value,
422+
"IsNothrowMove");
423+
static_assert(!std::is_nothrow_move_constructible<zserio::InplaceOptionalHolder<ThrowMoveCtor>>::value,
424+
"!IsNothrowMove");
425+
static_assert(!std::is_nothrow_move_assignable<zserio::InplaceOptionalHolder<ThrowMoveCtor>>::value,
426+
"!IsNothrowMove");
427+
}
428+
403429
} // namespace zserio

0 commit comments

Comments
 (0)