add a simple NotNull struct #477

Closed
wants to merge 10 commits into
from

Conversation

Projects
None yet
7 participants
@adamdruppe
Contributor

adamdruppe commented Mar 5, 2012

I've talked about implementing these a lot of times on the newsgroup, but I don't see one in Phobos yet!

This is my latest idea on how to implement a NotNull type with tests and documentation, added
to std.typecons (right below Nullable).

I haven't actually used any NotNull in practice, but from the attached tests, it looks like it will work well - hence, the pull request.

I'm open to any comments, however.

@klickverbot

View changes

std/typecons.d
+ try
+ {
+ takesNotNull(notNull(test)); // test is null now, so this should throw an assert failure
+ assert(0, "it let us pass a null value to a NotNull function");

This comment has been minimized.

@klickverbot

klickverbot Mar 5, 2012

Member

Isn't this going to be caught by the catch clause as well? See assertThrown and friends.

@klickverbot

klickverbot Mar 5, 2012

Member

Isn't this going to be caught by the catch clause as well? See assertThrown and friends.

This comment has been minimized.

@adamdruppe

adamdruppe Mar 5, 2012

Contributor

Oh, you're right! Duh. Changed to assertThrown!AssertError

@adamdruppe

adamdruppe Mar 5, 2012

Contributor

Oh, you're right! Duh. Changed to assertThrown!AssertError

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Mar 5, 2012

Shouldn't assert(rhs !is null); be changed to enforce(rhs !is null);?

ghost commented Mar 5, 2012

Shouldn't assert(rhs !is null); be changed to enforce(rhs !is null);?

@adamdruppe

This comment has been minimized.

Show comment
Hide comment
@adamdruppe

adamdruppe Mar 5, 2012

Contributor

I was thinking of the not null as being similar to a contract; it looks for usage errors in the program rather than something like user input, so assert is the thing. I think assert is right.

Contributor

adamdruppe commented Mar 5, 2012

I was thinking of the not null as being similar to a contract; it looks for usage errors in the program rather than something like user input, so assert is the thing. I think assert is right.

@alexrp

This comment has been minimized.

Show comment
Hide comment
@alexrp

alexrp Mar 18, 2012

Member

FWIW, I agree that we should use assert for this.

Member

alexrp commented Mar 18, 2012

FWIW, I agree that we should use assert for this.

@alexrp

View changes

std/typecons.d
+ }
+}
+
+/// A convenience function to construct a NotNull value. If you pass it null, it will throw.

This comment has been minimized.

@alexrp

alexrp Mar 18, 2012

Member

Should be "it will assert"?

@alexrp

alexrp Mar 18, 2012

Member

Should be "it will assert"?

@klickverbot

This comment has been minimized.

Show comment
Hide comment
@klickverbot

klickverbot Mar 18, 2012

Member

Maybe an assumeNonNull/checkNonNull pair would provide a nice unified entry point to the NonNull universe?

Member

klickverbot commented Mar 18, 2012

Maybe an assumeNonNull/checkNonNull pair would provide a nice unified entry point to the NonNull universe?

@JakobOvrum

This comment has been minimized.

Show comment
Hide comment
@JakobOvrum

JakobOvrum Apr 22, 2012

Member

How about adding an opAssign overload for T (which would assert)?

Also, this type does not work for all possible types T, thus it should have a template constraint.

Member

JakobOvrum commented Apr 22, 2012

How about adding an opAssign overload for T (which would assert)?

Also, this type does not work for all possible types T, thus it should have a template constraint.

@adamdruppe

This comment has been minimized.

Show comment
Hide comment
@adamdruppe

adamdruppe Apr 22, 2012

Contributor

I actually had the opAssign before, but removed it in tonight's commit. The reason is you might want the type system to catch an accidental assignment so you can make a decision - check null or assume null - at that time and write it explicitly.

The same logic could apply to the constructor, but the fact that you can't use auto is annoying enough as it is, and having to do NotNull!T t = assumeNotNull(new T()); is probably just too far.

Contributor

adamdruppe commented Apr 22, 2012

I actually had the opAssign before, but removed it in tonight's commit. The reason is you might want the type system to catch an accidental assignment so you can make a decision - check null or assume null - at that time and write it explicitly.

