Place a typed pointer instead of a bool into etl::typed_storage

[thomas-herrmann-1]

Hi! in OpenBSW project (https://github.com/eclipse-openbsw/openbsw) we've lately replaced ESTD with ETL library. As part of the migration we were replacing estd::typed_mem with etl::typed_storage. We're using this class for wrapping our main configuration components (we're calling them lifecycle components) that build up the system. This helps us to e.g. avoid the compiler to generate unwanted default destructor code and might in future also help to determine and control more precisely the point in time when actually such components are created.

Unfortunately the current implementation of etl::typed_storage is not really helpful for debugging the wrapped typed instances nor is it easy for performing detailed RAM analysis (how big are the members of the wrapped structs) on such binary blobs. This is how variables are displayed in an Eclipse debug IDE for ARM gcc:

The idea is now to replace member valid (type bool) with a member value which is a typed pointer that is set once there's a valid instance. The variable would then be displayed like:

Sure, the pointer member is obviously bigger. But on 32-bit architectures this might, due to required alignment, not necessarily lead to more RAM consumption. Therefore, what do you think about this idea?

[jwellbelove]

Can Eclipse show the return value of a function?

struct S
{
  S(int i_, char c_)
   : i(i_)
   , c(c_)
  {
  }

  int i;
  char c;
};

etl::typed_storage<S> a;
a.create(123, 4);

Does setting a watch to a.has_value() show the value true?

I was thinking in terms of adding a

const_pointer ptr() const
{
  return has_value() ? data.template get_address<value_type>() : ETL_NULLPTR;
}

and then watching a.ptr()

[thomas-herrmann-1]

Just tried out. It doesn't seem Eclipse can evaluate this expression.

And for me this is more an example for the problem I see: there's no (pointer/reference) variable or member of the intended type. Therefore there's always some additional (somehow manual) info required in order to make the typed view of the binary blob visible. We have the same problem when e.g. applying some RAM analysis tool (checking elf files) on such variables. The tool can automatically find out about the size and structure of class members (or in case of pointer/reference members - the types they're referencing). This gets much harder if it's a result type of a (mostly inlined) method that actually might refer to the desired type to analyze.

What would be your concerns to use a pointer instead of a bool?

[jwellbelove]

My only issue would be the possibility of a larger memory footprint.

[thomas-herrmann-1]

Yes, for some platforms a potentially larger footprint. Which would only be significant in case of small wrapped types.

But I've another idea: typed_storage is very similar to optional but without calling the contained object's destructor from the destructor. So if typed_storage would have the same/similar internal representation then the contained union would give access to the typed object.

The slight difference between these classes would then only be the avoidable destructor code for typed_storage.

[jwellbelove]

That sounds like an interesting idea. I'll take a look at that tomorrow.

[jwellbelove]

I've modified typed_storage to to use a union, and it seem to work well.
I've also modified the code slightly to make calling create twice a non-error, and made calling destroy on a valueless typed_storage a 'do nothing'.
I've also added an 'emplace' constructor.

etl-#1170-typed_storage-25-08-26.patch

[thomas-herrmann-1]

Wow, that was fast! Thanks for working on that idea.

I've just tried out your patch. Compared to the previous version I now face the problem that with the new code a (re)creation is supported which again requires an assignment operator to be defined for the underlying type. So in my case it didn't compile due to missing assignment operators. The previous version had a separate method destroy simply calling the in-place destructor. And emplace would simply assert in case an instance was already in place.
I think your idea is good but increases now the requirements on the wrapped object to also support operator=.

I have taken the liberty of adapting your suggested patch and reduce it to mainly adapt the inner representation of the class without changing the behaviour of the API. With this the I was able to build and the debugger showed the internals of the type (as expected).

0001-Change-to-typed_storage-without-requiring-operator.patch.txt
(somehow I couldn't add the patch file with the intended extension here)

What do you think?

[jwellbelove]

Your changes are reasonable. I overlooked that that I had added new conditions for the type stored.