Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

[experimental] Add NotNull!T #1321

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
6 participants
Member

9rnsr commented May 29, 2013

After reading the forum thread "[article] Language Design Deal Breakers", I'd like to propose one NotNull!T design.

Note that, this is still experimental proposal. If it does not work by current compiler bug, the bug should be fixed first. If this NonNull interface is not good, it should be discussed in here.

Discussion is welcome!

Member

9rnsr commented May 29, 2013

One another design by @adamdruppe : #477

@ghost

ghost commented May 29, 2013

I might be missing something, but wasn't the point of NotNull in a language to have the determination that something isn't null at compile-time? Having an invariant that throws on null doesn't do much more than trying to dereference a null pointer or access a field on a null class object reference, it's still a runtime error. Perhaps I'm wrong though.. ?

Contributor

adamdruppe commented May 29, 2013

On Wed, May 29, 2013 at 01:02:49PM -0700, Andrej Mitrovic wrote:

I might be missing something, but wasn't the point of NotNull in a language to have the determination that something isn't null at compile-time?

The type system does that. If you ask for a NotNull!T instead of a
T, you know that wherever it is, you'll be ok except for the one
assignment point.

int* a = something_not_null;
/* lots of code */

*a = 10; // segfault. How did a become null? Gotta look at lots of code

NotNull!(int*) a = something_not_null;

/* lots of code */

*a = 10; // guaranteed to be fine here

If lots of code included an "a = null", that would be a compile error
and if it was "a = b" where b is null, it'd be a runtime error, yes,
but right there at the assignment point instead of waiting until the
usage point.

The other thing some languages do is on the assignment, force a null
check like this:

if(a is null) {
// handle
} else {
// a's type magically changes to NotNull!(typeof(a)) in this branch
}

D can't change a variable's type after it is declared, so we just can't
do that exactly (and I've tried a few approximations, none of them are
really any good except what follows). We can however do this at least:

if(a_maybe is null) {
// handle it
} else {
auto a = assumeNotNull(a_maybe);
// use a here which has the new type
}

and get pretty much the same result.

@ghost

ghost commented May 29, 2013

Thanks @adamdruppe, that explains it.

Contributor

adamdruppe commented May 29, 2013

BTW here's the comment i wrote up for the doc before

/**
NotNull ensures a null value can never be stored.

    * You must initialize it when declared

    * You must never assign the null literal to it (this is a compile time error)

    * If you assign a null value at runtime to it, it will immediately throw an Error
      at the point of assignment.

    NotNull!T can be substituted for T at any time, but T cannot become
    NotNull without some attention: either declaring NotNull!T, or using
    the convenience function, assumeNotNull.

    Examples:
    ---
            int myInt;
            NotNull!(int *) not_null = &myInt;
            // you can now use variable not_null anywhere you would
            // have used a regular int*, but with the assurance that
            // it never stored null.
    ---

*/

I don't remember why my original pull request with this died. Maybe it was just time, or maybe some flaw, but I definitely remember doing this before and it didn't get committed so I encourage you all to destroy the code.

Member

jmdavis commented May 29, 2013

I don't remember why my original pull request with this died. Maybe it was just time, or maybe some flaw, but I definitely remember doing this before and it didn't get committed so I encourage you all to destroy the code.

According to the pull, more work on it was needed, and then you were too busy to do it, so it got closed.

Member

9rnsr commented May 30, 2013

@AndrejMitrovic Essentially having invariant() { assert(_store !is null); } is redundant, because NonNull!T does not permit initializing/changing its _store field to null by the I/F design.

I removed the redundancy.

Contributor

adamdruppe commented May 30, 2013

On Wed, May 29, 2013 at 06:39:39PM -0700, Hara Kenji wrote:

I removed the redundancy.

Are unittests redundant too? If all works well, the invariant is
redundant, just like any other assert(), but the point of having
them is to catch a bug where something isn't working right.

Member

9rnsr commented May 30, 2013

@adamdruppe OK. Removing invariant was too hasty. I reverted it with detailed comment to explain the purpose why invariant is kept.

Member

9rnsr commented May 30, 2013

I'd like to explain improved points from #477.

  1. NonNull!T does not directly accept T anymore.
    When you need to make NonNull!T, you should always use assumeNotNull function. It's not a convenient utility, rather that is only one entry of NonNull!T.
    Using assumeNotNull clearly argues that the non-null validation has been already done for the given value. Self-documenting is good.
  2. assumeNonNull guarantees non-nullable under the @safe-ty.
    Inside @safe code block, assert is usually tested. Only when -release switch is specified, it would be removed. I think it is balanced behavior between safety and efficiency.
Contributor

ntrel commented Aug 5, 2013

I'd like to see this in Phobos. (Personally I prefer NullSafe to NotNull for the struct name).

assumeNonNull guarantees non-nullable under the @safe-ty

Although assumeNotNull is memory-safe, I would prefer to make it @system. This is to discourage direct use of assumeNotNull. I think most users should instead call a wrapper function that doesn't assert on null, e.g.:

NotNull!T assumeNotNull(T)(T t) @system;

NotNull!T notNull(T)(T t) @trusted
{
    if (t is null)
        throw new NullPointerException;
    else
        return assumeNotNull(t);
}

Using notNull is null-safe at compile-time, even in release mode @system code. The other advantage is that multiple NullPointerException throw-sites can be handled by the same catch block.

dejlek commented Nov 13, 2013

Will this be merged into Phobos please? Is it possible to check at compile-time that function may return a null, and get an error if it is so?

Collaborator

monarchdodra commented Nov 13, 2013

Will this be merged into Phobos please?

This looks good to me. I like the design, and I see no bugs. It also seems useful, and is something I'd use. While I'm not a fan of the alias this, due to the dangers of implicit pointer dereference, I'll make an exception here, since the whole idea is that said pointer is never null anyways, so the operation should be safe.

This works with classes too, right? There's no coverage in the unittests for it. Some extra useage documentation in the form of documented unittest would also be welcome.

I suppose that, like "Nullable", a future improvement would be to the ability to specify what the "null state" is, eg: NotNull!(int, 0) //An int that is guaranteed to be non-0.

Can I get some more opinion? @andralex or @AndrejMitrovic , maybe?

Is it possible to check at compile-time that function may return a null, and get an error if it is so?

I'd say it depends on your definition of "may". For example, the compiler could simply error out 100% of the time, if a function returns a pointer or a NotNull, since that "may" be null :/

As for NotNull, breaking it is as easy as:

auto p = NotNull!(int*).init; //Actually null.

BUT, I think the current design of NotNull is robust enough to give some pretty damn good guarantees, and blow up quite soon if it isn't.

dejlek commented Nov 13, 2013

Maybe we should remove "may" from that statement. Basically I want to mark a function as a function that never returns a null. If it does, I get a compile-time error. Originally I was thinking of writing an UDA @NotNull and mark a function with it, but then Adam reminded me of NotNull!T ...

@ghost

ghost commented Nov 13, 2013

I think this deserves a community review and not just a pull request review. NotNull is constantly being discussed after all.. And we don't want to end up in the same situation as with Tuple, where we're stuck having half the features in the language and the other half in the library.

@ghost

ghost commented Feb 15, 2014

Since NotNull has recently been discussed as a language feature (with backing from Andrei & Walter), it's probably best to close this until we have a consensus on the way forward.

@ghost ghost closed this Feb 15, 2014

This issue was closed.

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