From 6b224d62c730a94c63b7b7560006a3f41f88c22f Mon Sep 17 00:00:00 2001 From: Brendan Emery Date: Tue, 16 Jun 2026 19:23:04 +0200 Subject: [PATCH 1/8] mw/com: Add ReferenceToMoveableOwner and ReferenceToMoveable In various places in our codebase, we have the issue in which we hand out a reference to a class which is moveable. If the class is moved, then we need to update this reference. E.g. SkeletonBase stores a map of references to its service elements. Each service element therefore must store a reference to the SkeletonBase and update the reference to itself when it's moved. The SkeletonBase must also update the reference to itself within each service element when it is moved. To avoid such complexity, this commit introduces a class ReferenceToMoveable which can be given to the SkeletonBase which is stored in the heap so a reference to it will never be invalidated. This then stores a reference to the service element which is updated by the service element if it's moved. With this approach, the service element doesn't need to store a reference to the parent SkeletonBase so the parent SkeletonBase doesn't need to inform the service element when it moves. --- score/mw/com/impl/BUILD | 23 ++++ score/mw/com/impl/reference_to_moveable.cpp | 13 ++ score/mw/com/impl/reference_to_moveable.h | 114 ++++++++++++++++ .../com/impl/reference_to_moveable_test.cpp | 127 ++++++++++++++++++ 4 files changed, 277 insertions(+) create mode 100644 score/mw/com/impl/reference_to_moveable.cpp create mode 100644 score/mw/com/impl/reference_to_moveable.h create mode 100644 score/mw/com/impl/reference_to_moveable_test.cpp diff --git a/score/mw/com/impl/BUILD b/score/mw/com/impl/BUILD index 245c319cb..6579d66e8 100644 --- a/score/mw/com/impl/BUILD +++ b/score/mw/com/impl/BUILD @@ -1064,6 +1064,18 @@ cc_library( ], ) +cc_library( + name = "reference_to_moveable", + srcs = [ + "reference_to_moveable.cpp", + ], + hdrs = [ + "reference_to_moveable.h", + ], + features = COMPILER_WARNING_FEATURES, + tags = ["FFI"], +) + cc_unit_test( name = "unit_test_runtime_single_exec", srcs = ["runtime_single_exec_test.cpp"], @@ -1329,6 +1341,17 @@ cc_unit_test( ], ) +cc_unit_test( + name = "reference_to_moveable_test", + srcs = [ + "reference_to_moveable_test.cpp", + ], + features = COMPILER_WARNING_FEATURES, + deps = [ + ":reference_to_moveable", + ], +) + cc_unit_test( name = "unit_test", srcs = [ diff --git a/score/mw/com/impl/reference_to_moveable.cpp b/score/mw/com/impl/reference_to_moveable.cpp new file mode 100644 index 000000000..ac1578a60 --- /dev/null +++ b/score/mw/com/impl/reference_to_moveable.cpp @@ -0,0 +1,13 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#include "score/mw/com/impl/reference_to_moveable.h" diff --git a/score/mw/com/impl/reference_to_moveable.h b/score/mw/com/impl/reference_to_moveable.h new file mode 100644 index 000000000..b9d7e50d9 --- /dev/null +++ b/score/mw/com/impl/reference_to_moveable.h @@ -0,0 +1,114 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#ifndef SCORE_MW_COM_IMPL_REFERENCE_TO_MOVEABLE_H +#define SCORE_MW_COM_IMPL_REFERENCE_TO_MOVEABLE_H + +#include + +namespace score::mw::com::impl +{ + +/// \brief Allows moveable classes to hand out references to themselves while allowing these references to be updated +/// when the moveable class T is moved. +/// +/// Moveable class, T, will store a ReferenceToMoveable and hand out references to a +/// ReferenceToMoveable::Reference (instead of pointers to itself) via Get. The ReferenceToMoveable::Reference is +/// stored on the heap and will never be moved. The class T must manually update the reference to itself via Update in +/// this class when moved. +/// +/// See the test containing DummyClass for an example of how to use this class together with +/// ReferenceToMoveable::Reference. +/// +/// Reason for this class: In various places in our codebase, we have the issue in which we hand out a reference to a +/// class which is moveable. If the class is moved, then we need to update this reference. E.g. SkeletonBase stores a +/// map of references to its service elements. Each service element therefore must store a reference to the SkeletonBase +/// and update the reference to itself when it's moved. The SkeletonBase must also update the reference to itself within +/// each service element when it is moved. +/// +/// To avoid such complexity, this commit introduces a class ReferenceToMoveable::Reference which can be +/// given to the SkeletonBase which is stored in the heap so a reference to it will never be invalidated. This then +/// stores a reference to the service element which is updated by the service element if it's moved. With this approach, +/// the service element doesn't need to store a reference to the parent SkeletonBase so the parent SkeletonBase doesn't +/// need to inform the service element when it moves. +/// +/// NOTE: This class is not thread safe. It is the responsibility of the user to ensure that the reference is updated +/// before it is accessed after a move. In our current architecture, this is not an issue as we only move our service +/// elements when the user Skeleton / Proxy object is moved which owns the service elements. So the user cannot be +/// accessing the service elements while they are being moved. +template +class ReferenceToMoveable +{ + public: + class Reference + { + friend class ReferenceToMoveable; + + struct PrivateConstructorTag + { + }; + + public: + Reference(PrivateConstructorTag, T& reference_object) : reference_object_{reference_object} {} + + Reference(const Reference&) = delete; + Reference& operator=(const Reference&) = delete; + Reference(Reference&&) = delete; + Reference& operator=(Reference&&) = delete; + ~Reference() = default; + + const T& Get() const + { + return reference_object_.get(); + } + + T& Get() + { + return reference_object_.get(); + } + + private: + void Update(T& field) + { + reference_object_ = field; + } + + std::reference_wrapper reference_object_; + }; + + explicit ReferenceToMoveable(T& field) + : reference_to_moveable_{std::make_unique(typename Reference::PrivateConstructorTag{}, field)} + { + } + + Reference& Get() + { + return *(reference_to_moveable_); + } + + const Reference& Get() const + { + return *(reference_to_moveable_); + } + + void Update(T& field) + { + reference_to_moveable_->Update(field); + } + + private: + std::unique_ptr reference_to_moveable_; +}; + +} // namespace score::mw::com::impl + +#endif // SCORE_MW_COM_IMPL_REFERENCE_TO_MOVEABLE_H diff --git a/score/mw/com/impl/reference_to_moveable_test.cpp b/score/mw/com/impl/reference_to_moveable_test.cpp new file mode 100644 index 000000000..0d7be1926 --- /dev/null +++ b/score/mw/com/impl/reference_to_moveable_test.cpp @@ -0,0 +1,127 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#include "score/mw/com/impl/reference_to_moveable.h" + +#include + +namespace score::mw::com::impl +{ + +namespace +{ + +TEST(ReferenceToMoveableTest, NotCopyableOrMoveable) +{ + static_assert(!std::is_copy_constructible_v::Reference>, "Is wrongly copyable"); + static_assert(!std::is_copy_assignable_v::Reference>, "Is wrongly copyable"); + static_assert(!std::is_move_constructible_v::Reference>, "Is wrongly move constructible"); + static_assert(!std::is_move_assignable_v::Reference>, "Is wrongly move assignable"); +} + +TEST(ReferenceToMoveableTest, NotCopyable) +{ + static_assert(!std::is_copy_constructible_v>, "Is wrongly copyable"); + static_assert(!std::is_copy_assignable_v>, "Is wrongly copyable"); +} + +TEST(ReferenceToMoveableTest, IsMoveable) +{ + static_assert(std::is_move_constructible_v>, "Is not move constructible"); + static_assert(std::is_move_assignable_v>, "Is not move assignable"); +} + +TEST(ReferenceToMoveableTest, GetReturnsReferenceToObject) +{ + // Given a ReferenceToMoveable pointing to an int + const int value = 42; + ReferenceToMoveable wrapper{value}; + + // When Get is called + auto& reference_to_moveable = wrapper.Get(); + + // Then the reference points to the original int + EXPECT_EQ(reference_to_moveable.Get(), value); + EXPECT_EQ(&reference_to_moveable.Get(), &value); +} + +TEST(ReferenceToMoveableTest, GetReturnsReferenceToUpdatedObject) +{ + // Given a ReferenceToMoveable pointing to an int + const int value = 42; + ReferenceToMoveable wrapper{value}; + + // When calling Update with a new int + const int new_value = 100; + wrapper.Update(new_value); + + // Then Get returns a reference to the ned int + auto& reference_to_moveable = wrapper.Get(); + EXPECT_EQ(reference_to_moveable.Get(), new_value); + EXPECT_EQ(&reference_to_moveable.Get(), &new_value); +} + +// Note. This class and the test below is added as a demonstration of how ReferenceToMoveable and +// ReferenceToMoveable::Reference can be used together to allow moveable classes to hand out references to themselves +// while allowing these references to be updated when the moveable class is moved. +class DummyClass +{ + public: + DummyClass() : ptr_wrapper_{*this} {} + + ~DummyClass() = default; + + DummyClass(const DummyClass&) = delete; + DummyClass& operator=(const DummyClass&) & = delete; + + DummyClass(DummyClass&& other) noexcept : ptr_wrapper_{std::move(other.ptr_wrapper_)} + { + ptr_wrapper_.Update(*this); + } + DummyClass& operator=(DummyClass&& other) & noexcept + { + if (this != &other) + { + ptr_wrapper_ = std::move(other.ptr_wrapper_); + ptr_wrapper_.Update(*this); + } + return *this; + } + + ReferenceToMoveable::Reference& Get() + { + return ptr_wrapper_.Get(); + } + + private: + ReferenceToMoveable ptr_wrapper_; +}; + +TEST(ReferenceToMoveableDummyClassTest, GetReturnsReferenceToUpdatedObjectWhenMoveConstructingDummyClass) +{ + // Given an DummyClass with a ReferenceToMoveable pointing to itself + DummyClass owning_class{}; + + // and given that we want to store a reference to DummyClass which should be updated when DummyClass is moved + auto& owning_class_reference = owning_class.Get(); + + // When the DummyClass is move constructed + DummyClass moved_owning_class{std::move(owning_class)}; + + // Then the ReferenceToMoveable retrieved from the original ReferenceToMoveable::Reference should point to the + // moved-to DummyClass + auto& reference_wrapper = owning_class_reference.Get(); + EXPECT_EQ(&reference_wrapper, &moved_owning_class); +} + +} // namespace +} // namespace score::mw::com::impl From a7c63699a3455dc8177164389884dc370e0eb5c3 Mon Sep 17 00:00:00 2001 From: Brendan Emery Date: Fri, 19 Jun 2026 08:46:24 +0200 Subject: [PATCH 2/8] mw/com: Remove updating of service element in SkeletonBase Previously, each service element had to store a reference to the parent SkeletonBase so that it could update the reference to itself in the SkeletonBase when the service element was moved. This meant that both SkeletonBase and the service elements had to store references to each other. Keeping these references in a valid state during moving was complex and error prone. We now use ReferenceToMoveableOwner in each service element which is updated when the service element moves. The SkeletonBase stores a ReferenceToMoveable to each service element, instead of a regular reference which is now always valid, regardless of moving of the SkeletonBase or service elements. --- .../structural_event_field_extensions.puml | 1 - .../skeleton_binding_model.puml | 8 +- .../skeleton_proxy_binding_model.puml | 8 +- score/mw/com/impl/BUILD | 5 + score/mw/com/impl/generic_skeleton_event.cpp | 2 +- score/mw/com/impl/methods/BUILD | 1 + score/mw/com/impl/methods/skeleton_method.h | 39 +----- .../com/impl/methods/skeleton_method_base.cpp | 8 -- .../com/impl/methods/skeleton_method_base.h | 40 +++++- .../methods/skeleton_method_base_test.cpp | 56 -------- .../skeleton_wrapper_class_test_view.h | 4 +- score/mw/com/impl/skeleton_base.cpp | 132 ++++-------------- score/mw/com/impl/skeleton_base.h | 29 ++-- score/mw/com/impl/skeleton_base_test.cpp | 96 +++++++------ score/mw/com/impl/skeleton_event.h | 41 +----- score/mw/com/impl/skeleton_event_base.h | 53 ++++--- score/mw/com/impl/skeleton_event_test.cpp | 6 +- score/mw/com/impl/skeleton_field.h | 64 +-------- score/mw/com/impl/skeleton_field_base.h | 50 ++++--- .../mw/com/impl/skeleton_field_base_test.cpp | 4 - score/mw/com/impl/skeleton_field_test.cpp | 32 +---- score/mw/com/impl/tracing/BUILD | 1 + .../mw/com/impl/tracing/skeleton_tracing.cpp | 25 ++-- score/mw/com/impl/tracing/skeleton_tracing.h | 10 +- .../impl/tracing/skeleton_tracing_test.cpp | 13 +- 25 files changed, 253 insertions(+), 475 deletions(-) diff --git a/score/mw/com/design/ipc_tracing/structural_event_field_extensions.puml b/score/mw/com/design/ipc_tracing/structural_event_field_extensions.puml index a1e164658..321121be3 100644 --- a/score/mw/com/design/ipc_tracing/structural_event_field_extensions.puml +++ b/score/mw/com/design/ipc_tracing/structural_event_field_extensions.puml @@ -27,7 +27,6 @@ class "score::mw::com::impl::SkeletonEventBase" { +SkeletonEventBase(std::unique_ptr binding) +PrepareOffer(): score::Result +PrepareStopOffer(): void - +UpdateSkeletonReference(SkeletonBase& base_skeleton): void -binding_: std::unique_ptr -tracing_data_: SkeletonEventTracing } diff --git a/score/mw/com/design/skeleton_proxy/skeleton_binding_model.puml b/score/mw/com/design/skeleton_proxy/skeleton_binding_model.puml index 6db774ecc..430bc06b1 100644 --- a/score/mw/com/design/skeleton_proxy/skeleton_binding_model.puml +++ b/score/mw/com/design/skeleton_proxy/skeleton_binding_model.puml @@ -40,8 +40,7 @@ abstract class "score::mw::com::impl::SkeletonBase" as SkeletonBase { +StopOfferService(): void -- Notes: - SkeletonBase is moveable but not copyable. On moving, the SkeletonBase should call - UpdateSkeletonReference on all events and fields within its events_ and fields_ maps. + SkeletonBase is moveable but not copyable. On moving, the SkeletonBase updates method references. } class SkeletonBinding { @@ -91,7 +90,6 @@ abstract class "score::mw::com::impl::SkeletonFieldBase" as SkeletonFieldBase { +SkeletonFieldBase(SkeletonBase&, std::string_view field_name, std::unique_ptr) +PrepareOffer(): Result +PrepareStopOffer(): void - +UpdateSkeletonReference(SkeletonBase& base_skeleton): void -IsInitialValueSaved(): bool = 0 -DoDeferredUpdate(): Result = 0 @@ -108,7 +106,6 @@ class "score::mw::com::impl::SkeletonEventBase" as SkeletonEventBase { +SkeletonEventBase(std::unique_ptr binding) +PrepareOffer(): score::Result +PrepareStopOffer(): void - +UpdateSkeletonReference(SkeletonBase& base_skeleton): void -- Notes: @@ -118,8 +115,7 @@ class "score::mw::com::impl::SkeletonEventBase" as SkeletonEventBase { This allows SkeletonEventBase to access the type independent methods of SkeletonEventBindingBase. Derived classes i.e. impl::SkeletonEvent should downcast binding_ to a SkeletonEventBinding in order to use type - dependent methods. On moving, the SkeletonEvent should call UpdateEvent on its - parent skeleton so that the parent stores its new address. + dependent methods. } class SkeletonServiceMethodBinding { diff --git a/score/mw/com/design/skeleton_proxy/skeleton_proxy_binding_model.puml b/score/mw/com/design/skeleton_proxy/skeleton_proxy_binding_model.puml index 91ebcc955..323e966eb 100644 --- a/score/mw/com/design/skeleton_proxy/skeleton_proxy_binding_model.puml +++ b/score/mw/com/design/skeleton_proxy/skeleton_proxy_binding_model.puml @@ -55,8 +55,7 @@ abstract class "score::mw::com::impl::SkeletonBase" as ScoreMwComImplSkeletonBas +StopOfferService(): void -- Notes: - SkeletonBase is moveable but not copyable. On moving, the SkeletonBase should call UpdateSkeletonReference on all - events and fields within its events_ and fields_ maps. + SkeletonBase is moveable but not copyable. On moving, the SkeletonBase updates method references. } abstract class "DummySkeleton\n{{generated}}" as DummySkeleton { @@ -75,7 +74,6 @@ class "score::mw::com::impl::SkeletonEventBase" as ScoreMwComImplSkeletonEventBa +SkeletonEventBase(std::unique_ptr binding) +PrepareOffer(): score::Result +PrepareStopOffer(): void - +UpdateSkeletonReference(SkeletonBase& base_skeleton): void -- Notes: SkeletonEventBase is moveable but not copyable. @@ -84,15 +82,13 @@ class "score::mw::com::impl::SkeletonEventBase" as ScoreMwComImplSkeletonEventBa This allows SkeletonEventBase to access the type independent methods of SkeletonEventBindingBase. Derived classes i.e. impl::SkeletonEvent should downcast binding_ to a SkeletonEventBinding in order to use type - dependent methods. On moving, the SkeletonEvent should call UpdateEvent on its - parent skeleton so that the parent stores its new address. + dependent methods. } class "score::mw::com::impl::SkeletonFieldBase" as ScoreMwComImplSkeletonFieldBase { +SkeletonFieldBase(SkeletonBase&, std::string_view field_name, std::unique_ptr) +PrepareOffer(): Result +PrepareStopOffer(): void - +UpdateSkeletonReference(SkeletonBase& base_skeleton): void -- #skeleton_event_dispatch_ : std::unique_ptr #skeleton_base_ : std::reference_wrapper diff --git a/score/mw/com/impl/BUILD b/score/mw/com/impl/BUILD index 6579d66e8..7871d6f7c 100644 --- a/score/mw/com/impl/BUILD +++ b/score/mw/com/impl/BUILD @@ -192,6 +192,7 @@ cc_library( deps = [ ":data_type_meta_info", ":generic_skeleton_event_binding", + ":reference_to_moveable", ":skeleton_base", ":skeleton_event_base", ":skeleton_event_binding", @@ -243,6 +244,7 @@ cc_library( ], deps = [ ":instance_identifier", + ":reference_to_moveable", ":runtime", ":skeleton_base", ":skeleton_event_base", @@ -355,6 +357,7 @@ cc_library( ], deps = [ ":flag_owner", + ":reference_to_moveable", ":skeleton_event_binding", "//score/mw/com/impl/tracing:skeleton_event_tracing", "//score/mw/com/impl/tracing:skeleton_event_tracing_data", @@ -374,6 +377,7 @@ cc_library( ], deps = [ ":error", + ":reference_to_moveable", ":skeleton_event_base", "//score/mw/com/impl/methods:skeleton_method_base", "@score_baselibs//score/language/futurecpp", @@ -396,6 +400,7 @@ cc_library( deps = [ ":flag_owner", ":instance_identifier", + ":reference_to_moveable", ":runtime", ":skeleton_binding", ":skeleton_event_base", diff --git a/score/mw/com/impl/generic_skeleton_event.cpp b/score/mw/com/impl/generic_skeleton_event.cpp index 8ca84b28a..1caa29c9b 100644 --- a/score/mw/com/impl/generic_skeleton_event.cpp +++ b/score/mw/com/impl/generic_skeleton_event.cpp @@ -26,7 +26,7 @@ GenericSkeletonEvent::GenericSkeletonEvent(SkeletonBase& skeleton_base, std::unique_ptr binding) : SkeletonEventBase(skeleton_base, event_name, std::move(binding)) { - SkeletonBaseView{skeleton_base}.RegisterEvent(event_name, *this); + SkeletonBaseView{skeleton_base}.RegisterEvent(event_name, reference_to_moveable_.Get()); if (binding_ != nullptr) { diff --git a/score/mw/com/impl/methods/BUILD b/score/mw/com/impl/methods/BUILD index 0c9ba0b8e..241bc0613 100644 --- a/score/mw/com/impl/methods/BUILD +++ b/score/mw/com/impl/methods/BUILD @@ -140,6 +140,7 @@ cc_library( deps = [ ":skeleton_method_binding", "//score/mw/com/impl:method_type", + "//score/mw/com/impl:reference_to_moveable", "//score/mw/com/impl/util:type_erased_storage", ], ) diff --git a/score/mw/com/impl/methods/skeleton_method.h b/score/mw/com/impl/methods/skeleton_method.h index ee2210947..ab12761c7 100644 --- a/score/mw/com/impl/methods/skeleton_method.h +++ b/score/mw/com/impl/methods/skeleton_method.h @@ -21,7 +21,6 @@ #include "score/result/result.h" #include -#include #include #include #include @@ -102,16 +101,14 @@ class SkeletonMethod final : public SkeletonMethodBase SkeletonMethod(const SkeletonMethod&) = delete; SkeletonMethod& operator=(const SkeletonMethod&) & = delete; - SkeletonMethod(SkeletonMethod&& other) noexcept; - SkeletonMethod& operator=(SkeletonMethod&& other) & noexcept; + SkeletonMethod(SkeletonMethod&& other) noexcept = default; + SkeletonMethod& operator=(SkeletonMethod&& other) & noexcept = default; /// \brief Register a handler with the binding, which will be executed by the binding when the Proxy calls this /// method. /// \return score::cpp::blank on success and ComErrc code specified by the binding on failiure template Result RegisterHandler(Callable&& callback); - - void UpdateSkeletonReference(SkeletonBase& skeleton_base) noexcept; }; template @@ -121,37 +118,7 @@ SkeletonMethod::SkeletonMethod(SkeletonBase& skeleton_b ::score::mw::com::impl::MethodType method_type) : SkeletonMethodBase(skeleton_base, method_name, std::move(skeleton_method_binding), method_type) { - auto skeleton_base_view = SkeletonBaseView{skeleton_base}; - skeleton_base_view.RegisterMethod(method_name_, *this); -} - -template -SkeletonMethod::SkeletonMethod(SkeletonMethod&& other) noexcept - : SkeletonMethodBase(std::move(other)) -{ - - SkeletonBaseView skeleton_base_view{skeleton_base_.get()}; - skeleton_base_view.UpdateMethod(method_name_, *this); -} - -template -SkeletonMethod& SkeletonMethod::operator=( - SkeletonMethod&& other) & noexcept -{ - if (this != &other) - { - SkeletonMethodBase::operator=(std::move(other)); - - SkeletonBaseView skeleton_base_view{skeleton_base_.get()}; - skeleton_base_view.UpdateMethod(method_name_, *this); - } - return *this; -} - -template -void SkeletonMethod::UpdateSkeletonReference(SkeletonBase& skeleton_base) noexcept -{ - skeleton_base_ = skeleton_base; + SkeletonBaseView{skeleton_base}.RegisterMethod(method_name_, reference_to_moveable_.Get()); } template diff --git a/score/mw/com/impl/methods/skeleton_method_base.cpp b/score/mw/com/impl/methods/skeleton_method_base.cpp index a0ee7fdf0..e0c182f51 100644 --- a/score/mw/com/impl/methods/skeleton_method_base.cpp +++ b/score/mw/com/impl/methods/skeleton_method_base.cpp @@ -11,11 +11,3 @@ * SPDX-License-Identifier: Apache-2.0 ********************************************************************************/ #include "score/mw/com/impl/methods/skeleton_method_base.h" - -namespace score::mw::com::impl -{ -void SkeletonMethodBase::UpdateSkeletonReference(SkeletonBase& skeleton_base) noexcept -{ - skeleton_base_ = skeleton_base; -} -} // namespace score::mw::com::impl diff --git a/score/mw/com/impl/methods/skeleton_method_base.h b/score/mw/com/impl/methods/skeleton_method_base.h index d72ed1272..69e659680 100644 --- a/score/mw/com/impl/methods/skeleton_method_base.h +++ b/score/mw/com/impl/methods/skeleton_method_base.h @@ -15,8 +15,8 @@ #include "score/mw/com/impl/method_type.h" #include "score/mw/com/impl/methods/skeleton_method_binding.h" +#include "score/mw/com/impl/reference_to_moveable.h" -#include #include namespace score::mw::com::impl @@ -36,14 +36,14 @@ class SkeletonMethodBase friend SkeletonMethodBaseView; public: - SkeletonMethodBase(SkeletonBase& skeleton_base, + SkeletonMethodBase(SkeletonBase&, const std::string_view method_name, std::unique_ptr skeleton_method_binding, MethodType method_type = MethodType::kMethod) : method_name_{method_name}, method_type_{method_type}, binding_{std::move(skeleton_method_binding)}, - skeleton_base_{skeleton_base} + reference_to_moveable_{*this} { } @@ -52,16 +52,42 @@ class SkeletonMethodBase SkeletonMethodBase(const SkeletonMethodBase&) = delete; SkeletonMethodBase& operator=(const SkeletonMethodBase&) & = delete; - SkeletonMethodBase(SkeletonMethodBase&&) noexcept = default; - SkeletonMethodBase& operator=(SkeletonMethodBase&&) & noexcept = default; + SkeletonMethodBase(SkeletonMethodBase&& other) noexcept + : method_name_{other.method_name_}, + method_type_{other.method_type_}, + binding_{std::move(other.binding_)}, + reference_to_moveable_{std::move(other.reference_to_moveable_)} + { + reference_to_moveable_.Update(*this); + } + + SkeletonMethodBase& operator=(SkeletonMethodBase&& other) & noexcept + { + if (this == &other) + { + return *this; + } - void UpdateSkeletonReference(SkeletonBase& skeleton_base) noexcept; + method_name_ = other.method_name_; + method_type_ = other.method_type_; + binding_ = std::move(other.binding_); + reference_to_moveable_ = std::move(other.reference_to_moveable_); + + reference_to_moveable_.Update(*this); + return *this; + } protected: std::string_view method_name_; MethodType method_type_; std::unique_ptr binding_; - std::reference_wrapper skeleton_base_; + + /// \brief Helper class for creating reference to this SkeletonMethodBase which is provided to SkeletonBase when + /// registering this event. + /// + /// Contains a heap allocated reference to this SkeletonMethodBase which is updated in the move constructor and move + /// assignment operator so that it's always valid, even after moving the SkeletonMethodBase. + ReferenceToMoveable reference_to_moveable_; }; class SkeletonMethodBaseView diff --git a/score/mw/com/impl/methods/skeleton_method_base_test.cpp b/score/mw/com/impl/methods/skeleton_method_base_test.cpp index 9674d717a..8afd1852a 100644 --- a/score/mw/com/impl/methods/skeleton_method_base_test.cpp +++ b/score/mw/com/impl/methods/skeleton_method_base_test.cpp @@ -12,54 +12,13 @@ ********************************************************************************/ #include "score/mw/com/impl/methods/skeleton_method_base.h" -#include "score/mw/com/impl/bindings/mock_binding/skeleton.h" -#include "score/mw/com/impl/bindings/mock_binding/skeleton_method.h" -#include "score/mw/com/impl/instance_identifier.h" -#include "score/mw/com/impl/methods/skeleton_method.h" -#include "score/mw/com/impl/skeleton_base.h" - -#include #include -#include -#include namespace score::mw::com::impl { namespace { -using ::testing::_; -using ::testing::Invoke; -using ::testing::Return; -using ::testing::StrictMock; - -SkeletonBase MakeEmptySkeleton(std::string_view service_name) -{ - const ServiceTypeDeployment empty_type_deployment{score::cpp::blank{}}; - const ServiceIdentifierType service{make_ServiceIdentifierType(std::string{service_name})}; - const auto instance_specifier = InstanceSpecifier::Create(std::string{"/dummy_instance_specifier"}).value(); - const ServiceInstanceDeployment empty_instance_deployment{ - service, score::cpp::blank{}, QualityType::kASIL_QM, instance_specifier}; - - return SkeletonBase(std::make_unique(), - make_InstanceIdentifier(empty_instance_deployment, empty_type_deployment)); -} - -const auto kMethodName{"DummyMethod1"}; - -auto kEmptySkeleton1 = MakeEmptySkeleton("bla"); -auto kEmptySkeleton2 = MakeEmptySkeleton("blabla"); - -class MyDummyMethod final : public SkeletonMethodBase -{ - public: - MyDummyMethod() : SkeletonMethodBase{kEmptySkeleton1, kMethodName, nullptr} {} - - std::reference_wrapper GetSkeletonReference() - { - return skeleton_base_; - } -}; TEST(SkeletonMethodBaseTests, NotCopyable) { @@ -73,21 +32,6 @@ TEST(SkeletonMethodBaseTests, IsMoveable) static_assert(std::is_move_assignable::value, "Is not move assignable"); } -TEST(SkeletonMethodBase, UpdateSkeletonReferenceUpdatesTheReference) -{ - // Given a constructed SkeletonMethod with a valid reference to a Skeleton base class - auto skeleton_method = std::make_unique(); - EXPECT_EQ(std::addressof(kEmptySkeleton1), std::addressof(skeleton_method->GetSkeletonReference().get())); - - // When UpdateSkeletonReference is called with a reference to a new Skeleton base class - skeleton_method->UpdateSkeletonReference(kEmptySkeleton2); - - // Then the reference in the skeleton method is updated correctly - EXPECT_EQ(std::addressof(kEmptySkeleton2), std::addressof(skeleton_method->GetSkeletonReference().get())); - - // and that the result is blank -} - } // namespace } // namespace score::mw::com::impl diff --git a/score/mw/com/impl/mocking/skeleton_wrapper_class_test_view.h b/score/mw/com/impl/mocking/skeleton_wrapper_class_test_view.h index 5699cd18e..48ccc0431 100644 --- a/score/mw/com/impl/mocking/skeleton_wrapper_class_test_view.h +++ b/score/mw/com/impl/mocking/skeleton_wrapper_class_test_view.h @@ -192,7 +192,7 @@ class SkeletonWrapperClassTestView [&events](auto& event_mock_pair) { auto event_base_it = events.find(std::string{event_mock_pair.event_name}); SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(event_base_it != events.end()); - auto& event_base = event_base_it->second.get(); + auto& event_base = event_base_it->second.get().Get(); InjectEventMock(event_base, event_mock_pair.mock); }(unpacked_tuple), ...); @@ -205,7 +205,7 @@ class SkeletonWrapperClassTestView [&fields](auto& field_mock_pair) { auto field_base_it = fields.find(std::string{field_mock_pair.field_name}); SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(field_base_it != fields.end()); - auto& field_base = field_base_it->second.get(); + auto& field_base = field_base_it->second.get().Get(); InjectFieldMock(field_base, field_mock_pair.mock); }(unpacked_tuple), ...); diff --git a/score/mw/com/impl/skeleton_base.cpp b/score/mw/com/impl/skeleton_base.cpp index 19b79cca5..db051acbd 100644 --- a/score/mw/com/impl/skeleton_base.cpp +++ b/score/mw/com/impl/skeleton_base.cpp @@ -14,7 +14,7 @@ #include "score/mw/com/impl/instance_identifier.h" #include "score/mw/com/impl/methods/skeleton_method_base.h" -#include "score/mw/com/impl/plumbing/skeleton_binding_factory.h" +#include "score/mw/com/impl/reference_to_moveable.h" #include "score/mw/com/impl/runtime.h" #include "score/mw/com/impl/skeleton_binding.h" #include "score/mw/com/impl/skeleton_event_base.h" @@ -28,7 +28,7 @@ #include #include -#include +#include #include #include @@ -55,7 +55,7 @@ SkeletonBinding::SkeletonEventBindings GetSkeletonEventBindingsMap(const Skeleto for (const auto& event : events) { const std::string_view event_name = event.first; - SkeletonEventBase& skeleton_event_base = event.second.get(); + SkeletonEventBase& skeleton_event_base = event.second.get().Get(); auto skeleton_event_base_view = SkeletonEventBaseView{skeleton_event_base}; auto* event_binding = skeleton_event_base_view.GetBinding(); @@ -72,7 +72,7 @@ SkeletonBinding::SkeletonFieldBindings GetSkeletonFieldBindingsMap(const Skeleto for (const auto& field : fields) { const std::string_view field_name = field.first; - SkeletonFieldBase& skeleton_field_base = field.second.get(); + SkeletonFieldBase& skeleton_field_base = field.second.get().Get(); auto skeleton_field_base_view = SkeletonFieldBaseView{skeleton_field_base}; auto* event_binding = skeleton_field_base_view.GetEventBinding(); @@ -96,49 +96,12 @@ SkeletonBase::SkeletonBase(std::unique_ptr skeleton_binding, In { } -SkeletonBase::SkeletonBase(SkeletonBase&& other) noexcept - : binding_{std::move(other.binding_)}, - events_{std::move(other.events_)}, - fields_{std::move(other.fields_)}, - methods_{std::move(other.methods_)}, - instance_id_{std::move(other.instance_id_)}, - skeleton_mock_{std::move(other.skeleton_mock_)}, - service_offered_flag_{std::move(other.service_offered_flag_)} -{ - UpdateAllServiceElementReferences(); -} - -// Suppress "AUTOSAR C++14 A6-2-1" rule violation. The rule states "Move and copy assignment operators shall either move -// or respectively copy base classes and data members of a class, without any side effects." Due to architectural -// decisions, SkeletonBase must perform a cleanup and update references to itself in its events and fields. Therefore, -// side effects are required. -// coverity[autosar_cpp14_a6_2_1_violation] -SkeletonBase& SkeletonBase::operator=(SkeletonBase&& other) noexcept -{ - if (this == &other) - { - return *this; - } - - binding_ = std::move(other.binding_); - events_ = std::move(other.events_); - fields_ = std::move(other.fields_); - methods_ = std::move(other.methods_); - instance_id_ = std::move(other.instance_id_); - skeleton_mock_ = std::move(other.skeleton_mock_); - service_offered_flag_ = std::move(other.service_offered_flag_); - - UpdateAllServiceElementReferences(); - - return *this; -} - score::Result SkeletonBase::OfferServiceEvents() const noexcept { - for (auto& event : events_) + for (const auto& event : events_) { const auto event_name = event.first; - auto& skeleton_event = event.second.get(); + auto& skeleton_event = event.second.get().Get(); const auto offer_result = skeleton_event.PrepareOffer(); if (!offer_result.has_value()) { @@ -153,10 +116,10 @@ score::Result SkeletonBase::OfferServiceEvents() const noexcept score::Result SkeletonBase::OfferServiceFields() const noexcept { - for (auto& field : fields_) + for (const auto& field : fields_) { const auto field_name = field.first; - auto& skeleton_field = field.second.get(); + auto& skeleton_field = field.second.get().Get(); const auto offer_result = skeleton_field.PrepareOffer(); if (!offer_result.has_value()) { @@ -247,11 +210,11 @@ auto SkeletonBase::StopOfferService() noexcept -> void for (auto& event : events_) { - event.second.get().PrepareStopOffer(); + event.second.get().Get().PrepareStopOffer(); } for (auto& field : fields_) { - field.second.get().PrepareStopOffer(); + field.second.get().Get().PrepareStopOffer(); } auto tracing_handler = tracing::CreateUnregisterShmObjectCallback(instance_id_, events_, fields_, *binding_); @@ -267,23 +230,23 @@ auto SkeletonBase::AreBindingsValid() const noexcept -> bool bool are_service_element_bindings_valid{true}; score::cpp::ignore = - std::for_each(events_.begin(), events_.end(), [&are_service_element_bindings_valid](const auto& element) { - if (SkeletonEventBaseView{element.second.get()}.GetBinding() == nullptr) + std::for_each(events_.begin(), events_.end(), [&are_service_element_bindings_valid](auto& element) { + if (SkeletonEventBaseView{element.second.get().Get()}.GetBinding() == nullptr) { are_service_element_bindings_valid = false; } }); score::cpp::ignore = - std::for_each(fields_.begin(), fields_.end(), [&are_service_element_bindings_valid](const auto& element) { - if (SkeletonFieldBaseView{element.second.get()}.GetEventBinding() == nullptr) + std::for_each(fields_.begin(), fields_.end(), [&are_service_element_bindings_valid](auto& element) { + if (SkeletonFieldBaseView{element.second.get().Get()}.GetEventBinding() == nullptr) { are_service_element_bindings_valid = false; } }); score::cpp::ignore = - std::for_each(methods_.begin(), methods_.end(), [&are_service_element_bindings_valid](const auto& element) { - if (SkeletonMethodBaseView{element.second.get()}.GetMethodBinding() == nullptr) + std::for_each(methods_.begin(), methods_.end(), [&are_service_element_bindings_valid](auto& element) { + if (SkeletonMethodBaseView{element.second.get().Get()}.GetMethodBinding() == nullptr) { are_service_element_bindings_valid = false; } @@ -292,22 +255,6 @@ auto SkeletonBase::AreBindingsValid() const noexcept -> bool return is_skeleton_binding_valid && are_service_element_bindings_valid; } -void SkeletonBase::UpdateAllServiceElementReferences() noexcept -{ - for (auto& event : events_) - { - event.second.get().UpdateSkeletonReference(*this); - } - for (auto& field : fields_) - { - field.second.get().UpdateSkeletonReference(*this); - } - for (auto& method : methods_) - { - method.second.get().UpdateSkeletonReference(*this); - } -} - SkeletonBaseView::SkeletonBaseView(SkeletonBase& skeleton_base) : skeleton_base_{skeleton_base} {} InstanceIdentifier SkeletonBaseView::GetAssociatedInstanceIdentifier() const @@ -320,57 +267,30 @@ SkeletonBinding* SkeletonBaseView::GetBinding() const return skeleton_base_.binding_.get(); } -void SkeletonBaseView::RegisterEvent(const std::string_view event_name, SkeletonEventBase& event) +void SkeletonBaseView::RegisterEvent(const std::string_view event_name, + ReferenceToMoveable::Reference& event) { - const auto result = skeleton_base_.events_.emplace(event_name, event); + const auto result = skeleton_base_.events_.emplace(event_name, std::ref(event)); const bool was_event_inserted = result.second; SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE(was_event_inserted, "Event cannot be registered as it already exists."); } -void SkeletonBaseView::RegisterField(const std::string_view field_name, SkeletonFieldBase& field) +void SkeletonBaseView::RegisterField(const std::string_view field_name, + ReferenceToMoveable::Reference& field) { - const auto result = skeleton_base_.fields_.emplace(field_name, field); + const auto result = skeleton_base_.fields_.emplace(field_name, std::ref(field)); const bool was_field_inserted = result.second; SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE(was_field_inserted, "Field cannot be registered as it already exists."); } -void SkeletonBaseView::RegisterMethod(const std::string_view method_name, SkeletonMethodBase& method) +void SkeletonBaseView::RegisterMethod(const std::string_view method_name, + ReferenceToMoveable::Reference& method) { - const auto result = skeleton_base_.methods_.emplace(method_name, method); + const auto result = skeleton_base_.methods_.emplace(method_name, std::ref(method)); const bool was_method_inserted = result.second; SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE(was_method_inserted, "Method cannot be registered as it already exists."); } -void SkeletonBaseView::UpdateEvent(const std::string_view event_name, SkeletonEventBase& event) noexcept -{ - auto event_name_it = skeleton_base_.events_.find(event_name); - SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE( - event_name_it != skeleton_base_.events_.cend(), - "SkeletonBaseView::UpdateEvent failed to update event because the requested event doesn't exist"); - - event_name_it->second = event; -} - -void SkeletonBaseView::UpdateField(const std::string_view field_name, SkeletonFieldBase& field) noexcept -{ - auto field_name_it = skeleton_base_.fields_.find(field_name); - SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE( - field_name_it != skeleton_base_.fields_.cend(), - "SkeletonBaseView::UpdateField failed to update field because the requested field doesn't exist"); - - field_name_it->second = field; -} - -void SkeletonBaseView::UpdateMethod(const std::string_view method_name, SkeletonMethodBase& method) noexcept -{ - auto method_it = skeleton_base_.methods_.find(method_name); - SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE( - method_it != skeleton_base_.methods_.cend(), - "SkeletonBaseView::UpdateMethod failed to update method because the requested method doesn't exist"); - - method_it->second = method; -} - const SkeletonBase::SkeletonEvents& SkeletonBaseView::GetEvents() const& noexcept { return skeleton_base_.events_; @@ -398,7 +318,7 @@ score::cpp::optional GetInstanceIdentifier(const InstanceSpe { return {}; } - const auto instance_identifier = instance_identifiers.front(); + auto instance_identifier = instance_identifiers.front(); return instance_identifier; } diff --git a/score/mw/com/impl/skeleton_base.h b/score/mw/com/impl/skeleton_base.h index 6c351d215..b71cbe958 100644 --- a/score/mw/com/impl/skeleton_base.h +++ b/score/mw/com/impl/skeleton_base.h @@ -17,6 +17,7 @@ #include "score/mw/com/impl/instance_identifier.h" #include "score/mw/com/impl/instance_specifier.h" #include "score/mw/com/impl/methods/skeleton_method_base.h" +#include "score/mw/com/impl/reference_to_moveable.h" #include "score/mw/com/impl/skeleton_binding.h" #include "score/mw/com/impl/mocking/i_skeleton_base.h" @@ -26,7 +27,6 @@ #include #include -#include #include #include #include @@ -44,9 +44,12 @@ class SkeletonMethodBase; class SkeletonBase { public: - using SkeletonEvents = std::map>; - using SkeletonFields = std::map>; - using SkeletonMethods = std::map>; + using SkeletonEvents = + std::map::Reference>>; + using SkeletonFields = + std::map::Reference>>; + using SkeletonMethods = + std::map::Reference>>; /// \brief Creation of service skeleton with provided Skeleton binding /// @@ -93,8 +96,8 @@ class SkeletonBase /// \brief A Skeleton shall be movable /// \requirement SWS_CM_00135 - SkeletonBase(SkeletonBase&& other) noexcept; - SkeletonBase& operator=(SkeletonBase&& other) noexcept; + SkeletonBase(SkeletonBase&& other) noexcept = default; + SkeletonBase& operator=(SkeletonBase&& other) noexcept = default; private: std::unique_ptr binding_; @@ -108,8 +111,6 @@ class SkeletonBase [[nodiscard]] score::Result OfferServiceEvents() const noexcept; [[nodiscard]] score::Result OfferServiceFields() const noexcept; - void UpdateAllServiceElementReferences() noexcept; - FlagOwner service_offered_flag_; }; @@ -122,17 +123,11 @@ class SkeletonBaseView [[nodiscard]] SkeletonBinding* GetBinding() const; - void RegisterEvent(std::string_view event_name, SkeletonEventBase& event); - - void RegisterField(std::string_view field_name, SkeletonFieldBase& field); - - void RegisterMethod(std::string_view method_name, SkeletonMethodBase& method); - - void UpdateEvent(std::string_view event_name, SkeletonEventBase& event) noexcept; + void RegisterEvent(std::string_view event_name, ReferenceToMoveable::Reference& event); - void UpdateField(std::string_view field_name, SkeletonFieldBase& field) noexcept; + void RegisterField(std::string_view field_name, ReferenceToMoveable::Reference& field); - void UpdateMethod(std::string_view method_name, SkeletonMethodBase& method) noexcept; + void RegisterMethod(std::string_view method_name, ReferenceToMoveable::Reference& method); [[nodiscard]] const SkeletonBase::SkeletonEvents& GetEvents() const& noexcept; diff --git a/score/mw/com/impl/skeleton_base_test.cpp b/score/mw/com/impl/skeleton_base_test.cpp index 6ca11aa52..6b27d86c2 100644 --- a/score/mw/com/impl/skeleton_base_test.cpp +++ b/score/mw/com/impl/skeleton_base_test.cpp @@ -17,6 +17,7 @@ #include "score/mw/com/impl/com_error.h" #include "score/mw/com/impl/configuration/test/configuration_store.h" #include "score/mw/com/impl/methods/skeleton_method_base.h" +#include "score/mw/com/impl/reference_to_moveable.h" #include "score/mw/com/impl/service_discovery_mock.h" #include "score/mw/com/impl/skeleton_event.h" #include "score/mw/com/impl/skeleton_event_base.h" @@ -661,8 +662,6 @@ class DummyField : public SkeletonFieldBase public: using SkeletonFieldBase::SkeletonFieldBase; - void UpdateSkeletonReference(SkeletonBase& skeleton_base) noexcept override {} - bool IsInitialValueSaved() const noexcept override { return false; @@ -707,6 +706,8 @@ class SkeletonBaseServiceElementReferencesFixture : public ::testing::Test SkeletonEventBase event_0_{skeleton_, event_name_0_, std::make_unique()}; SkeletonEventBase event_1_{skeleton_, event_name_1_, std::make_unique()}; + ReferenceToMoveable event_0_reference_wrapper_{event_0_}; + ReferenceToMoveable event_1_reference_wrapper_{event_1_}; std::unique_ptr field_event_dispatch_0_{ std::make_unique(skeleton_, @@ -719,9 +720,13 @@ class SkeletonBaseServiceElementReferencesFixture : public ::testing::Test DummyField field_0_{skeleton_, field_name_0_, std::move(field_event_dispatch_0_)}; DummyField field_1_{skeleton_, field_name_0_, std::move(field_event_dispatch_0_)}; + ReferenceToMoveable field_0_reference_wrapper_{field_0_}; + ReferenceToMoveable field_1_reference_wrapper_{field_1_}; SkeletonMethodBase method_0_{skeleton_, method_name_0_, std::make_unique()}; SkeletonMethodBase method_1_{skeleton_, method_name_1_, std::make_unique()}; + ReferenceToMoveable method_0_reference_wrapper_{method_0_}; + ReferenceToMoveable method_1_reference_wrapper_{method_1_}; }; TEST_F(SkeletonBaseServiceElementReferencesFixture, RegisteringServiceElementStoresReferenceInMap) @@ -729,39 +734,39 @@ TEST_F(SkeletonBaseServiceElementReferencesFixture, RegisteringServiceElementSto // Given a valid MySkeleton object // When registering 2 Events, Fields and Methods - SkeletonBaseView{skeleton_}.RegisterEvent(event_name_0_, event_0_); - SkeletonBaseView{skeleton_}.RegisterEvent(event_name_1_, event_1_); - SkeletonBaseView{skeleton_}.RegisterField(field_name_0_, field_0_); - SkeletonBaseView{skeleton_}.RegisterField(field_name_1_, field_1_); - SkeletonBaseView{skeleton_}.RegisterMethod(method_name_0_, method_0_); - SkeletonBaseView{skeleton_}.RegisterMethod(method_name_1_, method_1_); + SkeletonBaseView{skeleton_}.RegisterEvent(event_name_0_, event_0_reference_wrapper_.Get()); + SkeletonBaseView{skeleton_}.RegisterEvent(event_name_1_, event_1_reference_wrapper_.Get()); + SkeletonBaseView{skeleton_}.RegisterField(field_name_0_, field_0_reference_wrapper_.Get()); + SkeletonBaseView{skeleton_}.RegisterField(field_name_1_, field_1_reference_wrapper_.Get()); + SkeletonBaseView{skeleton_}.RegisterMethod(method_name_0_, method_0_reference_wrapper_.Get()); + SkeletonBaseView{skeleton_}.RegisterMethod(method_name_1_, method_1_reference_wrapper_.Get()); // Then the skeleton's reference maps should contain references to the registered elements const auto& events = skeleton_.GetEvents(); EXPECT_EQ(events.size(), 2U); - EXPECT_EQ(&events.at(event_name_0_).get(), &event_0_); - EXPECT_EQ(&events.at(event_name_1_).get(), &event_1_); + EXPECT_EQ(&events.at(event_name_0_).get().Get(), &event_0_); + EXPECT_EQ(&events.at(event_name_1_).get().Get(), &event_1_); const auto& fields = skeleton_.GetFields(); EXPECT_EQ(fields.size(), 2U); - EXPECT_EQ(&fields.at(field_name_0_).get(), &field_0_); - EXPECT_EQ(&fields.at(field_name_1_).get(), &field_1_); + EXPECT_EQ(&fields.at(field_name_0_).get().Get(), &field_0_); + EXPECT_EQ(&fields.at(field_name_1_).get().Get(), &field_1_); const auto& methods = skeleton_.GetMethods(); EXPECT_EQ(methods.size(), 2U); - EXPECT_EQ(&methods.at(method_name_0_).get(), &method_0_); - EXPECT_EQ(&methods.at(method_name_1_).get(), &method_1_); + EXPECT_EQ(&methods.at(method_name_0_).get().Get(), &method_0_); + EXPECT_EQ(&methods.at(method_name_1_).get().Get(), &method_1_); } TEST_F(SkeletonBaseServiceElementReferencesFixture, MoveConstructingUpdatesReferencesToServiceElements) { // Given a valid MySkeleton object on which 2 Events, Fields and Methods were registered - SkeletonBaseView{skeleton_}.RegisterEvent(event_name_0_, event_0_); - SkeletonBaseView{skeleton_}.RegisterEvent(event_name_1_, event_1_); - SkeletonBaseView{skeleton_}.RegisterField(field_name_0_, field_0_); - SkeletonBaseView{skeleton_}.RegisterField(field_name_1_, field_1_); - SkeletonBaseView{skeleton_}.RegisterMethod(method_name_0_, method_0_); - SkeletonBaseView{skeleton_}.RegisterMethod(method_name_1_, method_1_); + SkeletonBaseView{skeleton_}.RegisterEvent(event_name_0_, event_0_reference_wrapper_.Get()); + SkeletonBaseView{skeleton_}.RegisterEvent(event_name_1_, event_1_reference_wrapper_.Get()); + SkeletonBaseView{skeleton_}.RegisterField(field_name_0_, field_0_reference_wrapper_.Get()); + SkeletonBaseView{skeleton_}.RegisterField(field_name_1_, field_1_reference_wrapper_.Get()); + SkeletonBaseView{skeleton_}.RegisterMethod(method_name_0_, method_0_reference_wrapper_.Get()); + SkeletonBaseView{skeleton_}.RegisterMethod(method_name_1_, method_1_reference_wrapper_.Get()); // When move constructing a new MySkeleton object MySkeleton moved_to_skeleton{std::move(skeleton_)}; @@ -769,18 +774,18 @@ TEST_F(SkeletonBaseServiceElementReferencesFixture, MoveConstructingUpdatesRefer // Then the moved-to skeleton's reference maps should still contain references to the registered elements const auto& events = moved_to_skeleton.GetEvents(); ASSERT_EQ(events.size(), 2U); - EXPECT_EQ(&events.at(event_name_0_).get(), &event_0_); - EXPECT_EQ(&events.at(event_name_1_).get(), &event_1_); + EXPECT_EQ(&events.at(event_name_0_).get().Get(), &event_0_); + EXPECT_EQ(&events.at(event_name_1_).get().Get(), &event_1_); const auto& fields = moved_to_skeleton.GetFields(); ASSERT_EQ(fields.size(), 2U); - EXPECT_EQ(&fields.at(field_name_0_).get(), &field_0_); - EXPECT_EQ(&fields.at(field_name_1_).get(), &field_1_); + EXPECT_EQ(&fields.at(field_name_0_).get().Get(), &field_0_); + EXPECT_EQ(&fields.at(field_name_1_).get().Get(), &field_1_); const auto& methods = moved_to_skeleton.GetMethods(); EXPECT_EQ(methods.size(), 2U); - EXPECT_EQ(&methods.at(method_name_0_).get(), &method_0_); - EXPECT_EQ(&methods.at(method_name_1_).get(), &method_1_); + EXPECT_EQ(&methods.at(method_name_0_).get().Get(), &method_0_); + EXPECT_EQ(&methods.at(method_name_1_).get().Get(), &method_1_); } TEST_F(SkeletonBaseServiceElementReferencesFixture, MoveAssigningUpdatesReferencesToServiceElements) @@ -798,28 +803,31 @@ TEST_F(SkeletonBaseServiceElementReferencesFixture, MoveAssigningUpdatesReferenc mock_binding::Skeleton skeleton_binding_mock{}; // Given a valid MySkeleton object on which 2 Events, Fields and Methods were registered - SkeletonBaseView{skeleton_}.RegisterEvent(event_name_0_, event_0_); - SkeletonBaseView{skeleton_}.RegisterField(field_name_0_, field_0_); - SkeletonBaseView{skeleton_}.RegisterMethod(method_name_0_, method_0_); - SkeletonBaseView{skeleton_}.RegisterEvent(event_name_1_, event_1_); - SkeletonBaseView{skeleton_}.RegisterField(field_name_1_, field_1_); - SkeletonBaseView{skeleton_}.RegisterMethod(method_name_1_, method_1_); + SkeletonBaseView{skeleton_}.RegisterEvent(event_name_0_, event_0_reference_wrapper_.Get()); + SkeletonBaseView{skeleton_}.RegisterEvent(event_name_1_, event_1_reference_wrapper_.Get()); + SkeletonBaseView{skeleton_}.RegisterField(field_name_0_, field_0_reference_wrapper_.Get()); + SkeletonBaseView{skeleton_}.RegisterField(field_name_1_, field_1_reference_wrapper_.Get()); + SkeletonBaseView{skeleton_}.RegisterMethod(method_name_0_, method_0_reference_wrapper_.Get()); + SkeletonBaseView{skeleton_}.RegisterMethod(method_name_1_, method_1_reference_wrapper_.Get()); // and given a second valid MySkeleton object MySkeleton skeleton_2{std::make_unique(skeleton_binding_mock), instance_identifier_}; // and given that an Event, Field and Method were registered on the second skeleton SkeletonEventBase event{skeleton_2, other_event_name, std::make_unique()}; + ReferenceToMoveable event_reference_wrapper{event}; auto field_event_dispatch = std::make_unique( skeleton_2, other_field_name, std::make_unique()); DummyField field{skeleton_2, other_field_name, std::move(field_event_dispatch)}; + ReferenceToMoveable field_reference_wrapper{field}; SkeletonMethodBase method{skeleton_2, other_method_name, std::make_unique()}; - SkeletonBaseView{skeleton_2}.RegisterEvent(other_event_name, event); - SkeletonBaseView{skeleton_2}.RegisterField(other_field_name, field); - SkeletonBaseView{skeleton_2}.RegisterMethod(other_method_name, method); + ReferenceToMoveable method_reference_wrapper{method}; + SkeletonBaseView{skeleton_2}.RegisterEvent(other_event_name, event_reference_wrapper.Get()); + SkeletonBaseView{skeleton_2}.RegisterField(other_field_name, field_reference_wrapper.Get()); + SkeletonBaseView{skeleton_2}.RegisterMethod(other_method_name, method_reference_wrapper.Get()); // When move assigning the first MySkeleton object to the second skeleton_2 = std::move(skeleton_); @@ -827,29 +835,31 @@ TEST_F(SkeletonBaseServiceElementReferencesFixture, MoveAssigningUpdatesReferenc // Then the second skeleton's reference maps should contain references to the first skeleton's registered elements const auto& events = skeleton_2.GetEvents(); ASSERT_EQ(events.size(), 2U); - EXPECT_EQ(&events.at(event_name_0_).get(), &event_0_); - EXPECT_EQ(&events.at(event_name_1_).get(), &event_1_); + EXPECT_EQ(&events.at(event_name_0_).get().Get(), &event_0_); + EXPECT_EQ(&events.at(event_name_1_).get().Get(), &event_1_); const auto& fields = skeleton_2.GetFields(); ASSERT_EQ(fields.size(), 2U); - EXPECT_EQ(&fields.at(field_name_0_).get(), &field_0_); - EXPECT_EQ(&fields.at(field_name_1_).get(), &field_1_); + EXPECT_EQ(&fields.at(field_name_0_).get().Get(), &field_0_); + EXPECT_EQ(&fields.at(field_name_1_).get().Get(), &field_1_); const auto& methods = skeleton_2.GetMethods(); EXPECT_EQ(methods.size(), 2U); - EXPECT_EQ(&methods.at(method_name_0_).get(), &method_0_); - EXPECT_EQ(&methods.at(method_name_1_).get(), &method_1_); + EXPECT_EQ(&methods.at(method_name_0_).get().Get(), &method_0_); + EXPECT_EQ(&methods.at(method_name_1_).get().Get(), &method_1_); } TEST_F(SkeletonBaseServiceElementReferencesFixture, MoveAssigningToItselfDoesNotDoAnything) { mock_binding::Skeleton skeleton_binding_mock{}; + // Given a valid MySkeleton object MySkeleton skeleton_2{std::make_unique(skeleton_binding_mock), instance_identifier_}; - // When move assigning the MySkeleton object to itself - auto other_name_same_skeleton_p = &skeleton_2; + // When move assigning the MySkeleton object to itself + auto* other_name_same_skeleton_p = &skeleton_2; skeleton_2 = std::move(*other_name_same_skeleton_p); + // Then nothing happens. // In case of self assignement we would want to know that actually nothing happens and no sideffects occur. // Abscence of sideeffects is not possible to test for. This test only validates that the self assignement branchcan diff --git a/score/mw/com/impl/skeleton_event.h b/score/mw/com/impl/skeleton_event.h index 27c341639..2bb45b3ef 100644 --- a/score/mw/com/impl/skeleton_event.h +++ b/score/mw/com/impl/skeleton_event.h @@ -89,8 +89,8 @@ class SkeletonEvent : public SkeletonEventBase SkeletonEvent(const SkeletonEvent&) = delete; SkeletonEvent& operator=(const SkeletonEvent&) & = delete; - SkeletonEvent(SkeletonEvent&& other) noexcept; - SkeletonEvent& operator=(SkeletonEvent&& other) & noexcept; + SkeletonEvent(SkeletonEvent&& other) noexcept = default; + SkeletonEvent& operator=(SkeletonEvent&& other) & noexcept = default; /** * \api @@ -141,8 +141,7 @@ SkeletonEvent::SkeletonEvent(SkeletonBase& skeleton_base, const event_name)}, skeleton_event_mock_{nullptr} { - SkeletonBaseView base_skeleton_view{skeleton_base}; - base_skeleton_view.RegisterEvent(event_name, *this); + SkeletonBaseView{skeleton_base}.RegisterEvent(event_name, reference_to_moveable_.Get()); if (binding_ != nullptr) { @@ -181,40 +180,6 @@ SkeletonEvent::SkeletonEvent(SkeletonBase& skeleton_base, { } -template -SkeletonEvent::SkeletonEvent(SkeletonEvent&& other) noexcept - : SkeletonEventBase(std::move(static_cast(other))), - skeleton_event_mock_{std::move(other.skeleton_event_mock_)} -{ - // Since the address of this event has changed, we need update the address stored in the parent skeleton. - SkeletonBaseView base_skeleton_view{skeleton_base_.get()}; - base_skeleton_view.UpdateEvent(event_name_, *this); - - other.skeleton_event_mock_ = nullptr; -} - -template -// Suppress "AUTOSAR C++14 A6-2-1" rule violation. The rule states "Move and copy assignment operators shall either move -// or respectively copy base classes and data members of a class, without any side effects." -// Rationale: The parent skeleton stores a reference to the SkeletonEvent. The address that is pointed to must be -// updated when the SkeletonEvent is moved. Therefore, side effects are required. -// coverity[autosar_cpp14_a6_2_1_violation] -auto SkeletonEvent::operator=(SkeletonEvent&& other) & noexcept -> SkeletonEvent& -{ - if (this != &other) - { - SkeletonEventBase::operator=(std::move(other)); - - // Since the address of this event has changed, we need update the address stored in the parent skeleton. - SkeletonBaseView base_skeleton_view{skeleton_base_.get()}; - base_skeleton_view.UpdateEvent(event_name_, *this); - - skeleton_event_mock_ = std::move(other.skeleton_event_mock_); - other.skeleton_event_mock_ = nullptr; - } - return *this; -} - template Result SkeletonEvent::Send(const EventType& sample_value) noexcept { diff --git a/score/mw/com/impl/skeleton_event_base.h b/score/mw/com/impl/skeleton_event_base.h index 128277e94..c264bde6c 100644 --- a/score/mw/com/impl/skeleton_event_base.h +++ b/score/mw/com/impl/skeleton_event_base.h @@ -14,6 +14,7 @@ #define SCORE_MW_COM_IMPL_SKELETON_EVENT_BASE_H #include "score/mw/com/impl/flag_owner.h" +#include "score/mw/com/impl/reference_to_moveable.h" #include "score/mw/com/impl/skeleton_event_binding.h" #include "score/mw/com/impl/tracing/skeleton_event_tracing.h" #include "score/mw/com/impl/tracing/skeleton_event_tracing_data.h" @@ -46,20 +47,15 @@ class SkeletonEventBase const std::string_view event_name, std::unique_ptr binding) : binding_{std::move(binding)}, - skeleton_base_{skeleton_base}, event_name_{event_name}, tracing_data_{}, - service_offered_flag_{} + service_offered_flag_{}, + reference_to_moveable_(*static_cast(this)) { } virtual ~SkeletonEventBase() = default; - void UpdateSkeletonReference(SkeletonBase& skeleton_base) noexcept - { - skeleton_base_ = skeleton_base; - } - /// \brief Used to indicate that the event shall be available to consumer /// Performs binding independent functionality and then dispatches to the binding score::Result PrepareOffer() noexcept @@ -87,27 +83,52 @@ class SkeletonEventBase SkeletonEventBase(const SkeletonEventBase&) = delete; SkeletonEventBase& operator=(const SkeletonEventBase&) & = delete; - SkeletonEventBase(SkeletonEventBase&&) noexcept = default; - SkeletonEventBase& operator=(SkeletonEventBase&& other) & noexcept = default; + SkeletonEventBase(SkeletonEventBase&& other) noexcept + : binding_{std::move(other.binding_)}, + event_name_{other.event_name_}, + tracing_data_{std::move(other.tracing_data_)}, + service_offered_flag_{std::move(other.service_offered_flag_)}, + reference_to_moveable_{std::move(other.reference_to_moveable_)} + { + reference_to_moveable_.Update(*this); + } + + SkeletonEventBase& operator=(SkeletonEventBase&& other) & noexcept + { + if (this == &other) + { + return *this; + } + + binding_ = std::move(other.binding_); + event_name_ = other.event_name_; + tracing_data_ = std::move(other.tracing_data_); + service_offered_flag_ = std::move(other.service_offered_flag_); + reference_to_moveable_ = std::move(other.reference_to_moveable_); + + reference_to_moveable_.Update(*this); + + return *this; + } // Suppress "AUTOSAR C++14 M11-0-1" rule findings. This rule states: "Member data in non-POD class types shall // be private.". We need these data elements to exchange this information between the SkeletonEventBase and the // SkeletonEvent. // coverity[autosar_cpp14_m11_0_1_violation] std::unique_ptr binding_; - - // The SkeletonEventBase must contain a reference to the SkeletonBase so that a SkeletonBase can call - // UpdateSkeletonReference whenever it is moved to a new address. A SkeletonBase only has a reference to a - // SkeletonEventBase, not a typed SkeletonEvent, which is why UpdateSkeletonReference has to be in this class - // despite skeleton_base_ being used in the derived class, SkeletonEvent. - // coverity[autosar_cpp14_m11_0_1_violation] - std::reference_wrapper skeleton_base_; // coverity[autosar_cpp14_m11_0_1_violation] std::string_view event_name_; // coverity[autosar_cpp14_m11_0_1_violation] tracing::SkeletonEventTracingData tracing_data_; // coverity[autosar_cpp14_m11_0_1_violation] FlagOwner service_offered_flag_; + + /// \brief Helper class for creating reference to this SkeletonEventBase which is provided to SkeletonBase when + /// registering this event. + /// + /// Contains a heap allocated reference to this SkeletonEventBase which is updated in the move constructor and move + /// assignment operator so that it's always valid, even after moving the SkeletonEventBase. + ReferenceToMoveable reference_to_moveable_; }; class SkeletonEventBaseView diff --git a/score/mw/com/impl/skeleton_event_test.cpp b/score/mw/com/impl/skeleton_event_test.cpp index 45aa2f81c..16a806eba 100644 --- a/score/mw/com/impl/skeleton_event_test.cpp +++ b/score/mw/com/impl/skeleton_event_test.cpp @@ -574,7 +574,7 @@ TEST(SkeletonEventTest, SkeletonEventsRegisterThemselvesWithSkeleton) EXPECT_EQ(event_name, kEventName); // and the event in the map corresponds to the correct skeleton event address - EXPECT_EQ(&event, &unit.my_dummy_event_); + EXPECT_EQ(&event.Get(), &unit.my_dummy_event_); } TEST(SkeletonEventTest, MovingConstructingSkeletonUpdatesEventMapReference) @@ -606,7 +606,7 @@ TEST(SkeletonEventTest, MovingConstructingSkeletonUpdatesEventMapReference) EXPECT_EQ(event_name, kEventName); // and the event in the map corresponds to the new skeleton event address - EXPECT_EQ(&event, &unit2.my_dummy_event_); + EXPECT_EQ(&event.Get(), &unit2.my_dummy_event_); } TEST(SkeletonEventTest, MovingAssigningSkeletonUpdatesEventMapReference) @@ -650,7 +650,7 @@ TEST(SkeletonEventTest, MovingAssigningSkeletonUpdatesEventMapReference) EXPECT_EQ(event_name, kEventName); // and the event in the map corresponds to the new skeleton event address - EXPECT_EQ(&event, &unit2.my_dummy_event_); + EXPECT_EQ(&event.Get(), &unit2.my_dummy_event_); } } // namespace diff --git a/score/mw/com/impl/skeleton_field.h b/score/mw/com/impl/skeleton_field.h index 4893676f6..9e844a52e 100644 --- a/score/mw/com/impl/skeleton_field.h +++ b/score/mw/com/impl/skeleton_field.h @@ -94,8 +94,8 @@ class SkeletonField : public SkeletonFieldBase SkeletonField(const SkeletonField&) = delete; SkeletonField& operator=(const SkeletonField&) & = delete; - SkeletonField(SkeletonField&& other) noexcept; - SkeletonField& operator=(SkeletonField&& other) & noexcept; + SkeletonField(SkeletonField&& other) noexcept = default; + SkeletonField& operator=(SkeletonField&& other) & noexcept = default; /** * \api @@ -171,27 +171,6 @@ class SkeletonField : public SkeletonFieldBase return set_method_->RegisterHandler(std::move(wrapped_callback)); } - /// \brief Updates the reference to SkeletonBase held by this SkeletonField and also the owned methods. - /// - /// This is necessary when a Skeleton (which owns its events, fields and methods) is moved to a new address. When - /// this happens, the references to the SkeletonBase are pointing to the old address and must be updated. This must - /// be done also for the get and set method since they call a SkeletonMethod constructor which does not register - /// them with the SkeletonBase. Rather, they're considered as part of the SkeletonField and it's the field's - /// responsibility to update their SkeletonBase reference when it's moved. - void UpdateSkeletonReference(SkeletonBase& skeleton_base) noexcept override - { - skeleton_base_ = skeleton_base; - - if (set_method_ != nullptr) - { - set_method_->UpdateSkeletonReference(skeleton_base); - } - if (get_method_ != nullptr) - { - get_method_->UpdateSkeletonReference(skeleton_base); - } - } - private: using SetMethodSignature = FieldType(FieldType); using GetMethodSignature = FieldType(); @@ -303,44 +282,7 @@ SkeletonField::SkeletonField( get_method_{std::move(skeleton_get_method_dispatch)} { SkeletonBaseView skeleton_base_view{parent}; - skeleton_base_view.RegisterField(field_name, *this); -} - -template -SkeletonField::SkeletonField(SkeletonField&& other) noexcept - : SkeletonFieldBase(static_cast(other)), - // known llvm bug (https://github.com/llvm/llvm-project/issues/63202) - // This usage is safe because the previous line only moves the base class portion via static_cast. - // The derived class member 'initial_field_value_' remains untouched by the base class move constructor, - // so it's still valid to access it here for moving into our own member. - // coverity[autosar_cpp14_a12_8_3_violation] This is a false-positive. - initial_field_value_{std::move(other.initial_field_value_)}, - skeleton_field_mock_{other.skeleton_field_mock_}, - is_set_handler_registered_{other.is_set_handler_registered_}, - set_method_{std::move(other.set_method_)}, - get_method_{std::move(other.get_method_)} -{ - SkeletonBaseView skeleton_base_view{skeleton_base_.get()}; - skeleton_base_view.UpdateField(field_name_, *this); -} - -template -auto SkeletonField::operator=(SkeletonField&& other) & noexcept - -> SkeletonField& -{ - if (this != &other) - { - SkeletonFieldBase::operator=(std::move(other)); - - initial_field_value_ = std::move(other.initial_field_value_); - skeleton_field_mock_ = std::move(other.skeleton_field_mock_); - is_set_handler_registered_ = other.is_set_handler_registered_; - set_method_ = std::move(other.set_method_); - get_method_ = std::move(other.get_method_); - SkeletonBaseView skeleton_base_view{skeleton_base_.get()}; - skeleton_base_view.UpdateField(field_name_, *this); - } - return *this; + skeleton_base_view.RegisterField(field_name, reference_to_moveable_.Get()); } /// \brief FieldType is allocated by the user and provided to the middleware to send. Dispatches to diff --git a/score/mw/com/impl/skeleton_field_base.h b/score/mw/com/impl/skeleton_field_base.h index 55eb98d67..8d0cf5e62 100644 --- a/score/mw/com/impl/skeleton_field_base.h +++ b/score/mw/com/impl/skeleton_field_base.h @@ -15,6 +15,7 @@ #include "score/mw/com/impl/com_error.h" #include "score/mw/com/impl/methods/skeleton_method_base.h" +#include "score/mw/com/impl/reference_to_moveable.h" #include "score/mw/com/impl/skeleton_event_base.h" #include "score/mw/log/logging.h" @@ -23,6 +24,7 @@ #include #include #include +#include namespace score::mw::com::impl { @@ -43,8 +45,8 @@ class SkeletonFieldBase std::unique_ptr skeleton_event_base) : skeleton_event_dispatch_{std::move(skeleton_event_base)}, was_prepare_offer_called_{false}, - skeleton_base_{skeleton_base}, - field_name_{field_name} + field_name_{field_name}, + reference_to_moveable_{*this} { } @@ -53,12 +55,6 @@ class SkeletonFieldBase SkeletonFieldBase(const SkeletonFieldBase&) = delete; SkeletonFieldBase& operator=(const SkeletonFieldBase&) & = delete; - /// \brief Updates the reference to SkeletonBase held by the SkeletonField and also the owned methods. - /// - /// This must happen in the derived class since the derived class owns the methods (this is required since they are - /// templated with the FieldType, which SkeletonFieldBase doesn't know). - virtual void UpdateSkeletonReference(SkeletonBase& skeleton_base) noexcept = 0; - /// \brief Used to indicate that the field shall be available to consumer (e.g. binding specific preparation) Result PrepareOffer() noexcept { @@ -106,8 +102,29 @@ class SkeletonFieldBase } protected: - SkeletonFieldBase(SkeletonFieldBase&&) noexcept = default; - SkeletonFieldBase& operator=(SkeletonFieldBase&&) & noexcept = default; + SkeletonFieldBase(SkeletonFieldBase&& other) noexcept + : skeleton_event_dispatch_{std::move(other.skeleton_event_dispatch_)}, + was_prepare_offer_called_{other.was_prepare_offer_called_}, + field_name_{other.field_name_}, + reference_to_moveable_{std::move(other.reference_to_moveable_)} + { + reference_to_moveable_.Update(*this); + } + + SkeletonFieldBase& operator=(SkeletonFieldBase&& other) & noexcept + { + if (this == &other) + { + return *this; + } + + skeleton_event_dispatch_ = std::move(other.skeleton_event_dispatch_); + was_prepare_offer_called_ = other.was_prepare_offer_called_; + field_name_ = other.field_name_; + reference_to_moveable_ = std::move(other.reference_to_moveable_); + reference_to_moveable_.Update(*this); + return *this; + } // Suppress "AUTOSAR C++14 M11-0-1" rule findings. This rule states: "Member data in non-POD class types shall // be private.". We need these data elements to exchange this information between the SkeletonBase and the // SkeletonField. @@ -116,15 +133,16 @@ class SkeletonFieldBase // coverity[autosar_cpp14_m11_0_1_violation] bool was_prepare_offer_called_; - // The SkeletonFieldBase must contain a reference to the SkeletonBase so that a SkeletonBase can call - // UpdateSkeletonReference whenever it is moved to a new address. A SkeletonBase only has a reference to a - // SkeletonFieldBase, not a typed SkeletonField, which is why UpdateSkeletonReference has to be in this class - // despite skeleton_base_ being used in the derived class, SkeletonField. - // coverity[autosar_cpp14_m11_0_1_violation] - std::reference_wrapper skeleton_base_; // coverity[autosar_cpp14_m11_0_1_violation] std::string_view field_name_; + /// \brief Helper class for creating reference to this SkeletonFieldBase which is provided to SkeletonBase when + /// registering this event. + /// + /// Contains a heap allocated reference to this SkeletonFieldBase which is updated in the move constructor and move + /// assignment operator so that it's always valid, even after moving the SkeletonFieldBase. + ReferenceToMoveable reference_to_moveable_; + private: /// \brief Returns whether the initial value has been saved by the user to be used by DoDeferredUpdate [[nodiscard]] virtual bool IsInitialValueSaved() const noexcept = 0; diff --git a/score/mw/com/impl/skeleton_field_base_test.cpp b/score/mw/com/impl/skeleton_field_base_test.cpp index a141d7e36..7e0808d06 100644 --- a/score/mw/com/impl/skeleton_field_base_test.cpp +++ b/score/mw/com/impl/skeleton_field_base_test.cpp @@ -64,8 +64,6 @@ class MyDummyField : public SkeletonFieldBase { } - void UpdateSkeletonReference(SkeletonBase& skeleton_base) noexcept override {} - StrictMock* GetMockEventBinding() noexcept { auto* const skeleton_field_base_binding = SkeletonFieldBaseView{*this}.GetEventBinding(); @@ -97,8 +95,6 @@ class MyDummyField : public SkeletonFieldBase class MyDummyFieldFailingDeferredUpdate final : public MyDummyField { public: - void UpdateSkeletonReference(SkeletonBase& skeleton_base) noexcept override {} - Result DoDeferredUpdate() noexcept override { return MakeUnexpected(ComErrc::kCommunicationLinkError); diff --git a/score/mw/com/impl/skeleton_field_test.cpp b/score/mw/com/impl/skeleton_field_test.cpp index 83b6d880b..c657c411f 100644 --- a/score/mw/com/impl/skeleton_field_test.cpp +++ b/score/mw/com/impl/skeleton_field_test.cpp @@ -775,7 +775,7 @@ TEST_F(SkeletonFieldTestFixture, SkeletonFieldsRegisterThemselvesWithSkeleton) ASSERT_EQ(fields.size(), 1); const auto field_name = fields.begin()->first; - const auto& field = fields.begin()->second.get(); + const auto& field = fields.begin()->second.get().Get(); // the name corresponds to the correct field name EXPECT_EQ(field_name, kFieldName); @@ -798,7 +798,7 @@ TEST_F(SkeletonFieldTestFixture, MovingConstructingSkeletonUpdatesFieldMapRefere ASSERT_EQ(fields.size(), 1); const auto field_name = fields.begin()->first; - const auto& field = fields.begin()->second.get(); + const auto& field = fields.begin()->second.get().Get(); // the name corresponds to the correct field name EXPECT_EQ(field_name, kFieldName); @@ -833,7 +833,7 @@ TEST_F(SkeletonFieldTestFixture, MovingAssigningSkeletonUpdatesFieldMapReference ASSERT_EQ(fields.size(), 1); const auto field_name = fields.begin()->first; - const auto& field = fields.begin()->second.get(); + const auto& field = fields.begin()->second.get().Get(); // the name corresponds to the correct field name EXPECT_EQ(field_name, kFieldName); @@ -842,24 +842,6 @@ TEST_F(SkeletonFieldTestFixture, MovingAssigningSkeletonUpdatesFieldMapReference EXPECT_EQ(&field, &unit2.my_dummy_field_); } -using SkeletonFieldDeathTest = SkeletonFieldTestFixture; -TEST_F(SkeletonFieldDeathTest, UpdateWithInvalidFieldNameTriggersTermination) -{ - // Given a skeleton created based on a Lola binding - MyDummySkeleton unit{std::make_unique(), kInstanceIdWithLolaBinding}; - - // Expect that the field map stored in the skeleton - const auto& fields = SkeletonBaseView{unit}.GetFields(); - - // contains a single field - ASSERT_EQ(fields.size(), 1); - - auto& field = fields.begin()->second.get(); - - // Then the program terminates as expected due to the invalid field name - EXPECT_DEATH(SkeletonBaseView{unit}.UpdateField("non_existing_test_field_name", field);, ".*"); -} - // Helper skeleton that holds a SkeletonField with all three tags. class MySetterSkeleton : public SkeletonBase { @@ -1250,12 +1232,8 @@ using SkeletonFieldMoveConstructionFixture = SkeletonFieldTestFixture; TEST_F(SkeletonFieldMoveConstructionFixture, SecondRegisterSetHandlerReplacesHandler) { // Note. This test verifies that moving a skeleton does not break the getter / setter methods stored within a field. - // When moving, UpdateSkeletonReference must update the references to SkeletonBase in the field instance as well as - // the stored methods. However, the SkeletonBase reference in the methods are only used when moving the method (to - // update the reference to the method in SkeletonBase). Since the methods are stored in the field as unique_ptrs, - // they will never actually be moved. Therefore, with the current implementation, the SkeletonBase reference in the - // getter and setter are never used and so we have no way of ensuring that they are updated correctly. We can only - // verify that the method is still valid after move construction. + // The getter and setter methods are owned by the field via unique_ptr, so they remain valid across skeleton moves. + // This test verifies usability after move construction. // Given a skeleton containing a field with a setter enabled MySetterSkeleton unit{std::make_unique(), kInstanceIdWithLolaBinding}; diff --git a/score/mw/com/impl/tracing/BUILD b/score/mw/com/impl/tracing/BUILD index 4b55adcac..1e0bf0dfb 100644 --- a/score/mw/com/impl/tracing/BUILD +++ b/score/mw/com/impl/tracing/BUILD @@ -116,6 +116,7 @@ cc_library( deps = [ ":tracing_runtime", "//score/mw/com/impl:instance_identifier", + "//score/mw/com/impl:reference_to_moveable", "//score/mw/com/impl:runtime", "//score/mw/com/impl:skeleton_binding", "//score/mw/com/impl:skeleton_event_base", diff --git a/score/mw/com/impl/tracing/skeleton_tracing.cpp b/score/mw/com/impl/tracing/skeleton_tracing.cpp index ca61d2e4a..ef2a6f029 100644 --- a/score/mw/com/impl/tracing/skeleton_tracing.cpp +++ b/score/mw/com/impl/tracing/skeleton_tracing.cpp @@ -16,10 +16,10 @@ /// #include "score/mw/com/impl/tracing/skeleton_tracing.h" +#include "score/mw/com/impl/reference_to_moveable.h" #include "score/mw/com/impl/runtime.h" #include "score/mw/com/impl/skeleton_event_base.h" #include "score/mw/com/impl/tracing/configuration/service_element_instance_identifier_view.h" -#include "score/mw/com/impl/tracing/tracing_runtime.h" #include @@ -35,11 +35,12 @@ bool IsTracingEnabledForInterfaceEvent(const SkeletonEventTracingData& skeleton_ } bool IsTracingEnabledForAnyEvent( - const std::map>& events) noexcept + const std::map::Reference>>& + events) noexcept { for (const auto& event : events) { - auto& skeleton_event_base = event.second.get(); + auto& skeleton_event_base = event.second.get().Get(); auto skeleton_event_base_view = SkeletonEventBaseView(skeleton_event_base); const auto& skeleton_event_tracing = skeleton_event_base_view.GetSkeletonEventTracing(); const bool tracing_enabled = IsTracingEnabledForInterfaceEvent(skeleton_event_tracing); @@ -52,11 +53,12 @@ bool IsTracingEnabledForAnyEvent( } bool IsTracingEnabledForAnyField( - const std::map>& fields) noexcept + const std::map::Reference>>& + fields) noexcept { for (const auto& field : fields) { - auto& skeleton_field_base = field.second.get(); + auto& skeleton_field_base = field.second.get().Get(); auto skeleton_field_base_view = SkeletonFieldBaseView(skeleton_field_base); auto skeleton_event_base_view = SkeletonEventBaseView(skeleton_field_base_view.GetEventBase()); const auto& skeleton_event_tracing = skeleton_event_base_view.GetSkeletonEventTracing(); @@ -71,8 +73,9 @@ bool IsTracingEnabledForAnyField( bool IsTracingEnabledForInstance( ITracingRuntime* const tracing_runtime, - const std::map>& events, - const std::map>& fields) noexcept + const std::map::Reference>>& events, + const std::map::Reference>>& + fields) noexcept { if (tracing_runtime == nullptr) { @@ -92,8 +95,8 @@ bool IsTracingEnabledForInstance( std::optional CreateRegisterShmObjectCallback( const InstanceIdentifier& instance_id, - const std::map>& events, - const std::map>& fields, + const std::map::Reference>>& events, + const std::map::Reference>>& fields, const SkeletonBinding& skeleton_binding) noexcept { std::optional tracing_handler{}; @@ -125,8 +128,8 @@ std::optional CreateRegisterShm std::optional CreateUnregisterShmObjectCallback( const InstanceIdentifier& instance_id, - const std::map>& events, - const std::map>& fields, + const std::map::Reference>>& events, + const std::map::Reference>>& fields, const SkeletonBinding& skeleton_binding) noexcept { std::optional tracing_handler{}; diff --git a/score/mw/com/impl/tracing/skeleton_tracing.h b/score/mw/com/impl/tracing/skeleton_tracing.h index bb8456f1a..87500384a 100644 --- a/score/mw/com/impl/tracing/skeleton_tracing.h +++ b/score/mw/com/impl/tracing/skeleton_tracing.h @@ -19,10 +19,12 @@ #define SCORE_MW_COM_IMPL_SKELETON_TRACING_H #include "score/mw/com/impl/instance_identifier.h" +#include "score/mw/com/impl/reference_to_moveable.h" #include "score/mw/com/impl/skeleton_binding.h" #include "score/mw/com/impl/skeleton_event_base.h" #include "score/mw/com/impl/skeleton_field_base.h" +#include #include #include @@ -31,13 +33,13 @@ namespace score::mw::com::impl::tracing std::optional CreateRegisterShmObjectCallback( const InstanceIdentifier& instance_id, - const std::map>& events, - const std::map>& fields, + const std::map::Reference>>& events, + const std::map::Reference>>& fields, const SkeletonBinding& skeleton_binding) noexcept; std::optional CreateUnregisterShmObjectCallback( const InstanceIdentifier& instance_id, - const std::map>& events, - const std::map>& fields, + const std::map::Reference>>& events, + const std::map::Reference>>& fields, const SkeletonBinding& skeleton_binding) noexcept; } // namespace score::mw::com::impl::tracing diff --git a/score/mw/com/impl/tracing/skeleton_tracing_test.cpp b/score/mw/com/impl/tracing/skeleton_tracing_test.cpp index e987fc35f..1a24f759e 100644 --- a/score/mw/com/impl/tracing/skeleton_tracing_test.cpp +++ b/score/mw/com/impl/tracing/skeleton_tracing_test.cpp @@ -16,6 +16,7 @@ #include "score/mw/com/impl/bindings/mock_binding/skeleton.h" #include "score/mw/com/impl/bindings/mock_binding/skeleton_event.h" #include "score/mw/com/impl/configuration/test/configuration_store.h" +#include "score/mw/com/impl/reference_to_moveable.h" #include "score/mw/com/impl/service_element_type.h" #include "score/mw/com/impl/skeleton_base.h" #include "score/mw/com/impl/skeleton_event_base.h" @@ -67,8 +68,6 @@ class MyDummyField : public SkeletonFieldBase { } - void UpdateSkeletonReference(SkeletonBase& skeleton_base) noexcept override {} - bool IsInitialValueSaved() const noexcept override { return true; @@ -98,8 +97,8 @@ class SkeletonTracingFixture : public ::testing::Test kEventName, std::make_unique>()}, skeleton_field_base_{empty_skeleton_, kFieldName, std::make_unique()}, - events_map_{{kEventName, skeleton_event_base_}}, - fields_map_{{kFieldName, skeleton_field_base_}} + events_map_{{kEventName, skeleton_event_base_ref_.Get()}}, + fields_map_{{kFieldName, skeleton_field_base_ref_.Get()}} { ON_CALL(mock_skeleton_binding_, GetBindingType).WillByDefault(Return(BindingType::kFake)); ON_CALL(runtime_mock_guard_.runtime_mock_, GetTracingRuntime()).WillByDefault(Return(&tracing_runtime_mock_)); @@ -128,10 +127,12 @@ class SkeletonTracingFixture : public ::testing::Test SkeletonBase empty_skeleton_; mock_binding::Skeleton& mock_skeleton_binding_; SkeletonEventBase skeleton_event_base_; + ReferenceToMoveable skeleton_event_base_ref_{skeleton_event_base_}; MyDummyField skeleton_field_base_; + ReferenceToMoveable skeleton_field_base_ref_{skeleton_field_base_}; - std::map> events_map_; - std::map> fields_map_; + std::map::Reference>> events_map_; + std::map::Reference>> fields_map_; }; using SkeletonTracingCreateRegisterShmCallbackFixture = SkeletonTracingFixture; From d92f60bc793fd5e48240864b7d8e105925a9449b Mon Sep 17 00:00:00 2001 From: Brendan Emery Date: Wed, 29 Apr 2026 12:01:21 +0200 Subject: [PATCH 3/8] mw/com: Add helpers for checking method handler signatures --- score/mw/com/impl/methods/BUILD | 55 ++ score/mw/com/impl/methods/callable_traits.cpp | 13 + score/mw/com/impl/methods/callable_traits.h | 79 +++ .../com/impl/methods/callable_traits_impl.h | 72 +++ .../com/impl/methods/callable_traits_test.cpp | 243 ++++++++ .../impl/methods/method_traits_checker.cpp | 13 + .../com/impl/methods/method_traits_checker.h | 206 +++++++ .../methods/method_traits_checker_test.cpp | 550 ++++++++++++++++++ score/mw/com/impl/methods/test/BUILD | 24 + .../test/callable_traits_resources.cpp | 13 + .../methods/test/callable_traits_resources.h | 214 +++++++ 11 files changed, 1482 insertions(+) create mode 100644 score/mw/com/impl/methods/callable_traits.cpp create mode 100644 score/mw/com/impl/methods/callable_traits.h create mode 100644 score/mw/com/impl/methods/callable_traits_impl.h create mode 100644 score/mw/com/impl/methods/callable_traits_test.cpp create mode 100644 score/mw/com/impl/methods/method_traits_checker.cpp create mode 100644 score/mw/com/impl/methods/method_traits_checker.h create mode 100644 score/mw/com/impl/methods/method_traits_checker_test.cpp create mode 100644 score/mw/com/impl/methods/test/BUILD create mode 100644 score/mw/com/impl/methods/test/callable_traits_resources.cpp create mode 100644 score/mw/com/impl/methods/test/callable_traits_resources.h diff --git a/score/mw/com/impl/methods/BUILD b/score/mw/com/impl/methods/BUILD index 241bc0613..21cc6323c 100644 --- a/score/mw/com/impl/methods/BUILD +++ b/score/mw/com/impl/methods/BUILD @@ -28,6 +28,46 @@ cc_library( ], ) +cc_library( + name = "method_traits_checker", + srcs = ["method_traits_checker.cpp"], + hdrs = ["method_traits_checker.h"], + features = COMPILER_WARNING_FEATURES, + tags = ["FFI"], + visibility = [ + "//score/mw/com:__subpackages__", + ], + deps = [ + ":callable_traits", + "@score_baselibs//score/language/futurecpp", + ], +) + +cc_library( + name = "callable_traits", + hdrs = [ + "callable_traits.h", + "callable_traits_impl.h", + ], + features = COMPILER_WARNING_FEATURES, + tags = ["FFI"], + visibility = [ + "//score/mw/com:__subpackages__", + ], + deps = [ + ], +) + +cc_unit_test( + name = "callable_traits_test", + srcs = ["callable_traits_test.cpp"], + deps = [ + ":callable_traits", + "//score/mw/com/impl/methods/test:callable_traits_resources", + "@googletest//:gtest", + ], +) + cc_library( name = "proxy_method_base", srcs = ["proxy_method_base.cpp"], @@ -111,6 +151,20 @@ cc_unit_test( ], ) +cc_unit_test( + name = "method_traits_checker_test", + srcs = ["method_traits_checker_test.cpp"], + features = COMPILER_WARNING_FEATURES + [ + # These tests catch exceptions instead of using gtest EXPECT_DEATH so we disable aborts_upon_exception. + "-aborts_upon_exception", + ], + deps = [ + ":method_traits_checker", + "@googletest//:gtest", + "@score_baselibs//score/language/futurecpp:futurecpp_test_support", + ], +) + cc_unit_test( name = "proxy_method_test", srcs = ["proxy_method_test.cpp"], @@ -187,6 +241,7 @@ cc_unit_test( deps = [ ":skeleton_method", "//score/mw/com/impl/bindings/mock_binding", + "//score/mw/com/impl/methods/test:callable_traits_resources", "//score/mw/com/impl/plumbing:skeleton_method_binding_factory_mock", "//score/mw/com/impl/test:binding_factory_resources", "@score_baselibs//score/language/futurecpp", diff --git a/score/mw/com/impl/methods/callable_traits.cpp b/score/mw/com/impl/methods/callable_traits.cpp new file mode 100644 index 000000000..b874eecf8 --- /dev/null +++ b/score/mw/com/impl/methods/callable_traits.cpp @@ -0,0 +1,13 @@ +/******************************************************************************** + * Copyright (c) 2025 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#include "score/mw/com/impl/methods/method_handler_traits.h" diff --git a/score/mw/com/impl/methods/callable_traits.h b/score/mw/com/impl/methods/callable_traits.h new file mode 100644 index 000000000..8dd8f6f63 --- /dev/null +++ b/score/mw/com/impl/methods/callable_traits.h @@ -0,0 +1,79 @@ +/******************************************************************************** + * Copyright (c) 2025 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#ifndef SCORE_MW_COM_IMPL_METHODS_METHOD_CALLABLE_TRAITS_H +#define SCORE_MW_COM_IMPL_METHODS_METHOD_CALLABLE_TRAITS_H + +#include +#include + +namespace score::mw::com::impl +{ + +/// \brief Implementation of std::remove_cvref which is only available in C++20. +template +using remove_cvref_t = std::remove_cv_t>; + +/// \brief Helper struct to check if a tuple type is empty. +template +struct is_tuple_empty : std::false_type +{ +}; + +template +struct is_tuple_empty == 0U>> : std::true_type +{ +}; + +template +using is_tuple_empty_t = typename is_tuple_empty::type; + +/// \brief Helper struct to extract the tail of a tuple (i.e. all elements except the first one) as a tuple type. +template +struct get_tuple_tail; + +template +struct get_tuple_tail> +{ + using type = std::tuple; +}; + +template +using get_tuple_tail_t = typename get_tuple_tail::type; + +/// \brief Helper struct to extract the in argument types of a callable as a tuple type in member alias `type`. +/// +/// e.g. for a callable of type `int(const double&, int, const std::string), `get_callable_args_types::type` will be `std::tuple`. +/// Full implementation is in callable_traits_impl.h. +template +struct get_callable_args_types : public get_callable_args_types::operator())> +{ +}; + +/// \brief Helper struct to extract the return type of a callable as a type in member alias `type`. +/// +/// e.g. for a callable of type `int(const double&, int, const std::string), `get_callable_return_type::type` will be `int`. +/// Full implementation is in callable_traits_impl.h. +template +struct get_callable_return_type : public get_callable_return_type::operator())> +{ +}; + +} // namespace score::mw::com::impl + +// To avoid keeping the large number of struct specializations for different callable types in the header file, we put +// them in a separate implementation header file. +#include "score/mw/com/impl/methods/callable_traits_impl.h" + +#endif // SCORE_MW_COM_IMPL_METHODS_METHOD_CALLABLE_TRAITS_H diff --git a/score/mw/com/impl/methods/callable_traits_impl.h b/score/mw/com/impl/methods/callable_traits_impl.h new file mode 100644 index 000000000..d95f7f143 --- /dev/null +++ b/score/mw/com/impl/methods/callable_traits_impl.h @@ -0,0 +1,72 @@ +/******************************************************************************** + * Copyright (c) 2025 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#ifndef SCORE_MW_COM_IMPL_METHODS_METHOD_CALLABLE_TRAITS_IMPL_H +#define SCORE_MW_COM_IMPL_METHODS_METHOD_CALLABLE_TRAITS_IMPL_H + +#include +#include + +namespace score::mw::com::impl +{ + +template +struct get_callable_args_types +{ + using type = std::tuple; +}; + +template +struct get_callable_args_types +{ + using type = std::tuple; +}; + +template +struct get_callable_args_types +{ + using type = std::tuple; +}; + +template +struct get_callable_args_types +{ + using type = std::tuple; +}; + +template +struct get_callable_return_type +{ + using type = Ret; +}; + +template +struct get_callable_return_type +{ + using type = Ret; +}; + +template +struct get_callable_return_type +{ + using type = Ret; +}; + +template +struct get_callable_return_type +{ + using type = Ret; +}; + +} // namespace score::mw::com::impl + +#endif // SCORE_MW_COM_IMPL_METHODS_METHOD_CALLABLE_TRAITS_IMPL_H diff --git a/score/mw/com/impl/methods/callable_traits_test.cpp b/score/mw/com/impl/methods/callable_traits_test.cpp new file mode 100644 index 000000000..9a555fc03 --- /dev/null +++ b/score/mw/com/impl/methods/callable_traits_test.cpp @@ -0,0 +1,243 @@ +/******************************************************************************** + * Copyright (c) 2025 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#include "score/mw/com/impl/methods/callable_traits.h" +#include "score/mw/com/impl/methods/test/callable_traits_resources.h" + +#include + +#include + +#include +#include +#include + +namespace score::mw::com::impl +{ + +// Test data structures +struct MyDataStruct +{ + int i; + std::string s; + bool b; +}; + +template +struct ReturnTypeTestData +{ + using CallableType = Callable; + using ExpectedReturnType = ExpectedReturn; +}; + +template +class CallableTraitsReturnTypeFixture : public ::testing::Test +{ + public: + using CallableType = typename ReturnTypeTestData::CallableType; + using ExpectedReturnType = typename ReturnTypeTestData::ExpectedReturnType; +}; + +using MyReturnTypeTestData = ::testing::Types< + ReturnTypeTestData, int>, + ReturnTypeTestData, MyDataStruct>, + ReturnTypeTestData, void>, + ReturnTypeTestData, void>, + + ReturnTypeTestData, int>, + ReturnTypeTestData, MyDataStruct>, + ReturnTypeTestData, void>, + ReturnTypeTestData, void>, + + ReturnTypeTestData, int>, + ReturnTypeTestData, MyDataStruct>, + ReturnTypeTestData, void>, + ReturnTypeTestData, void>, + + ReturnTypeTestData, int>, + ReturnTypeTestData, MyDataStruct>, + ReturnTypeTestData, void>, + ReturnTypeTestData, void>, + + ReturnTypeTestData, int>, + ReturnTypeTestData, MyDataStruct>, + ReturnTypeTestData, void>, + ReturnTypeTestData, void>, + + ReturnTypeTestData, int>, + ReturnTypeTestData, MyDataStruct>, + ReturnTypeTestData, void>, + ReturnTypeTestData, void>, + + ReturnTypeTestData::Create()), int>, + ReturnTypeTestData::Create()), MyDataStruct>, + ReturnTypeTestData::Create()), void>, + ReturnTypeTestData::Create()), void>, + + ReturnTypeTestData::Create()), int>, + ReturnTypeTestData::Create()), MyDataStruct>, + ReturnTypeTestData::Create()), void>, + ReturnTypeTestData::Create()), void>, + + ReturnTypeTestData::Create()), int>, + ReturnTypeTestData::Create()), MyDataStruct>, + ReturnTypeTestData::Create()), void>, + ReturnTypeTestData::Create()), void>, + + ReturnTypeTestData::Create()), int>, + ReturnTypeTestData::Create()), MyDataStruct>, + ReturnTypeTestData::Create()), void>, + ReturnTypeTestData::Create()), void> + + >; +TYPED_TEST_SUITE(CallableTraitsReturnTypeFixture, MyReturnTypeTestData); + +TYPED_TEST(CallableTraitsReturnTypeFixture, GetCallableReturnTypeForCallable) +{ + // When extracting the return type from a callable type + using ActualReturnType = typename get_callable_return_type::type; + + // Then the extracted return type matches the expected return type + EXPECT_TRUE((std::is_same_v)); +} + +template +struct InArgTypesTestData +{ + using CallableType = Callable; + using ExpectedInArgTypes = ExpectedInArgs; +}; + +template +class CallableTraitsInArgTypesFixture : public ::testing::Test +{ + public: + using CallableType = typename InArgTypesTestData::CallableType; + using ExpectedInArgTypes = typename InArgTypesTestData::ExpectedInArgTypes; +}; + +// clang-format off +using MyInArgTypesTestData = ::testing::Types< + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + InArgTypesTestData, std::tuple>, + + InArgTypesTestData::Create()), std::tuple>, + InArgTypesTestData::Create()), std::tuple>, + InArgTypesTestData::Create()), std::tuple>, + InArgTypesTestData::Create()), std::tuple>, + + InArgTypesTestData::Create()), std::tuple>, + InArgTypesTestData::Create()), std::tuple>, + InArgTypesTestData::Create()), std::tuple>, + InArgTypesTestData::Create()), std::tuple>, + + InArgTypesTestData::Create()), std::tuple>, + InArgTypesTestData::Create()), std::tuple>, + InArgTypesTestData::Create()), std::tuple>, + InArgTypesTestData::Create()), std::tuple>, + + InArgTypesTestData::Create()), std::tuple>, + InArgTypesTestData::Create()), std::tuple>, + InArgTypesTestData::Create()), std::tuple>, + InArgTypesTestData::Create()), std::tuple> + >; +// clang-format on + +TYPED_TEST_SUITE(CallableTraitsInArgTypesFixture, MyInArgTypesTestData); + +TYPED_TEST(CallableTraitsInArgTypesFixture, GetCallableInArgTypesForCallable) +{ + // When extracting the InArg types from a callable type + using ActualInArgTypes = typename get_callable_args_types::type; + + // Then the extracted InArg types match the expected InArg types + EXPECT_TRUE((std::is_same_v)); +} + +TEST(CallableTraitsIsTupleEmptyTest, EmptyTupleReturnsTrue) +{ + // When checking if a tuple containg no types is empty + // Then the result is true + EXPECT_TRUE(is_tuple_empty>::value); +} + +TEST(CallableTraitsIsTupleEmptyTest, TupleWithOneArgReturnsFalse) +{ + // When checking if a tuple containg one type is empty + // Then the result is false + EXPECT_FALSE(is_tuple_empty>::value); +} + +TEST(CallableTraitsIsTupleEmptyTest, TupleWithMultipleArgsReturnsFalse) +{ + // When checking if a tuple containg multiple types is empty + // Then the result is false + EXPECT_FALSE((is_tuple_empty>::value)); +} + +TEST(CallableTraitsGetTupleTailTest, GetTupleTailOfSingleElementTupleReturnsEmptyTuple) +{ + // When extracting the tail of a tuple containing one type + using Tail = typename get_tuple_tail>::type; + + // Then the extracted tail is an empty tuple + EXPECT_TRUE((std::is_same_v>)); +} + +TEST(CallableTraitsGetTupleTailTest, GetTupleTailOfMultiElementTupleReturnsTail) +{ + // When extracting the tail of a tuple containing multiple types + using Tail = typename get_tuple_tail>::type; + + // Then the extracted tail is a tuple containing all types except the first one + EXPECT_TRUE((std::is_same_v>)); +} + +// TODO: Tests for invalid callables that should cause static assertion failures. These tests are disabled since we +// currently don't have infrastructure for compile time testing. To be enabled in SWP-46885. +#if 0 +TEST(CallableTraitsGetTupleTailTest, GetTupleTailOfEmptyTupleFailsToCompile) +{ + // When extracting the tail of an empty tuple + // Then we should get a compiler error + using Tail = typename get_tuple_tail>::type; +} +#endif + +} // namespace score::mw::com::impl diff --git a/score/mw/com/impl/methods/method_traits_checker.cpp b/score/mw/com/impl/methods/method_traits_checker.cpp new file mode 100644 index 000000000..ccc28d1eb --- /dev/null +++ b/score/mw/com/impl/methods/method_traits_checker.cpp @@ -0,0 +1,13 @@ +/******************************************************************************** + * Copyright (c) 2025 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#include "score/mw/com/impl/methods/method_traits_checker.h" diff --git a/score/mw/com/impl/methods/method_traits_checker.h b/score/mw/com/impl/methods/method_traits_checker.h new file mode 100644 index 000000000..9235467ea --- /dev/null +++ b/score/mw/com/impl/methods/method_traits_checker.h @@ -0,0 +1,206 @@ +/******************************************************************************** + * Copyright (c) 2025 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#ifndef SCORE_MW_COM_IMPL_METHODS_METHOD_TRAITS_CHECKER_H +#define SCORE_MW_COM_IMPL_METHODS_METHOD_TRAITS_CHECKER_H + +#include "score/mw/com/impl/methods/callable_traits.h" + +#include + +#include +#include +#include + +namespace score::mw::com::impl +{ +namespace detail +{ + +/// \brief Utility to extract the method return type from a tuple of the form `std::tuple` +/// The tuple can be extracter from a callabe using `get_callable_args_types`. +/// +/// \return type of the first element of the tuple (i.e. `ReturnType`) +template +struct get_method_return_type; + +template +struct get_method_return_type> +{ + using type = std::tuple_element_t<0, std::tuple>; +}; + +template +using get_method_return_type_t = typename get_method_return_type::type; + +/// \brief Utility to extract the method in argument types from a tuple of the form `std::tuple` +/// The tuple can be extracter from a callabe using `get_callable_args_types`. +/// +/// \return tuple type `std::tuple` +template +struct get_method_in_args; + +template +struct get_method_in_args> +{ + using type = get_tuple_tail_t>; +}; + +template +using get_method_in_args_t = typename get_method_in_args::type; + +} // namespace detail + +enum class FailureMode +{ + COMPILE_TIME, + RUNTIME +}; + +// TODO: We currently use FailureMode as a workaround to allow testing AssertCallableMatchesMethodSignature at runtime +// since we don't currently have infrastructure for compile time testing. When compile time testing is enabled in +// SWP-46885, we can remove FailureMode and the runtime tests and replace them with compile time tests. +template +void CompileOrRuntimeAssert(const char* message) +{ + if constexpr (failure_mode == FailureMode::COMPILE_TIME) + { + static_assert(condition); + } + else + { + if (!condition) + { + SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(condition, message); + } + } +} + +/// \brief Checks at compile time whether a given callable type matches the expected signature for a method handler. +/// +/// For a method signature of the form `ReturnType(ArgTypes...)`, the expected callable signature is: +/// - If `ReturnType` is not `void` and there are in arguments: +/// `void(ReturnType&, const ArgTypes&...)` +/// - If `ReturnType` is not `void` and there are no in arguments: +/// `void(ReturnType&)` +/// - If `ReturnType` is `void` and there are in arguments: +/// `void(const ArgTypes&...)` +/// - If `ReturnType` is `void` and there are no in arguments: +/// `void()` +template +constexpr void AssertCallableMatchesMethodSignature() +{ + using CallableReturnType = typename get_callable_return_type::type; + CompileOrRuntimeAssert, FailureMode>( + "Registered method callable must have void return type! The actual method return type is passed as " + "first argument to the callable as non-const reference!"); + + using CallableArgs = typename get_callable_args_types::type; + + constexpr bool has_in_args = (sizeof...(ArgTypes) != 0); + constexpr bool has_return_type = !std::is_same_v; + + // Since we want to be able to check assertions at runtime during testing using CompileOrRuntimeAssert, we have to + // avoid compile time assertion failures in other places. Anything in the else branch of a constexpr if statement + // will not be compiled if the condition is false. So we use else branches instead of early returns. Once compile + // time testing is enabled in SWP-46885, we can likely simplify the branching in this code. + if constexpr (!has_in_args && !has_return_type) + { + CompileOrRuntimeAssert>, FailureMode>( + "Registered method callable must not have any arguments since the method signature does not specify " + "any in arguments or return type!"); + } + else + { + if constexpr (is_tuple_empty::value) + { + CompileOrRuntimeAssert::value, FailureMode>( + "Registered method callable must have at least one argument (a return type and/or one or more in " + "args)"); + } + else + { + + if constexpr (has_in_args && has_return_type) + { + using MethodInArgTuple = detail::get_method_in_args_t; + using MethodReturnType = detail::get_method_return_type_t; + + using ExpectedInArgTypeTuple = std::tuple; + using ExpectedReturnType = ReturnType&; + + CompileOrRuntimeAssert, FailureMode>( + "Registered method callable must have the method return type as first argument as non-const " + "reference!"); + CompileOrRuntimeAssert, FailureMode>( + "Registered method callable must have the same in argument types as the method signature " + "following the return type, but with const reference semantics!"); + } + else if constexpr (!has_in_args && has_return_type) + { + using MethodReturnType = detail::get_method_return_type_t; + using ExpectedReturnType = ReturnType&; + + CompileOrRuntimeAssert, FailureMode>( + "Registered method callable must have the method return type as only argument as non-const " + "reference " + "!"); + } + else if constexpr (has_in_args && !has_return_type) + { + using MethodInArgTuple = CallableArgs; + using ExpectedInArgTypeTuple = std::tuple; + + CompileOrRuntimeAssert, FailureMode>( + "Registered method callable must have only the in argument types from the method signature, but " + "with const reference semantics!"); + } + } + } +} + +/// \brief Checks at compile time that the given callable is not a std::bind expression. +/// +/// std::bind expressions make checking the callable signature in AssertCallableMatchesMethodSignature difficult. Since +/// the functionality of std::bind can be achieved using lambdas (which is even recommended over std::bind in general), +/// we disallow std::bind expressions as method handlers. +template +constexpr void AssertMethodCallableIsNotStdBind() +{ + CompileOrRuntimeAssert, FailureMode>( + "std::bind expressions are not allowed as method handlers as they are not supported by " + "AssertCallableMatchesMethodSignature which checks the callable signature. Please use a lambda instead."); +} + +/// \brief Checks at compile time that the method signature does not contain pointers or references in the return type +/// or in args types. +/// +/// Pointers and references in a handler will only be valid in the process which initializes them. Since method return +/// types and in args are placed in shared memory, we disallow pointers and references. +template +constexpr void AssertMethodSignatureDoesNotContainPointersOrReferences() +{ + CompileOrRuntimeAssert, FailureMode>("Method return type must not be a pointer!"); + CompileOrRuntimeAssert, FailureMode>( + "Method return type must not be a reference!"); + + constexpr bool in_args_are_not_pointers = (... && !std::is_pointer_v); + CompileOrRuntimeAssert("Method in args must not be pointers!"); + + constexpr bool in_args_are_not_references = (... && !std::is_reference_v); + CompileOrRuntimeAssert("Method in args must not be references!"); +} + +} // namespace score::mw::com::impl + +#endif // SCORE_MW_COM_IMPL_METHODS_METHOD_TRAITS_CHECKER_H diff --git a/score/mw/com/impl/methods/method_traits_checker_test.cpp b/score/mw/com/impl/methods/method_traits_checker_test.cpp new file mode 100644 index 000000000..abf6f80cb --- /dev/null +++ b/score/mw/com/impl/methods/method_traits_checker_test.cpp @@ -0,0 +1,550 @@ +/******************************************************************************** + * Copyright (c) 2025 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#include "score/mw/com/impl/methods/method_traits_checker.h" + +#include + +#include + +#include + +namespace score::mw::com::impl +{ +namespace +{ + +TEST(MethodTraitsCheckerReturnAndInArgsCompileTimeTest, CallingAssertCallableWithCallableDoesNotTerminate) +{ + // Given a method with a return type and InArgs. + using MethodReturnType = int; + using FirstInArgType = int; + using SecondInArgType = double; + + // and given a callable that accepts the return and InArgs. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the call does not terminate + AssertCallableMatchesMethodSignature(); +} + +TEST(MethodTraitsCheckerReturnOnlyCompileTimeTest, CallingAssertCallableWithCallableDoesNotTerminate) +{ + // Given a method with a return type and no InArgs. + using MethodReturnType = int; + + // and given a callable that accepts the return and no InArgs. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the call does not terminate + AssertCallableMatchesMethodSignature(); +} + +TEST(MethodTraitsCheckerInArgsOnlyCompileTimeTest, CallingAssertCallableWithCallableDoesNotTerminate) +{ + // Given a method with no return type and InArgs. + using MethodReturnType = void; + using FirstInArgType = int; + using SecondInArgType = double; + + // and given a callable that accepts no return and InArgs. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the call does not terminate + AssertCallableMatchesMethodSignature(); +} + +TEST(MethodTraitsCheckerNoReturnAndNoInArgsCompileTimeTest, CallingAssertCallableWithCallableDoesNotTerminate) +{ + // Given a method with no return type and no InArgs. + using MethodReturnType = void; + + // and given a callable that accepts no return and no InArgs. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the call does not terminate + AssertCallableMatchesMethodSignature(); +} + +TEST(MethodTraitsCheckerGetMethodReturnType, CallingWithTupleContainingOneElementReturnsFirstElement) +{ + using ExpectedMethodReturnType = int; + using MethodArgsTuple = std::tuple; + + // When extracting the method return type from a tuple containing a single element + using ActualMethodReturnType = detail::get_method_return_type_t; + + // Then the extracted method return type is the same as the first element of the tuple + EXPECT_TRUE((std::is_same_v)); +} + +TEST(MethodTraitsCheckerGetMethodReturnType, CallingWithTupleContainingMultipleElementsReturnsFirstElement) +{ + using ExpectedMethodReturnType = int; + using MethodArgsTuple = std::tuple; + + // When extracting the method return type from a tuple containing multiple elements + using ActualMethodReturnType = detail::get_method_return_type_t; + + // Then the extracted method return type is the same as the first element of the tuple + EXPECT_TRUE((std::is_same_v)); +} + +// TODO: Tests for invalid callables that should cause static assertion failures. These tests are disabled since we +// currently don't have infrastructure for compile time testing. To be enabled in SWP-46885. +#if 0 + +TEST(MethodTraitsCheckerGetMethodReturnType, CallingWithEmptyTupleFailsToCompile) +{ + // When extracting the method return type from an empty tuple + // Then we should get a compiler error + using ActualMethodReturnType = detail::get_method_return_type_t>; +} + +TEST(MethodTraitsCheckerGetMethodReturnType, CallingWithCallableFailsToCompile) +{ + // When extracting the method return type from a callable type + // Then we should get a compiler error + using ActualMethodReturnType = detail::get_method_return_type_t>; +} + +#endif + +TEST(MethodTraitsCheckerGetMethodInArgType, CallingWithTupleContainingOneElementReturnsEmptyTuple) +{ + using MethodArgsTuple = std::tuple; + + // When extracting the method in argument types from a tuple containing a single element + using ActualMethodInArgTypes = detail::get_method_in_args_t; + + // Then the extracted method in argument types is an empty tuple (since the first argument of the tuple is the + // return type) + EXPECT_TRUE((std::is_same_v>)); +} + +TEST(MethodTraitsCheckerGetMethodInArgType, CallingWithTupleContainingMultipleElementsReturnsTupleTail) +{ + using ExpectedMethodInArgTypes = std::tuple; + using MethodArgsTuple = std::tuple; + + // When extracting the method in argument types from a tuple containing multiple elements + using ActualMethodInArgTypes = detail::get_method_in_args_t; + + // Then the extracted method in argument types is a tuple containing all elements of the original tuple except the + // first element (since the first element of the tuple is the return type) + EXPECT_TRUE((std::is_same_v)); +} + +// TODO: Tests for invalid callables that should cause static assertion failures. These tests are disabled since we +// currently don't have infrastructure for compile time testing. To be enabled in SWP-46885. +#if 0 + +TEST(MethodTraitsCheckerGetMethodInArgType, CallingWithEmptyTupleFailsToCompile) +{ + // When extracting the method in argument types from an empty tuple + // Then we should get a compiler error + using ActualMethodInArgTypes = detail::get_method_in_args_t>; +} + +TEST(MethodTraitsCheckerGetMethodInArgType, CallingWithCallableFailsToCompile) +{ + // When extracting the method in argument types from a callable type + // Then we should get a compiler error + using ActualMethodInArgTypes = detail::get_method_in_args_t>; +} + +#endif + +// TODO: We currently use FailureMode as a workaround to allow testing AssertCallableMatchesMethodSignature at runtime +// since we don't currently have infrastructure for compile time testing. When compile time testing is enabled in +// SWP-46885, we can remove FailureMode and the runtime tests and replace them with compile time tests. +TEST(MethodTraitsCheckerReturnAndInArgsRuntimeTest, CallingAssertCallableWithCallableDoesNotTerminate) +{ + // Given a method with a return type and InArgs. + using MethodReturnType = int; + using FirstInArgType = int; + using SecondInArgType = double; + + // and given a callable that accepts the return and InArgs. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the call does not terminate + AssertCallableMatchesMethodSignature(); +} + +TEST(MethodTraitsCheckerReturnAndInArgsRuntimeTest, CallingAssertCallableWithCallableWithMissingReturnTerminates) +{ + // Given a method with a return type and InArgs. + using MethodReturnType = int; + using FirstInArgType = int; + using SecondInArgType = double; + + // and given a callable that accepts only the InArgs. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the function terminates with a contract violation + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED((AssertCallableMatchesMethodSignature())); +} + +TEST(MethodTraitsCheckerReturnAndInArgsRuntimeTest, CallingAssertCallableWithCallableWithMissingInArgTerminates) +{ + // Given a method with a return type and InArgs. + using MethodReturnType = int; + using FirstInArgType = int; + using SecondInArgType = double; + + // and given a callable that accepts only the return and only one of the InArgs. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the function terminates with a contract violation + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED((AssertCallableMatchesMethodSignature())); +} + +TEST(MethodTraitsCheckerReturnAndInArgsRuntimeTest, CallingAssertCallableWithCallableWithConstReturnTypeRefTerminates) +{ + // Given a method with a return type and InArgs. + using MethodReturnType = int; + using FirstInArgType = int; + using SecondInArgType = double; + + // and given a callable that accepts the correct arguments but the return type is a const reference instead of + // non-const reference. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the function terminates with a contract violation + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED((AssertCallableMatchesMethodSignature())); +} + +TEST(MethodTraitsCheckerReturnAndInArgsRuntimeTest, CallingAssertCallableWithCallableWithNonConstInArgRefTerminates) +{ + // Given a method with a return type and InArgs. + using MethodReturnType = int; + using FirstInArgType = int; + using SecondInArgType = double; + + // and given a callable that accepts the correct arguments but one of the InArgs is a non-const reference instead of + // a const reference. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the function terminates with a contract violation + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED((AssertCallableMatchesMethodSignature())); +} + +TEST(MethodTraitsCheckerReturnOnlyRuntimeTest, CallingAssertCallableWithCallableDoesNotTerminate) +{ + // Given a method with a return type and no InArgs. + using MethodReturnType = int; + + // and given a callable that accepts the return and no InArgs. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the call does not terminate + AssertCallableMatchesMethodSignature(); +} + +TEST(MethodTraitsCheckerReturnOnlyRuntimeTest, CallingAssertCallableWithCallableWithMissingReturnTerminates) +{ + // Given a method with a return type and no InArgs. + using MethodReturnType = int; + + // and given a callable that accepts no return and no InArgs. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the function terminates with a contract violation + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED( + (AssertCallableMatchesMethodSignature())); +} + +TEST(MethodTraitsCheckerReturnOnlyRuntimeTest, CallingAssertCallableWithCallableWithConstReturnTypeRefTerminates) +{ + // Given a method with a return type and no InArgs. + using MethodReturnType = int; + + // and given a callable that accepts the return type as const reference instead of non-const reference. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the function terminates with a contract violation + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED( + (AssertCallableMatchesMethodSignature())); +} + +TEST(MethodTraitsCheckerInArgsOnlyRuntimeTest, CallingAssertCallableWithCallableDoesNotTerminate) +{ + // Given a method with no return type and InArgs. + using MethodReturnType = void; + using FirstInArgType = int; + using SecondInArgType = double; + + // and given a callable that accepts no return and InArgs. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the call does not terminate + AssertCallableMatchesMethodSignature(); +} + +TEST(MethodTraitsCheckerInArgsOnlyRuntimeTest, CallingAssertCallableWithCallableWithMissingInArgTerminates) +{ + // Given a method with no return type and InArgs. + using MethodReturnType = void; + using FirstInArgType = int; + using SecondInArgType = double; + + // and given a callable that accepts only one InArg. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the function terminates with a contract violation + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED((AssertCallableMatchesMethodSignature())); +} + +TEST(MethodTraitsCheckerInArgsOnlyRuntimeTest, CallingAssertCallableWithCallableWithNonConstInArgRefTerminates) +{ + // Given a method with no return type and InArgs. + using MethodReturnType = void; + using FirstInArgType = int; + using SecondInArgType = double; + + // and given a callable that accepts one of the InArgs as non-const reference instead of const reference. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the function terminates with a contract violation + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED((AssertCallableMatchesMethodSignature())); +} + +TEST(MethodTraitsCheckerNoReturnAndNoInArgsRuntimeTest, CallingAssertCallableWithCallableDoesNotTerminate) +{ + // Given a method with no return type and no InArgs. + using MethodReturnType = void; + + // and given a callable that accepts no return and no InArgs. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the call does not terminate + AssertCallableMatchesMethodSignature(); +} + +TEST(MethodTraitsCheckerNoReturnAndNoInArgsRuntimeTest, CallingAssertCallableWithCallableWithInArgTerminates) +{ + // Given a method with no return type and no InArgs. + using MethodReturnType = void; + + // and given a callable that accepts an InArg. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the function terminates with a contract violation + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED( + (AssertCallableMatchesMethodSignature())); +} + +TEST(MethodTraitsCheckerNoReturnAndNoInArgsRuntimeTest, CallingAssertCallableWithCallableWithReturnTypeTerminates) +{ + // Given a method with no return type and no InArgs. + using MethodReturnType = void; + + // and given a callable that returns a non-void value. + using Callable = std::function; + + // When calling AssertCallableMatchesMethodSignature + // Then the function terminates with a contract violation + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED( + (AssertCallableMatchesMethodSignature())); +} + +TEST(MethodTraitsCheckerAssertMethodCallableIsNotStdBindCompileTimeTest, CallingWithNonStdBindCallableDoesNotTerminate) +{ + // When calling AssertMethodCallableIsNotStdBind with a non-std::bind type + // Then the call does not terminate + auto non_std_bind_callable = []() {}; + AssertMethodCallableIsNotStdBind(); +} + +TEST(MethodTraitsCheckerAssertMethodCallableIsNotStdBindRuntimeTimeTest, CallingWithNonStdBindCallableDoesNotTerminate) +{ + // When calling AssertMethodCallableIsNotStdBind with a non-std::bind type + // Then the call does not terminate + auto non_std_bind_callable = []() {}; + AssertMethodCallableIsNotStdBind(); +} + +TEST(MethodTraitsCheckerAssertMethodCallableIsNotStdBindRuntimeTimeTest, CallingWithStdBindTypeTerminates) +{ + // When calling AssertMethodCallableIsNotStdBind with a std::bind type + // Then the function terminates with a contract violation + auto std_bind_expression = std::bind([]() {}); + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED( + (AssertMethodCallableIsNotStdBind())); +} + +TEST(AssertMethodSignatureDoesNotContainPointersOrReferencesCompileTimeTest, + CallingWithSignatureWithNoPointerOrReferenceArgsDoesNotTerminate) +{ + // Given a method with no pointer or reference arguments. + using MethodReturnType = int; + using FirstInArgType = double; + using SecondInArgType = std::string; + + // When calling AssertMethodSignatureDoesNotContainPointersOrReferences with a method signature where no argument is + // a pointer or reference + // Then the call does not terminate + AssertMethodSignatureDoesNotContainPointersOrReferences(); +} + +TEST(AssertMethodSignatureDoesNotContainPointersOrReferencesRuntimeTimeTest, + CallingWithSignatureWithNoPointerOrReferenceArgsDoesNotTerminate) +{ + // Given a method with no pointer or reference arguments. + using MethodReturnType = int; + using FirstInArgType = double; + using SecondInArgType = std::string; + + // When calling AssertMethodSignatureDoesNotContainPointersOrReferences with a method signature where no argument is + // a pointer or reference + // Then the call does not terminate + AssertMethodSignatureDoesNotContainPointersOrReferences(); +} + +TEST(AssertMethodSignatureDoesNotContainPointersOrReferencesRuntimeTimeTest, + CallingWithSignatureWithPointerArgTerminates) +{ + // Given a method with a pointer argument. + using MethodReturnType = int; + using FirstInArgType = double*; + using SecondInArgType = std::string; + + // When calling AssertMethodSignatureDoesNotContainPointersOrReferences with a method signature where one of the + // arguments is a pointer + // Then the function terminates with a contract violation + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED( + (AssertMethodSignatureDoesNotContainPointersOrReferences())); +} + +TEST(AssertMethodSignatureDoesNotContainPointersOrReferencesRuntimeTimeTest, + CallingWithSignatureWithPointerReturnTerminates) +{ + // Given a method with a pointer return type. + using MethodReturnType = double*; + using FirstInArgType = int; + using SecondInArgType = std::string; + + // When calling AssertMethodSignatureDoesNotContainPointersOrReferences with a method signature where the return + // type is a pointer + // Then the function terminates with a contract violation + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED( + (AssertMethodSignatureDoesNotContainPointersOrReferences())); +} + +TEST(AssertMethodSignatureDoesNotContainPointersOrReferencesRuntimeTimeTest, + CallingWithSignatureWithReferenceArgTerminates) +{ + // Given a method with a reference argument. + using MethodReturnType = int; + using FirstInArgType = double&; + using SecondInArgType = std::string; + + // When calling AssertMethodSignatureDoesNotContainPointersOrReferences with a method signature where one of the + // arguments is a reference + // Then the function terminates with a contract violation + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED( + (AssertMethodSignatureDoesNotContainPointersOrReferences())); +} + +TEST(AssertMethodSignatureDoesNotContainPointersOrReferencesRuntimeTimeTest, + CallingWithSignatureWithReferenceReturnTerminates) +{ + // Given a method with a reference return type. + using MethodReturnType = double&; + using FirstInArgType = int; + using SecondInArgType = std::string; + + // When calling AssertMethodSignatureDoesNotContainPointersOrReferences with a method signature where the return + // type is a reference + // Then the function terminates with a contract violation + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED( + (AssertMethodSignatureDoesNotContainPointersOrReferences())); +} + +} // namespace +} // namespace score::mw::com::impl diff --git a/score/mw/com/impl/methods/test/BUILD b/score/mw/com/impl/methods/test/BUILD new file mode 100644 index 000000000..770ae9ead --- /dev/null +++ b/score/mw/com/impl/methods/test/BUILD @@ -0,0 +1,24 @@ +# ******************************************************************************* +# Copyright (c) 2025 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* + +load("@rules_cc//cc:defs.bzl", "cc_library") +load("//score/mw:common_features.bzl", "COMPILER_WARNING_FEATURES") + +cc_library( + name = "callable_traits_resources", + hdrs = ["callable_traits_resources.h"], + features = COMPILER_WARNING_FEATURES, + visibility = [ + "//score/mw/com/impl/methods:__pkg__", + ], +) diff --git a/score/mw/com/impl/methods/test/callable_traits_resources.cpp b/score/mw/com/impl/methods/test/callable_traits_resources.cpp new file mode 100644 index 000000000..00659a00d --- /dev/null +++ b/score/mw/com/impl/methods/test/callable_traits_resources.cpp @@ -0,0 +1,13 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#include "score/mw/com/impl/methods/test/callable_traits_resources.h" diff --git a/score/mw/com/impl/methods/test/callable_traits_resources.h b/score/mw/com/impl/methods/test/callable_traits_resources.h new file mode 100644 index 000000000..63f17b091 --- /dev/null +++ b/score/mw/com/impl/methods/test/callable_traits_resources.h @@ -0,0 +1,214 @@ +/******************************************************************************** + * Copyright (c) 2025 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#ifndef SCORE_MW_COM_IMPL_METHODS_TEST_CALLABLE_TRAITS_RESOURCES_H +#define SCORE_MW_COM_IMPL_METHODS_TEST_CALLABLE_TRAITS_RESOURCES_H + +namespace score::mw::com::impl +{ + +/// \brief Factory that constructs a default-initialised CallableWrapper via a static Create(). +/// +/// \tparam CallableWrapper A template-template parameter such as score::cpp::callback or std::function. +/// \tparam HandlerSignature The concrete function signature passed to CallableWrapper, e.g. void(int&). +template