AIDASoft / podio

PODIO
GNU General Public License v3.0
23 stars 57 forks source link

collection push_back and value_type incompatible with some std iterator adaptors #546

Closed m-fila closed 2 months ago

m-fila commented 7 months ago

The std adaptors often rely on a typedef value_type. The PODIO mutable semantics complicate matters, as Collection::value_type is simply defined as an immutable type.

Collection push_back has overloads for both mutable and immutable types. The overloads have different behaviors, for instance, for Collection::value_type overload the non-subset collections throws:

invalid_argument: Can only add immutable objects to subset collections

While itself it's correct behavior, some std adapters do implicit conversion to value_type and end up using the overload for immutable type, e.g copying elements withstd::back_insert_iterator:

// this throws because the immutable type is pushed
std::transform(std::begin(a), std::end(a), std::back_inserter(b),
[](const ExampleHit& i) -> MutableExampleHit {
      return i.clone();
});

// this doesn't throw since the mutable type is pushed
std::for_each(std::begin(a), std::end(a), [&b](const auto& i) {
      b.push_back(i.clone());
});

I'm not sure if there are any possible solutions that don't change the value_type or push_back semantics

tmadlener commented 7 months ago

This seems to be one of the things where our collections fail to fully satisfy things that are part of the std::vector contract, while mostly pretending to do so. Not sure if this one is salvageable or not, but we might have to think about a clearer distinction between our collections and the c++ STL containers (regarding member typedefs, member functions, etc...)

(Somewhat) related: #479, #272, #150