Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,65 @@ struct vector
struct optional
{
};
// Distinct tag for enum-typed fields whose enumerator name table is known via a specialization
// of ::score::common::visitor::EnumTraits<T>. Carries the same wire-level integer encoding as
// signed_le / unsigned_le (see enum_serialized_descriptor::wire_tag) but lets downstream
// consumers (e.g. FIBEX generation) detect "this is a named enum" instead of falling through to
// the generic integral overloads.
struct enum_le
{
};

} // namespace payload_tags

// One value <-> name mapping of an enumerator, as declared in the originating proto `enum`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is misleading. This library is independent from proto. This might be you use case, but it is generally not correct to use this as the only use case of this library.

struct Enumerator
{
std::int64_t value;
const char* name;
};

// Per-enum-type metadata carrier, used by the serialization visitor and, downstream, by FIBEX
// generation to recover the enumerator name<->value table that would otherwise be lost once the
// enum is folded into a plain integral wire type.
//
// The primary (unspecialized) template intentionally carries no names: any C++ enum works with
// the existing signed/unsigned integral serialization path unless a specialization of this trait
// is provided for it. Code generators are expected to emit a specialization for every proto

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, the whole proto description is not relevant for this library in S-CORE.

The general comment, about the difference between how "specialized" enums vs regular enums are handle suffices.

// `enum` they generate, reusing the enumerator list they already have in memory during codegen.
// This is a compile-time-only, zero-runtime-cost side channel: it does not alter the wire format
// of the enum itself.
template <typename T>
struct EnumTraits
{
static constexpr bool kHasNames = false;
};

namespace details
{
/// \brief Compile-time predicate for "is T an enum with a signed underlying type?".
///
/// std::is_signed<T> is only meaningful for arithmetic types and is always false for enum types
/// (enums are not arithmetic types per the standard), even when their underlying type is signed.
/// This trait instead inspects the enum's underlying type, and is only instantiated for enum
/// types (via the bool non-type template parameter) so it stays safe to use unconditionally in
/// enable_if_t expressions alongside is_enum<T> for non-enum T.
/// \tparam T Candidate type; only its is_enum-ness is inspected here (this primary/false-type
/// template is selected for all non-enum T).
template <typename T, bool = std::is_enum<T>::value>
struct is_signed_enum : std::false_type
{
};

/// \brief Specialization selected for enum types: inherits from
/// std::is_signed<std::underlying_type_t<T>>, i.e. ::value is true iff T's underlying
/// integer type is signed.
/// \tparam T Enum type to inspect.
template <typename T>
struct is_signed_enum<T, true> : std::is_signed<typename std::underlying_type<T>::type>
{
};

template <typename S, typename T>
auto cast_to_source_serializable_data_span(const T* data, size_t size) -> score::cpp::span<const S>
{
Expand Down Expand Up @@ -972,6 +1026,21 @@ struct memcpy_serialized_descriptor
using payload_type = memcpy_serialized<sizeof(T)>;
};

// Descriptor for enum-typed fields whose enumerator names are known via EnumTraits<T>. Keeps the
// same on-wire integer representation as memcpy_serialized_descriptor (Tag is signed_le or
// unsigned_le, matching the enum's underlying signedness) but exposes the original enum type so
// that downstream consumers (e.g. the FIBEX generator's visitor) can look up EnumTraits<T> and
// recover the enumerator name<->value table. This does not change the serialized byte layout.
template <typename Tag, typename T>
// coverity[autosar_cpp14_a11_0_2_violation]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No coverity in S-CORE repository

struct enum_serialized_descriptor
{
using payload_tag = payload_tags::enum_le;
using payload_type = memcpy_serialized<sizeof(T)>;
using wire_tag = Tag;
using enum_type = T;
};

template <typename A,
typename T,
std::enable_if_t<(std::is_integral<T>::value) && (std::is_signed<T>::value), std::int32_t> = 0>
Expand Down Expand Up @@ -1003,9 +1072,34 @@ inline auto visit_as(serialized_visitor<A>& /*unused*/, T& /*unused*/)
return memcpy_serialized_descriptor<payload_tags::ieee754_float_le, T>();
}

template <typename A,
typename T,
std::enable_if_t<(std::is_enum<T>::value) && (std::is_signed<T>::value), std::int32_t> = 0>
template <
typename A,
typename T,
std::enable_if_t<(std::is_enum<T>::value) && (details::is_signed_enum<T>::value) && (EnumTraits<T>::kHasNames),
std::int32_t> = 0>
/// \brief Dispatches a signed enum with a registered EnumTraits<T> specialization to the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually the comments go before the template< ... > line

/// enum-aware descriptor.
/// \param T Enum type with a signed underlying type and EnumTraits<T>::kHasNames == true.
/// \return enum_serialized_descriptor<payload_tags::signed_le, T>, i.e. the same on-wire
/// signed integer representation as memcpy_serialized_descriptor, but additionally
/// exposing T so that EnumTraits<T> can be looked up by downstream consumers.
// This is false positive, Overload signatures are different.
// coverity[autosar_cpp14_m3_2_3_violation : FALSE]
// coverity[autosar_cpp14_a2_10_4_violation : FALSE]
Comment on lines +1087 to +1088

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, coverity

inline auto visit_as(serialized_visitor<A>& /*unused*/, T& /*unused*/)
{
return enum_serialized_descriptor<payload_tags::signed_le, T>();
}

template <
typename A,
typename T,
std::enable_if_t<(std::is_enum<T>::value) && (details::is_signed_enum<T>::value) && (!EnumTraits<T>::kHasNames),
std::int32_t> = 0>
/// \brief Dispatches a signed enum without a registered EnumTraits<T> specialization to the
/// plain integral descriptor (previous behavior, unchanged).
/// \param T Enum type with a signed underlying type and EnumTraits<T>::kHasNames == false.
/// \return memcpy_serialized_descriptor<payload_tags::signed_le, T>.
// This is false positive, Overload signatures are different.
// coverity[autosar_cpp14_m3_2_3_violation : FALSE]
// coverity[autosar_cpp14_a2_10_4_violation : FALSE]
Expand All @@ -1014,9 +1108,34 @@ inline auto visit_as(serialized_visitor<A>& /*unused*/, T& /*unused*/)
return memcpy_serialized_descriptor<payload_tags::signed_le, T>();
}

template <typename A,
typename T,
std::enable_if_t<(std::is_enum<T>::value) && (!std::is_signed<T>::value), std::int32_t> = 0>
template <
typename A,
typename T,
std::enable_if_t<(std::is_enum<T>::value) && (!details::is_signed_enum<T>::value) && (EnumTraits<T>::kHasNames),
std::int32_t> = 0>
/// \brief Dispatches an unsigned enum with a registered EnumTraits<T> specialization to the
/// enum-aware descriptor.
/// \param T Enum type with an unsigned underlying type and EnumTraits<T>::kHasNames == true.
/// \return enum_serialized_descriptor<payload_tags::unsigned_le, T>, i.e. the same on-wire
/// unsigned integer representation as memcpy_serialized_descriptor, but additionally
/// exposing T so that EnumTraits<T> can be looked up by downstream consumers.
// This is false positive, Overload signatures are different.
// coverity[autosar_cpp14_m3_2_3_violation : FALSE]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More coverity here

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its fine for documentation of intent w.r.t to suppression until a new tool is introduced where the suppression could be removed OR carried over.

// coverity[autosar_cpp14_a2_10_4_violation : FALSE]
inline auto visit_as(serialized_visitor<A>& /*unused*/, T& /*unused*/)
{
return enum_serialized_descriptor<payload_tags::unsigned_le, T>();
}

template <
typename A,
typename T,
std::enable_if_t<(std::is_enum<T>::value) && (!details::is_signed_enum<T>::value) && (!EnumTraits<T>::kHasNames),
std::int32_t> = 0>
/// \brief Dispatches an unsigned enum without a registered EnumTraits<T> specialization to the
/// plain integral descriptor (previous behavior, unchanged).
/// \param T Enum type with an unsigned underlying type and EnumTraits<T>::kHasNames == false.
/// \return memcpy_serialized_descriptor<payload_tags::unsigned_le, T>.
// This is false positive, Overload signatures are different.
// coverity[autosar_cpp14_m3_2_3_violation : FALSE]
// coverity[autosar_cpp14_a2_10_4_violation : FALSE]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -971,4 +971,66 @@ TEST(clear_functionality_test, test_that_clear_function_can_clear_vector_of_int3
score::common::visitor::detail::clear(vector_wrapper_instance);
}

