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

Delete all the non-virtual default destructors #717

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

Conversation

jmcarcell
Copy link
Member

BEGINRELEASENOTES

ENDRELEASENOTES

Triggered because UserDataCollection's destructor was overriding without override and that was flagged in my editor, at that point all of them may as well be deleted. The side effect is a possibly useful comment in Frame.h and Writer.h removed.

Copy link
Contributor

@m-fila m-fila left a comment

Choose a reason for hiding this comment

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

I think in most of these cases, if not all of them, it's actually better to keep the destructors since at least one of the assignment/copy is also deleted/defined. So by having destructors we actually obey the rule of five ;)

@tmadlener
Copy link
Collaborator

I think I agree with @m-fila here. I haven't checked all of them, but the ones I saw had at least one "special" case for one of the other constructors. In case all of them are defaulted, we should remove all of them. Otherwise we should indeed go for rule of five.

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.

3 participants