From 60008d594769fe9562f4804650dbd4f0b4d13873 Mon Sep 17 00:00:00 2001 From: Xingyang Yuan Date: Thu, 16 Jan 2025 19:59:35 +0800 Subject: [PATCH] iox-#2414 Use placement new to construct a new object during assignment --- doc/website/release-notes/iceoryx-unreleased.md | 1 + .../test/moduletests/test_vocabulary_variant.cpp | 6 +++--- .../vocabulary/include/iox/detail/variant.inl | 16 ++++++---------- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/doc/website/release-notes/iceoryx-unreleased.md b/doc/website/release-notes/iceoryx-unreleased.md index 13a3b11c48..4ac9cd9aa5 100644 --- a/doc/website/release-notes/iceoryx-unreleased.md +++ b/doc/website/release-notes/iceoryx-unreleased.md @@ -148,6 +148,7 @@ - Depend on @ncurses when building with Bazel [#2372](https://github.com/eclipse-iceoryx/iceoryx/issues/2372) - Fix windows performance issue [#2377](https://github.com/eclipse-iceoryx/iceoryx/issues/2377) - Max Client and Server cannot be configured independently of Max Publisher and Subscriber [#2394](https://github.com/eclipse-iceoryx/iceoryx/issues/2394) +- Use placement new to construct a new object during assignment [#2414](https://github.com/eclipse-iceoryx/iceoryx/issues/2414) **Refactoring:** diff --git a/iceoryx_hoofs/test/moduletests/test_vocabulary_variant.cpp b/iceoryx_hoofs/test/moduletests/test_vocabulary_variant.cpp index 7746ab4ba9..37351dd16b 100644 --- a/iceoryx_hoofs/test/moduletests/test_vocabulary_variant.cpp +++ b/iceoryx_hoofs/test/moduletests/test_vocabulary_variant.cpp @@ -467,13 +467,13 @@ TEST_F(variant_Test, DirectValueAssignmentResultsInCorrectIndex) EXPECT_THAT(schlomo.index(), Eq(0U)); } -TEST_F(variant_Test, DirectValueAssignmentWhenAlreadyAssignedWithDifferentType) +TEST_F(variant_Test, DirectValueAssignmentWhenAlreadyAssignedWithDifferentTypeResultsInCorrectIndex) { - ::testing::Test::RecordProperty("TEST_ID", "a058c173-497b-43ec-ba03-2702f3ba8190"); + ::testing::Test::RecordProperty("TEST_ID", "f9f10061-b7f2-4e8f-a4b2-c04478046dcd"); iox::variant schlomo; schlomo = 123; schlomo = 123.01F; - EXPECT_THAT(schlomo.index(), Eq(0U)); + EXPECT_THAT(schlomo.index(), Eq(1U)); } TEST_F(variant_Test, HoldsAlternativeForCorrectType) diff --git a/iceoryx_hoofs/vocabulary/include/iox/detail/variant.inl b/iceoryx_hoofs/vocabulary/include/iox/detail/variant.inl index 537dd48a0a..4c3c4b2c14 100644 --- a/iceoryx_hoofs/vocabulary/include/iox/detail/variant.inl +++ b/iceoryx_hoofs/vocabulary/include/iox/detail/variant.inl @@ -149,25 +149,21 @@ template inline typename std::enable_if&>::value, variant>::type& variant::operator=(T&& rhs) noexcept { - if (m_type_index == INVALID_VARIANT_INDEX) + if (m_type_index != internal::get_index_of_type<0, T, Types...>::index) { + call_element_destructor(); + // NOLINTJUSTIFICATION clang-tidy suggests that std::move(rhs) might erroneously move an lvalue, but here rhs is already T&&, so it's safe. + // NOLINTNEXTLINE(bugprone-move-forwarding-reference) + new (&m_storage) T(std::move(rhs)); m_type_index = internal::get_index_of_type<0, T, Types...>::index; } - - if (!has_bad_variant_element_access()) + else { // AXIVION Next Construct AutosarC++19_03-M5.2.8: conversion to typed pointer is intentional, it is correctly aligned and points to sufficient memory for a T by design // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) auto storage = reinterpret_cast(&m_storage); *storage = std::forward(rhs); } - else - { - error_message(__PRETTY_FUNCTION__, - "wrong variant type assignment, another type is already " - "set in variant"); - } - return *this; }