The same logic could apply to the constructor, but the fact that you can't use auto is annoying enough as it is, and having to do NotNull!T t = assumeNotNull(new T()); is probably just too far.

@JakobOvrum

This comment has been minimized.

Show comment
Hide comment
@JakobOvrum

JakobOvrum Apr 22, 2012

Member

I agree, the transition from nullable to non-nullable should be explicit when possible, it just seemed a little out of sync with the constructor. I suppose construction isn't a problem because the name of the type, NotNull, will always be explicit at least once.

Member

JakobOvrum commented Apr 22, 2012

I agree, the transition from nullable to non-nullable should be explicit when possible, it just seemed a little out of sync with the constructor. I suppose construction isn't a problem because the name of the type, NotNull, will always be explicit at least once.

+}
+
+/// A convenience function to construct a NotNull value from something you know isn't null.
+NotNull!T assumeNotNull(T)(T t)

This comment has been minimized.

@JesseKPhillips

JesseKPhillips Apr 22, 2012

Contributor

I think this function should contain an assert(t !is null)

@JesseKPhillips

JesseKPhillips Apr 22, 2012

Contributor

I think this function should contain an assert(t !is null)

This comment has been minimized.

@klickverbot

klickverbot Apr 22, 2012

Member

The constructor asserts.

@klickverbot

klickverbot Apr 22, 2012

Member

The constructor asserts.

@klickverbot

View changes

std/typecons.d
+}
+
+/// A convenience function to check for null. If you pass null, it will throw an exception. Otherwise, return NotNull!T.
+NotNull!T checkNotNull(T)(T t)

This comment has been minimized.

@klickverbot

klickverbot Apr 22, 2012

Member

I know I had that name in my previous comment, but maybe we should call this enforceNotNull for consistency – opinions?

@klickverbot

klickverbot Apr 22, 2012

Member

I know I had that name in my previous comment, but maybe we should call this enforceNotNull for consistency – opinions?

+ // have used a regular int*, but with the assurance that
+ // it never stored null.
+ ---
+*/

This comment has been minimized.

@jmdavis

jmdavis May 4, 2012

Member

There are tabs in the comments. Please replace them with spaces.

@jmdavis

jmdavis May 4, 2012

Member

There are tabs in the comments. Please replace them with spaces.

+ return _notNullData;
+ }
+ // Apparently a compiler bug - the invariant being uncommented breaks all kinds of stuff.
+ // invariant() { assert(_notNullData !is null); }

This comment has been minimized.

@jmdavis

jmdavis May 4, 2012

Member

Have you reported the bug?

@jmdavis

jmdavis May 4, 2012

Member

Have you reported the bug?

@andralex

This comment has been minimized.

Show comment
Hide comment
@andralex

andralex Jul 2, 2012

Member

This seems to be languishing. @adamdruppe, still there?

Member

andralex commented Jul 2, 2012

This seems to be languishing. @adamdruppe, still there?

@adamdruppe

This comment has been minimized.

Show comment
Hide comment
@adamdruppe

adamdruppe Jul 2, 2012

Contributor

I've been very busy the last couple months with a lot of
real life stuff.

Going to be settling down again soon; moving this week
and then thing should start returning to normal.

On Sun, Jul 01, 2012 at 07:02:27PM -0700, Andrei Alexandrescu wrote:

This seems to be languishing. @adamdruppe, still there?


Reply to this email directly or view it on GitHub:
D-Programming-Language#477 (comment)

Contributor

adamdruppe commented Jul 2, 2012

I've been very busy the last couple months with a lot of
real life stuff.

Going to be settling down again soon; moving this week
and then thing should start returning to normal.

On Sun, Jul 01, 2012 at 07:02:27PM -0700, Andrei Alexandrescu wrote:

This seems to be languishing. @adamdruppe, still there?


Reply to this email directly or view it on GitHub:
D-Programming-Language#477 (comment)

@andralex

This comment has been minimized.

Show comment
Hide comment
@andralex

andralex Jul 9, 2012

Member

waiting

Member

andralex commented Jul 9, 2012

waiting

@andralex

This comment has been minimized.

Show comment
Hide comment
@andralex

andralex Jul 16, 2012

Member

Will close this for now, please reopen as fit. Thanks!

Member

andralex commented Jul 16, 2012

Will close this for now, please reopen as fit. Thanks!

@andralex andralex closed this Jul 16, 2012

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