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

Adds create_with_same_size function #15204

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

Conversation

MarcelKoch
Copy link

This PR adds a static function create_with_same_size to (hopefully) all vector types. This function can be uses instead of the pattern

VectorType v;
v.reinit(u);

to create a new vector that has the same size (and possible other properties) as the input vector. In most places create_with_same_size is actually implemented using the above pattern. But this function can be used for non-default-constructible vectors (such as the ones introduced in #15190), while the above pattern would not work.

The pattern above is replaced where I could find it. Also, some functions are changed a bit more to be able to use the new interface.

This allows to change the pattern
```
VectorType v;
v.reinit(other);
```
to
```
auto v = VectorType::create_with_same_size(other);
```
which does not require VectorType to be default constructible
This changes some functions to return new vectors, instead of using output parameters. In these cases, the output parameter were created directly before, so there should be no difference.
@MarcelKoch MarcelKoch changed the title Adds create Adds create_with_same_size function May 12, 2023
Copy link
Member

@tamiko tamiko left a comment

Choose a reason for hiding this comment

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

I strongly oppose this change of creating a static create_with_same_size function that acts as a factory function for creating a vector.

We have settled on our somewhat idiomatic, but nevertheless uniform set of default constructors, copy/assignment, and reinit() functions. I do not think that we should change this interface lightly.

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

Successfully merging this pull request may close these issues.

None yet

2 participants