Skip to content

Commit 963b232

Browse files
yuanxingyangyuanxingyang
committed
iox-#2414 Reset index after move construction
1 parent f33d582 commit 963b232

File tree

4 files changed

+59
-14
lines changed

4 files changed

+59
-14
lines changed

‎doc/website/release-notes/iceoryx-unreleased.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@
148148
- Depend on @ncurses when building with Bazel [#2372](https://github.com/eclipse-iceoryx/iceoryx/issues/2372)
149149
- Fix windows performance issue [#2377](https://github.com/eclipse-iceoryx/iceoryx/issues/2377)
150150
- Max Client and Server cannot be configured independently of Max Publisher and Subscriber [#2394](https://github.com/eclipse-iceoryx/iceoryx/issues/2394)
151+
- Fix issue where 'variant' index is not reset after move [#2414](https://github.com/eclipse-iceoryx/iceoryx/issues/2414)
151152

152153
**Refactoring:**
153154

‎iceoryx_hoofs/test/moduletests/test_vocabulary_expected.cpp‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -321,8 +321,7 @@ TEST_F(expected_test, CreateWithValueAndMoveCtorLeadsToMovedSource)
321321

322322
// NOLINTJUSTIFICATION we explicitly want to test the defined state of a moved expected
323323
// NOLINTBEGIN(bugprone-use-after-move,hicpp-invalid-access-moved,clang-analyzer-cplusplus.Move)
324-
ASSERT_TRUE(sutSource.has_value());
325-
EXPECT_TRUE(sutSource.value().m_moved);
324+
ASSERT_FALSE(sutSource.has_value());
326325
// NOLINTEND(bugprone-use-after-move,hicpp-invalid-access-moved,clang-analyzer-cplusplus.Move)
327326
ASSERT_TRUE(sutDestination.has_value());
328327
EXPECT_FALSE(sutDestination.value().m_moved);
@@ -340,8 +339,8 @@ TEST_F(expected_test, CreateWithErrorAndMoveCtorLeadsToMovedSource)
340339

341340
// NOLINTJUSTIFICATION we explicitly want to test the defined state of a moved expected
342341
// NOLINTBEGIN(bugprone-use-after-move,hicpp-invalid-access-moved,clang-analyzer-cplusplus.Move)
343-
ASSERT_TRUE(sutSource.has_error());
344-
EXPECT_TRUE(sutSource.error().m_moved);
342+
ASSERT_FALSE(sutSource.has_error());
343+
ASSERT_FALSE(sutSource.has_value());
345344
// NOLINTEND(bugprone-use-after-move,hicpp-invalid-access-moved,clang-analyzer-cplusplus.Move)
346345
ASSERT_TRUE(sutDestination.has_error());
347346
EXPECT_FALSE(sutDestination.error().m_moved);
@@ -359,8 +358,7 @@ TEST_F(expected_test, CreateWithValueAndMoveAssignmentLeadsToMovedSource)
359358

360359
// NOLINTJUSTIFICATION we explicitly want to test the defined state of a moved expected
361360
// NOLINTBEGIN(bugprone-use-after-move,hicpp-invalid-access-moved,clang-analyzer-cplusplus.Move)
362-
ASSERT_TRUE(sutSource.has_value());
363-
EXPECT_TRUE(sutSource.value().m_moved);
361+
ASSERT_FALSE(sutSource.has_value());
364362
// NOLINTEND(bugprone-use-after-move,hicpp-invalid-access-moved,clang-analyzer-cplusplus.Move)
365363
ASSERT_TRUE(sutDestination.has_value());
366364
EXPECT_FALSE(sutDestination.value().m_moved);
@@ -378,8 +376,8 @@ TEST_F(expected_test, CreateWithErrorAndMoveAssignmentLeadsToMovedSource)
378376

379377
// NOLINTJUSTIFICATION we explicitly want to test the defined state of a moved expected
380378
// NOLINTBEGIN(bugprone-use-after-move,hicpp-invalid-access-moved,clang-analyzer-cplusplus.Move)
381-
ASSERT_TRUE(sutSource.has_error());
382-
EXPECT_TRUE(sutSource.error().m_moved);
379+
ASSERT_FALSE(sutSource.has_error());
380+
ASSERT_FALSE(sutSource.has_value());
383381
// NOLINTEND(bugprone-use-after-move,hicpp-invalid-access-moved,clang-analyzer-cplusplus.Move)
384382
ASSERT_TRUE(sutDestination.has_error());
385383
EXPECT_FALSE(sutDestination.error().m_moved);

‎iceoryx_hoofs/test/moduletests/test_vocabulary_variant.cpp‎

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ TEST_F(variant_Test, MoveCTorWithValueLeadsToSameValue)
341341
EXPECT_THAT(*ignatz.get<int>(), Eq(123));
342342
// NOLINTJUSTIFICATION check if move is invalidating the object
343343
// NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved,clang-analyzer-cplusplus.Move)
344-
EXPECT_THAT(schlomo.index(), Eq(0U));
344+
EXPECT_THAT(schlomo.index(), Eq(iox::INVALID_VARIANT_INDEX));
345345
}
346346

347347
TEST_F(variant_Test, MoveCTorWithoutValueResultsInInvalidVariant)
@@ -352,6 +352,48 @@ TEST_F(variant_Test, MoveCTorWithoutValueResultsInInvalidVariant)
352352
ASSERT_THAT(ignatz.index(), Eq(iox::INVALID_VARIANT_INDEX));
353353
}
354354

355+
TEST_F(variant_Test, MoveCTorWithVariantLeadToSameValue)
356+
{
357+
::testing::Test::RecordProperty("TEST_ID", "dc2a2aff-1fcd-4679-9bfc-b2fb4d2ae928");
358+
iox::variant<int, float, ComplexClass> schlomo;
359+
schlomo = ComplexClass(2, 3.14F);
360+
iox::variant<int, float, ComplexClass> ignatz(std::move(schlomo));
361+
// NOLINTJUSTIFICATION check if move is invalidating the object
362+
// NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved,clang-analyzer-cplusplus.Move)
363+
ASSERT_THAT(schlomo.index(), Eq(iox::INVALID_VARIANT_INDEX));
364+
EXPECT_THAT(ignatz.get<ComplexClass>()->a, Eq(2));
365+
EXPECT_THAT(ignatz.get<ComplexClass>()->b, Eq(3.14F));
366+
}
367+
368+
TEST_F(variant_Test, MoveAssignmentWithDifferentTypeVariantLeadsToSameValue)
369+
{
370+
::testing::Test::RecordProperty("TEST_ID", "562a38c3-aac2-4b1f-be55-c2d1b49e6c53");
371+
iox::variant<int, float, ComplexClass> schlomo;
372+
schlomo = ComplexClass(2, 3.14F);
373+
iox::variant<int, float, ComplexClass> ignatz(2.14F);
374+
ignatz = std::move(schlomo);
375+
// NOLINTJUSTIFICATION check if move is invalidating the object
376+
// NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved,clang-analyzer-cplusplus.Move)
377+
ASSERT_THAT(schlomo.index(), Eq(iox::INVALID_VARIANT_INDEX));
378+
EXPECT_THAT(ignatz.get<ComplexClass>()->a, Eq(2));
379+
EXPECT_THAT(ignatz.get<ComplexClass>()->b, Eq(3.14F));
380+
}
381+
382+
TEST_F(variant_Test, MoveAssignmentWithSameTypeVariantLeadsToSameValue)
383+
{
384+
::testing::Test::RecordProperty("TEST_ID", "e4a530af-05c0-49e5-ae04-f3512f299fbe");
385+
iox::variant<int, float, ComplexClass> schlomo;
386+
schlomo = ComplexClass(2, 3.14F);
387+
iox::variant<int, float, ComplexClass> ignatz;
388+
ignatz = ComplexClass(3, 4.14F);
389+
ignatz = std::move(schlomo);
390+
// NOLINTJUSTIFICATION check if move is invalidating the object
391+
// NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved,clang-analyzer-cplusplus.Move)
392+
ASSERT_THAT(schlomo.index(), Eq(iox::INVALID_VARIANT_INDEX));
393+
EXPECT_THAT(ignatz.get<ComplexClass>()->a, Eq(2));
394+
EXPECT_THAT(ignatz.get<ComplexClass>()->b, Eq(3.14F));
395+
}
396+
355397
TEST_F(variant_Test, MoveAssignmentWithValueLeadsToSameValue)
356398
{
357399
::testing::Test::RecordProperty("TEST_ID", "ee36df28-545f-42bc-9ef6-3699284f1a42");
@@ -429,12 +471,12 @@ TEST_F(variant_Test, CreatingSecondObjectViaMoveCTorResultsInTwoDTorCalls)
429471
EXPECT_THAT(DTorTest::dtorWasCalled, Eq(false));
430472
// NOLINTJUSTIFICATION check if move is invalidating the object
431473
// NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved,clang-analyzer-cplusplus.Move)
432-
EXPECT_THAT(ignatz.index(), Eq(1U));
474+
EXPECT_THAT(ignatz.index(), Eq(iox::INVALID_VARIANT_INDEX));
433475
}
434476
EXPECT_THAT(DTorTest::dtorWasCalled, Eq(true));
435477
DTorTest::dtorWasCalled = false;
436478
}
437-
EXPECT_THAT(DTorTest::dtorWasCalled, Eq(true));
479+
EXPECT_THAT(DTorTest::dtorWasCalled, Eq(false));
438480
}
439481

