not_null pointer type, and unique/shared constructors

[rianquinn]

Two questions:

• Why does not_null take a pointer type instead of a non-pointer type like span, unique_ptr and shared_ptr. That is:

auto safe_ptr = gsl::not_null<int *>(ptr) // Currently
auto safe_ptr = gsl::not_null<int>(ptr) // Why not this?

• Is there a reason there is no constructor for unique_ptr / shared_ptr instead of having to use get()?

[viboes]

I believe it is because you can have not_null<unique_ptr<int>>.

[rianquinn]

I'm assuming your answering the first question, and I can see that to some degree. Would it be possible to use enable_if to provide unique_ptr support while also having a consistent syntax with the rest of the APIs? that is support both?

[neilmacintosh]

@rianquinn The answer to your first question from @viboes is correct. However, I am not entirely convinced that using not_null with unique_ptr and\or shared_ptr makes sense. There are a number of serious issues that pop up as soon as you try it. The "80% case" for this type is using it with raw pointers.

So I would not want to make it easier to use unique_ptr with not_null just yet...when I think it is still up for discussion whether that should be supported at all.

Or, putting it another way: I'd prefer to concentrate on making not_null great for raw pointers for now.

[rianquinn]

@neilmacintosh Just for context, this is the use case that I ran into:

void foo(gsl::not_null<int *> ptr)
{
    <do awesome stuff here>
}

void main()
{
    auto ptr = std::make_unique<int>();
    foo(ptr.get());
}

So the reason for both questions is, unique_ptr, shared_ptr and span are all given a <int> while not_null is given a <int *> and the second issue I noticed was the use of .get() here. I'm actually fine with both, but I figured I'd ask to see if there were reasons for the inconsistency.

[MikeGitb]

Regarding the second point: Are you suggesting there should be an implicit conversion constructor from unique_ptr to not_null? I admit that I often would have liked to have an implicit conversion operator from unique_ptr to raw pointer, so this would be fine with me, but it would mean that not_null can something, that a raw pointer can't.

Regarding the first point: I find not_null<int*> more intuitive for the simple reason, that there isn't a "ptr" in it's name and I haven't perceived not_null as a smart pointer so far.

[rianquinn]

I am suggesting implicit conversions, and I agree that it would mean that not_null would be able to do something that a raw pointer could not.

Years ago I wrote my own reference counted pointer class for Qt (believe it or not, Qt's own shared pointer pukes if you use it with a QObject), and I added operator T* to prevent the need for .get() and it was wonderful. That being said, that's not how the STL works, and I do want to stay consistent with the STL.

My reason for suggesting the implicit conversions is because the above example that I just provided will likely happen a lot since we are advised to use both unique_ptr to own the reference, and not_null to ensure safety, so being able to get rid of the use of .get() here make sense IMO. Another way to look at it is, code will likely unique_ptr -> not_null -> not_null -> etc... and there is no need for .get from not_null to not_null, so this just cleans up the chain.

[neilmacintosh]

@MikeGitb that's interesting feedback about the way you feel about not_null.

@rianquinn I agree with you about unique_ptr. I personally think that unique_ptr could have an implicit conversion to T* without the world becoming a substantially more dangerous place, but a much more convenient one ;-)

I'm not sure initializing a not_null<T*> implicitly from a unique_ptr<T> with the T* (as opposed to the unique_ptr<T>will be very clear as long as we still allow not_null<unique_ptr<T>>.

Perhaps we could revisit this request once we've decided what to do about not_null<unique_ptr<T>>?

[rianquinn]

@neilmacintosh just to poke the bear, if not_null used <T> instead of <T*>, not_null<unique_ptr<T>> would not work 😄 .

That being said... does not_null<unique_ptr<T>> make any sense? Wouldn't the following be a copy which is not allowed for a unique_ptr:

void foo(gsl::not_null<unique_ptr<int>> ptr)
{}

auto ptr = std::make_unique<int>();
foo(ptr);

For me... this makes a lot more sense (assuming not_null is using implicit conversion of unique_ptr and using <T>):

void foo(gsl::not_null<int> ptr)
{}

auto ptr = std::make_unique<int>();
foo(ptr);

I view not_null<T> as a pointer type, just like I view unique_ptr<T> as a pointer type. The only difference I see between the two is, not_null ensures the raw ptr it stores is not null, while unique_ptr owns the raw pointer it points to. From this point of view, it doesn't make sense to use them together as that is asking a different question (i.e. does unique_ptr own a pointer or not) which we have operator bool() for.

[rianquinn]

Another way to look at it is.... does not_null<T> (i.e. non-pointer type) make any sense? It might which is why I'm asking. If it doesn't then why not enforce a pointer type like unique_ptr and span.

[neilmacintosh]

@rianquinn I get it. My point is that there are already a number of other threads here where the discussion has started. I have my own views, but I want to leave some room for others to present alternate thoughts about why not_null<unique_ptr<T>> makes sense and how it should work..

[cubbimew]

alternate thoughts about why not_null<unique_ptr<T>> makes sense

It seems like the appropriate return type for a factory function (the kind of factory function that returns a valid object or throws). since it's preserving the most information and making null checks unnecessary in the caller. Dropbox uses nn<unique_ptr<T>> - perhaps it makes sense to ask about their user experience?

[01e9]

I thought it was made like this to be able to specify const T* or T * const but:

• gsl::not_null<const T*> is equivalent to const gsl::not_null<T*>
• gsl::not_null<T* const> doesn't compile:

.../gsl/pointers:72:5: error: static assertion failed: T cannot be assigned nullptr.
     static_assert(std::is_assignable<T&, std::nullptr_t>::value, "T cannot be assigned nullptr.");
     ^~~~~~~~~~~~~

This raises another question: Why gsl::not_null<T* const> is not allowed?

[beinhaerter]

@arteniioleg

I thought it was made like this to be able to specify const T* or T * const but:

• gsl::not_null<const T*> is equivalent to const gsl::not_null<T*>
• gsl::not_null<T* const> doesn't compile:

This is not correct.

1. const T * p and T const * p are a pointer p to a T that is const.
2. T * const p is a pointer that is const and that points to T.
3. gsl::not_null<const T*> and gsl::not_null<T const *> are a not_null object that embeds a pointer to a const T.
4. const gsl::not_null<T*> is a const not_null object that embeds a pointer to a T.
5. gsl::not_null<T* const> is a not_null object that embeds a const pointer to T.

3 is not the same as 4. You can easily check that:

int i{0};
gsl::not_null<const int*> a{ &i };
const gsl::not_null<int*> b{ &i };
	a = &i;  // compiles
	b = &i;  // does not compile
	*a = 3;  // does not compile
	*b = 3;  // compiles

Regarding 5. gsl::not_null<T* const> I am not sure why you would want that. The type in 4. const gsl::not_null<T*> is already what you want.