Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added etl::unaligned_type_ext #1003

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

rolandreichweinbmw
Copy link

Implemented etl::unaligned_type_ext as discussed in #999.

//*************************************************************************
template <size_t Size_>
template <size_t Size_, typename StorageContainer_>
Copy link
Contributor

Choose a reason for hiding this comment

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

I see you are using CRTP to implement the changes.
The purpose of the base classes is to reduce code bloat by grouping functionality that only depends on a subset of the top level class's template parameters.
unaligned_type_common only depended on the size of the unaligned type, so it would be common to all types/endianness that had the same size.
By adding typename StorageContainer_ the base class template becomes unique to every type and endianness permutation.

Copy link
Author

Choose a reason for hiding this comment

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

Please note that StorageContainer here is not the arithmetic type but only to differentiate between internal and external storage, i.e. unaligned_type and unaligned_type_ext.

This way, the actual arithmetic type is only instantiated for individual unaligned_type's and unaligned_type_ext's (see unaligned_type_base), as originally intended by the base class unaligned_type_common.

An alternative would be to implement unaligned_type_ext separate with much redundancy to unaligned_type, and the result of separate instantiations would basically the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue is that StorageContainer could be a variety of unique template realisations, for example:-

etl::unaligned_type<int32_t, etl::endian::little>
etl::unaligned_type<int32_t, etl::endian::big>
etl::unaligned_type<uint32_t, etl::endian::little>
etl::unaligned_type<uint32_t, etl::endian::big>

These are all unique classes, but they require the exact same storage size.

For the proposed solution, the above types would generate four unique instantiations of the template etl::unaligned_type_common.

etl::unaligned_type_common<sizeof(int32_t), etl::unaligned_type<int32_t, etl::endian::little>>
etl::unaligned_type_common<sizeof(int32_t), etl::unaligned_type<int32_t, etl::endian::big>>
etl::unaligned_type_common<sizeof(uint32_t), etl::unaligned_type<int32_t, etl::endian::little>>
etl::unaligned_type_common<sizeof(uint32_t), etl::unaligned_type<int32_t, etl::endian::big>>

I've been giving it some thought over the last few days, as I liked the CRTP idea, but wanted to try to reduce the number of templated types that may be generated.
A solution that I've been experimenting with is the separation the storage and common functionality into their own template classes.

The storage template has two versions; one which declares an array of storage_type, for internal storage, and another that declares pointer to storage_type, for external storage. Both are templated on storage size only.

template <size_t Size>
class unaligned_type_storage; // Declares storage_type storage[Size];

template <size_t Size>
class unaligned_type_storage_ext; // Declares storage_type* storage;

etl::unaligned_type_common is now a CRTP style template class that expects a storage class.

etl::unaligned_type_storage inherits from etl::unaligned_type_common<etl::unaligned_type_storage<Size>>
etl::unaligned_type_storage_ext inherits from etl::unaligned_type_common<etl::unaligned_type_storage_ext<Size>>

This means that etl::unaligned_type_common is only effectively specialised by size alone, as in the original ETL code.
It can access the storage in the usual CRTP fashion by taking advantage of the fact that arrays decay to pointers.

static_cast<derived_type*>(this)->storage

These storage classes become the bases for the top level etl::unaligned_type and etl::unaligned_type_ext.

template <typename T, int Endian_>
class unaligned_type : public private_unaligned_type::unaligned_type_storage<sizeof(T)>
template <typename T, int Endian_>
class unaligned_type_ext : public private_unaligned_type::unaligned_type_storage_ext<sizeof(T)>

In my current version of the code I've also refactored the unaligned_copy classes by extracting them out of etl::unaligned_type_common to become traits that are inherited by etl::unaligned_type and etl::unaligned_type_ext.

It's possible that this technique could be applied to other _ext containers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants