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

common: add copy constructor macros #2596

Merged
merged 1 commit into from
Dec 19, 2023
Merged

common: add copy constructor macros #2596

merged 1 commit into from
Dec 19, 2023

Conversation

Riolku
Copy link
Contributor

@Riolku Riolku commented Dec 19, 2023

Instead of using std::unique_ptr all over the place to prevent copies, we can delete copy constructors. To enable this, this change adds three macros: EXPLICIT_COPY, NO_COPY, and NO_COPY_OR_MOVE. They should be added to any non-trivial class to define how copying and moving should be done.

By default, move constructors and move assignment operators are enabled, unless disabled with NO_COPY_OR_MOVE. EXPLICIT_COPY adds a .copy() method, which can be used for explicit copying, making it clear when we are paying the price of a full copy.

Future work could be done to ensure these macros are always added.

Instead of using std::unique_ptr all over the place to prevent copies,
we can delete copy constructors. To enable this, this change adds three
macros: EXPLICIT_COPY, NO_COPY, and NO_COPY_OR_MOVE. They should be
added to any non-trivial class to define how copying and moving should
be done.

By default, move constructors and move assignment operators are enabled,
unless disabled with NO_COPY_OR_MOVE. EXPLICIT_COPY adds a `.copy()`
method, which can be used for explicit copying, making it clear when we
are paying the price of a full copy.

Future work could be done to ensure these macros are always added.
@Riolku Riolku merged commit ef073c1 into master Dec 19, 2023
12 checks passed
@Riolku Riolku deleted the copy-constructor-macros branch December 19, 2023 16:49
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