440482
TEST_F(variant_Test, CreatingSecondObjectViaMoveAssignmentResultsInTwoDTorCalls)
@@ -450,13 +492,14 @@ TEST_F(variant_Test, CreatingSecondObjectViaMoveAssignmentResultsInTwoDTorCalls)
450492
schlomo = std::move(ignatz);
451493
// NOLINTJUSTIFICATION check if move is invalidating the object
452494
// NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved,clang-analyzer-cplusplus.Move)
453-
EXPECT_THAT(ignatz.index(), Eq(1U));
454-
EXPECT_THAT(DTorTest::dtorWasCalled, Eq(false));
495+
EXPECT_THAT(ignatz.index(), Eq(iox::INVALID_VARIANT_INDEX));
496+
DTorTest::dtorWasCalled = false;
455497
}
498+
// schlomo is destroyed when it goes out of scope
456499
EXPECT_THAT(DTorTest::dtorWasCalled, Eq(true));
457500
DTorTest::dtorWasCalled = false;
458501
}
459-
EXPECT_THAT(DTorTest::dtorWasCalled, Eq(true));
502+
EXPECT_THAT(DTorTest::dtorWasCalled, Eq(false));
460503
}
461504

462505
TEST_F(variant_Test, DirectValueAssignmentResultsInCorrectIndex)

‎iceoryx_hoofs/vocabulary/include/iox/detail/variant.inl‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ inline constexpr variant<Types...>::variant(variant&& rhs) noexcept
9797
if (m_type_index != INVALID_VARIANT_INDEX)
9898
{
9999
internal::call_at_index<0, Types...>::moveConstructor(m_type_index, &rhs.m_storage, &m_storage);
100+
rhs.m_type_index = INVALID_VARIANT_INDEX;
100101
}
101102
}
102103

@@ -114,13 +115,15 @@ inline constexpr variant<Types...>& variant<Types...>::operator=(variant&& rhs)
114115
if (m_type_index != INVALID_VARIANT_INDEX)
115116
{
116117
internal::call_at_index<0, Types...>::moveConstructor(m_type_index, &rhs.m_storage, &m_storage);
118+
rhs.m_type_index = INVALID_VARIANT_INDEX;
117119
}
118120
}
119121
else
120122
{
121123
if (m_type_index != INVALID_VARIANT_INDEX)
122124
{
123125
internal::call_at_index<0, Types...>::move(m_type_index, &rhs.m_storage, &m_storage);
126+
rhs.m_type_index = INVALID_VARIANT_INDEX;
124127
}
125128
}
126129
}

0 commit comments

Comments
 (0)