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
Smart pointer with allocator support #2901
Conversation
Roughly equivalent to not_null<std::unique_ptr<T>> except that it takes an allocator instead of a deleter. A Nullable version is also provided.
Specifically, change not_null<std::unique_ptr> to Box in ContinuousTrajectory and DiscreteTrajectory.
The changes away from the standard library types in many unrelated classes make me deeply uneasy, especially for a local semantics-preserving platform-specific performance improvement. You mentioned in #2899 that overloading If that is not feasible, could something more akin to #2900 work? You mention that
This is because |
I'll second @eggrobin's comment: not only is the introduction of For instance, I have been thinking for some time of moving away from I like the way that things were done in #2900, because it will Just Work™ without us having to use our brain. |
I agree that #2900 is much cleaner, and an approach that just works would be preferred (I would also like to replace all the unique ptrs instead of just some of them). The only ways I can see to do this cleanly are the two ways you mentioned: overload new or add a Regarding overloading Regarding +1 to replacing Closing this PR in favor of a future |
(To clarify the |
This really surprises me. If this is happening, it is a bug and we should track it down (we should be able to reproduce it on Windows, presumably). We are trying very hard to avoid ownership transfers between C# and C++: for instance, all the pointers allocated in C++ code and passed to C# should come back to C++ for deallocation (look for the |
I looked further into the crash induced when overloading So, no bug in Principia; what I was attempting to do just doesn't seem to work. |
Second part of a two-part fix for #2899.
This change adds a new smart pointer class,
Box
, which is equivalent tonot_null<std::unique_ptr>
except that it is parameterized on an allocator instead of a deleter. Because it is within theprincipia
namespace, it will automatically use the allocator from #2900 when the header from #2900 is present.Unlike
std::unique_ptr
, there is no way to construct aBox
from a raw pointer—it cannot be verified that the raw pointer was constructed with theBox
's allocator. For the case of classes with private constructors, there is a factory function inBox
which allocates memory for an object without constructing it; the object can then be constructed with placementnew
.A nullable version is also provided.
This change also replaces
not_null<std::unique_ptr>
withBox
inContinuousTrajectory
andDiscreteTrajectory
. Many other files which transferred unique_ptrs into or out of trajectories had to be changed as well.