On 10/2/2013 7:18 AM, Luke Bradford wrote:
Thanks for the feedback, Matt - some quick thoughts:
Not having a default constructor seems unnecessary to me. Why not just have the default constructor dynamically allocate a default constructed object? Similarly, for consistency with other smart pointers, having a conversion to bool would be useful and makes transition to the type simpler.
Often the object pointed to will not have a default constructor, so we wouldn't be able to construct one in shared_ptr_nonnull's default constructor. I also think that having a conversion to bool is misleading - in the use cases I've seen, it would lead to a lot of extraneous if( ptr ) statements, then essentially if( true ), which obscure the programmer's intentions and are easy to remove. I found the elimination of the default constructor and conversion to bool just as useful, if not more useful, than the runtime checks of the class, because they revealed places where we were default constructing or checking a shared_ptr that we expected to be valid at all times.
Keeping bool conversion allows your ptr type to be used with templated code that may do the checks, which would otherwise be complicated by the divergence from pointer semantics. Perhaps marking the operator constexpr would allow the compiler to optimize away any branches.
I don't think an exception here is proper. Initializing the object by passing a null pointer should be UB since it is a clear violation of the constructor's assumed precondition (don't pass a null pointer), and similar for other operations that would produce such a "null" non-null object. Rather, these should simply be asserts.
I could go either way on this one. One reason I opted for throwing is because that's the behavior of shared_ptr when you construct it with an empty weak_ptr, and this seemed like a similar situation.
As Matt said this would be a precondition violation so an assert would be better than a runtime exception. Jeff