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

Using `not_null` with *smart pointers*. #225

Closed
galik opened this Issue Sep 29, 2015 · 3 comments

Comments

Projects
None yet
4 participants
@galik
Contributor

galik commented Sep 29, 2015

I think this issue really needs to be brought up here first: Microsoft/GSL#89

I made a comment on that issue over at Microsoft/GSL but it occurred to me that it should be dealt with in the Guidelines, so I will repeat my comments here:

According to the guidelines for passing smart pointers it seems that the only reason we should be passing a smart pointer to a function is when that fuction modifies the ownership in some way. Otherwise, the recommendation is that we pass a raw pointer.

R.30: Take smart pointers as parameters only to explicitly express lifetime semantics

Reason: Accepting a smart pointer to a widget is wrong if the function just needs the widget itself.
It should be able to accept any widget object, not just ones whose lifetimes are managed by a particular kind of smart pointer.
A function that does not manipulate lifetime should take raw pointers or references instead.

Bearing that in mind I am not sure that not_null should be passing smart pointers into functions at all.

After all a function receiving a not_null generally wants to be able to make calls against the pointer without having to first check its validity. It is unlikely a function receiving a not_null would want to deal with its ownership and, even if it did, would we want to encourage that? After all such a function would have the power to reset the pointer breaking its not null contract.

Maybe there are use-cases I have missed but it seems to me the main use for the not_null (with respect to smart pointers) is for down-calling and therefore for passing only the raw pointer that the smart pointer is managing.

Therefore we might not want a not_null to contain a smart pointer, only its associated raw pointer.

To that effect we could create not_null specializations for both unique_ptr and shared_ptr whereby it captures their managed raw pointer using get() and implements its not null invariant on construction.

@bashrc-real

This comment has been minimized.

bashrc-real commented Oct 1, 2015

A use case where we may want to use the class with smart pointer is passing a shared resource while creating an object.
For eg BufferReader::BufferReader(const shared_ptr &stream) : m_stream(stream) {}
BufferReader may want to (and very rightly so) ensure that it's always created with a valid non-null stream.
Although I agree that get() for smart pointers should not return the actual object, because then handling smart pointers which assume sole ownership of the object becomes difficult. On the other hand creating a specialization for shared_ptr and unique_ptr seems odd since it makes the contract complicated. Probably we need a new class altogether for handling ownership as well as asserting validity of a pointer

@rodiers

This comment has been minimized.

rodiers commented Oct 1, 2015

@galik "Therefore we might not want a not_null to contain a smart pointer, only its associated raw pointer"

I see two use-cases:

  • Suppose we have some kind of factory class that return creates objects on the heap and transfers ownership to the clients of this factory and hence it return smart pointers (probably unique_ptr). Some factories do want to guarantee that the objects always exist and hence would like to mark it as not_null. It is its post-condition that the object always exists. Clients then don't need to test whether the object is not null.
  • A second use-case is a kind of sink class that takes ownership of the object it takes. Hence it only accepts smart pointers. It might not make sense for the sink to accept a nullptr and hence its precondition is that the smart pointer shouldn't be a nullptr. Hence it uses not_null.

@neilmacintosh neilmacintosh self-assigned this Oct 1, 2015

@neilmacintosh

This comment has been minimized.

Contributor

neilmacintosh commented Oct 1, 2015

rodiers has given two nice examples of where a function might want to accept or return a smart pointer (it is a factory, or a sink). In those cases, using not_null around a smart pointer would be entirely appropriate, it does not interfere with the fact that a smart pointer is still being passed/returned to express lifetime semantics.

So I think both guidelines are fine as-is: the guideline to only use smart pointers as parameters to express lifetime semantics and the recommendation to use not_null to indicate a particular property of a pointer - raw or smart.

The issue over at the GSL repo seems to be more about the specific characteristics of that implementation of not_null...which is a discussion best had over there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment