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

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

Closed
m-fila opened this issue Jan 25, 2024 · 1 comment · Fixed by #598
Closed

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

m-fila opened this issue Jan 25, 2024 · 1 comment · Fixed by #598

Comments

@m-fila
Copy link
Contributor

m-fila commented Jan 25, 2024

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

@m-fila m-fila changed the title collection push_back and value_type incompatible with std some iterator adaptors collection push_back and value_type incompatible with some std iterator adaptors Jan 25, 2024
@tmadlener
Copy link
Collaborator

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

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 a pull request may close this issue.

2 participants