no conversion from T to not_null<T> #401
Conversation
|
Hi @akrzemi1, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
| CHECK((void*)p.get() == (void*)t.get()); | ||
| } | ||
|
|
||
| TEST(TestNotNullAssignment) | ||
| { | ||
| int i = 12; | ||
| not_null<int*> p = &i; | ||
| not_null<int*> p(&i); |
There was a problem hiding this comment.
If we are getting rid of the assignment operator of T, do we need the test at all? I believe there are already tests for construction.
There was a problem hiding this comment.
That line isn't exercising assignment though.
There was a problem hiding this comment.
Do we need a make_not_null factory ?
void f(not_null<int*>);
f(make_not_null(&i))
Wondering if in the particular case of not_null<T*> it couldn't be copy-constructed and assigned from a T&.
f(i); // implicit conversion from T& to `not_null<T*>`
There was a problem hiding this comment.
@viboes actually, there's already some discussion (and previous code intent) to allow exactly that. See #396 for a recent post on the topic.
[Edit] Sorry to be clear, I meant on the topic of constructing from a T&. I like the idea of a make_not_null() factory function. The only reason I didn't originally add one is that I was nervous it would confuse people as to whether it allocated given the prior art of make_shared and make_unique
There was a problem hiding this comment.
@neilmacintosh make_not_null() is not a good idea, but what about gsl::addressof() just like the one in std?
|
please see discussion in #699. Closiing this since we decided to remove the explicit constructor and add strict_not_null type for practical purposes. |
No description provided.