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

Avoid the use of raw pointers. Use std::unique_ptr instead. #5501

Merged
merged 2 commits into from
Nov 28, 2023

Conversation

bangerth
Copy link
Contributor

Inspired by #5488, these are the only places I could find where we still call operator new explicitly. This is, strictly speaking, an incompatible change because it changes the interface of a plugin system. Thoughts?

@bangerth bangerth mentioned this pull request Nov 19, 2023
@tjhei
Copy link
Member

tjhei commented Nov 20, 2023

This certainly breaks any plugin that contains a postprocessor of that kind. Not ideal, but this should result in a compilation error and is easy to fix. Maybe we leave a comment in the function documentation about how to fix this? If yes, I would be ok to go ahead with it (I assume we will be doing larger breaking changes going to 3.0 anyways)

Copy link
Member

@gassmoeller gassmoeller left a comment

Choose a reason for hiding this comment

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

I agree, please add a note to the comment above the function and also a changelog entry (so that we can compile a list of incompatible changes for the release notes). Then this is good to go.

@bangerth
Copy link
Contributor Author

I added the changelog entry. As for the note with the function, do you mean the declaration of the function in the base class? I don't like such notes that discuss what the interface used to be very much because they tend to be forgotten there and in a couple of years, they just describe a state that has since long been forgotten -- in other words, they become stale quite quickly. My philosophy is that the documentation should document the current state. The changelog is the right place to mention changes.

@gassmoeller gassmoeller merged commit 49ff2a9 into geodynamics:main Nov 28, 2023
4 of 5 checks passed
@bangerth bangerth deleted the pointer branch December 7, 2023 04:57
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.

3 participants