// Enum used to verify that registering ::score::common::visitor::EnumTraits<T> only adds
// compile-time enumerator name metadata and leaves the serialized wire representation unchanged.
enum class EnumWithNames : std::int32_t
{
kFirst = 0,
kSecond = 1,
};

} // namespace test

template <>
struct score::common::visitor::EnumTraits<test::EnumWithNames>
{
static constexpr bool kHasNames = true;
// Enumerator name<->value table, as a real code generator would emit it, so the test can
// verify that names are actually retrievable through the specialized trait (not just that
// kHasNames is set).
static constexpr std::array<score::common::visitor::Enumerator, 2> kEnumerators = {{
{static_cast<std::int64_t>(test::EnumWithNames::kFirst), "kFirst"},
{static_cast<std::int64_t>(test::EnumWithNames::kSecond), "kSecond"},
}};
};

TEST(serializer_visitor, named_enum_wire_format_unchanged)
{
RecordProperty("ParentRequirement", "SCR-1633893");
RecordProperty("ASIL", "B");
RecordProperty("Description",
"Check that registering EnumTraits<T> for an enum type only adds compile-time "
"enumerator name metadata and does not alter its serialized wire representation.");
RecordProperty("TestingTechnique", "Requirements-based test");
RecordProperty("DerivationTechnique", "requirements-analysis"); // requirements
Comment on lines +999 to +1005

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of this makes sense in S-CORE, does it @4og ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It actually does, but the values are slightly different in S-CORE. The best is to see them in this markdown: https://github.com/eclipse-score/baselibs/blob/main/.agents/skills/requirements-management-skill/test-to-requirement-linking.md

We currently have an epic to clean all of this up across the codebase: #522

For this PR, I would just make the Description, TestingTechnique, DerivationTechnique aligned with the S-CORE guidelines (basically, only the TestingTechnique needs to be turned to ("TestType", "requirements-based"). For the rest I don't care for now.


static_assert(::score::common::visitor::EnumTraits<test::EnumWithNames>::kHasNames,
"EnumTraits<test::EnumWithNames>::kHasNames must be true once specialized");
static_assert(!::score::common::visitor::EnumTraits<test::E>::kHasNames,
"EnumTraits<T>::kHasNames must default to false for enums without a registered trait");

// Wire format (serialized size) must stay identical to the plain integral representation.
EXPECT_EQ(check_serialized<test::EnumWithNames>(), sizeof(test::EnumWithNames));

using Descriptor = decltype(::score::common::visitor::visit_as(
std::declval<::score::common::visitor::serialized_visitor<alloc_t>&>(), std::declval<test::EnumWithNames&>()));
static_assert(
std::is_same<typename Descriptor::payload_tag, ::score::common::visitor::payload_tags::enum_le>::value,
"an enum with a registered EnumTraits<T> specialization must dispatch to the enum_le payload tag");
static_assert(std::is_same<typename Descriptor::enum_type, test::EnumWithNames>::value,
"enum_serialized_descriptor must expose the original enum type");
// test::EnumWithNames has a signed underlying type (std::int32_t): the wire tag must be
// signed_le, not unsigned_le (regression check for is_signed being applied to the enum type
// itself rather than its underlying type).
static_assert(std::is_same<typename Descriptor::wire_tag, ::score::common::visitor::payload_tags::signed_le>::value,
"a named enum with a signed underlying type must use the signed_le wire tag");

// The whole point of EnumTraits<T> is to make the enumerator name<->value table retrievable
// through the trait, so verify the actual entries, not just that kHasNames is set.
const auto& enumerators = ::score::common::visitor::EnumTraits<test::EnumWithNames>::kEnumerators;
ASSERT_EQ(enumerators.size(), 2U);
EXPECT_EQ(enumerators[0].value, static_cast<std::int64_t>(test::EnumWithNames::kFirst));
EXPECT_STREQ(enumerators[0].name, "kFirst");
EXPECT_EQ(enumerators[1].value, static_cast<std::int64_t>(test::EnumWithNames::kSecond));
EXPECT_STREQ(enumerators[1].name, "kSecond");
}
Loading