I think `alias get this` is a misfeature.

If at all possible, compiletime errors should be preferred over runtime errors. The point of Nullable is that the value contained within may be absent. Considering the prevalence of type inference and UFCS chaining in idiomatic D, it is very possible that a change to a Nullable type, or a change of a parameter to a non-Nullable type, may not be noticed by the typesystem. In that case, the unhandled null state that could have been caught by the compiler will turn into a runtime exception.

This seems a poor, magical and unexpected choice of behavior.

Is there any strong reason why Nullable!T can implicitly convert to T? If not, I'd argue that `alias get this` should be deprecated, and removed at the earliest convenience. It throws away clear safety for unclear gain.

On Monday, April 16, 2018 08:34:13 FeepingCreature via Digitalmars-d wrote:
> I think `alias get this` is a misfeature.
>
> If at all possible, compiletime errors should be preferred over runtime errors. The point of Nullable is that the value contained within may be absent. Considering the prevalence of type inference and UFCS chaining in idiomatic D, it is very possible that a change to a Nullable type, or a change of a parameter to a non-Nullable type, may not be noticed by the typesystem. In that case, the unhandled null state that could have been caught by the compiler will turn into a runtime exception.
>
> This seems a poor, magical and unexpected choice of behavior.
>
> Is there any strong reason why Nullable!T can implicitly convert to T? If not, I'd argue that `alias get this` should be deprecated, and removed at the earliest convenience. It throws away clear safety for unclear gain.

I think that the whole point of the alias this was to try and make it so that code could treat Nullable!T as T if it didn't care about the difference - and plenty of code has no need to, especially if code above it already guaranteed that it has a value. I don't think that that's necessarily unreasonable and don't see anything unsafe or buggy about it.

However, on some level, the whole idea of treating Nullable!T as T in code that doesn't need to check for null falls flat on its face thanks to the fact that we don't have implicit construction in D. So, if you have something like

auto foo(Nullable!int i)
{
    ...
}

you can't pass it 42. You're forced to do something like Nullable!int(42) or nullable(42). I've found that to frequently be annoying when a function returns a Nullable. Ideally, the differences between Nullable!T and T would only be visible when get or some other member function on Nullable was called, and the alias this helps with that, but it really doesn't fix it.

And actually, even if we had implicit construction, it still wouldn't be enough, because alias this isn't used with template instantiations, and we don't currently have a way in the language to say that it should be. Without that, any templates that accepts T won't work with Nullable!T unless they accept implicit conversions to T (which usually a bad idea with templates). To really act like a T, Nullable!T would need to instantiate templates with T rather than Nullable!T (at least by default). So, we'd need several language changes in order to be able to have code treat Nullable!T the same as T except when it actually cared, and as such, the alias this on Nullable arguably doesn't achieve its goal. It just solves one piece of the puzzle and then causes frustration when it doesn't solve the others. I don't know if the frustration would go up or down if it didn't attempt to solve the problem at all and Nullable had no implicit conversion.

In any case, given that a Nullable!T really can't masquerade as a T in general, I don't know how valuable the alias this really is, and I'm not sure that I care whether it's there or not, but I fail to understand why it's actually a problem. I don't follow how this has anything to do with the type system or making anything safer. You don't get any more or fewer null checks with

foo(i.get);

than

foo(i);

The code is the same except for the fact that in the second case, the call to get is invisible. As such, all I see that you lose is that the programmer can't necessarily immediately see that i is a Nullable!T instead of a T. I can see why that might be undesirable for some folks, but I don't see why it would be a safety problem. And given that we already have alias this on Nullable, we need a reason for getting rid of it, not a reason for why it would be useful to have. I think that you need a more concrete reason as to why this is a problem other than it's too "magical." And you may have such a reason, but I don't see it clearly stated here.

- Jonathan M Davis

Say that we have a function that returns an int. We assign it to an auto variable and pass that to another function which takes an int. All is well.

Say we change the function to return a Nullable!int. We expect the compiler to warn us that we must now check for isNull; however, what we actually get is a runtime crash! It's almost like we're writing Javascript!

Say we have a function that takes a Nullable!int. However, we want to change the function to merely take an int, and move the isNull check outside it. However, we again face a runtime crash instead of a compiler warning.

Say we are simply misremembering the return type of a function. Say the call is in a little-used part of the code, just lying in wait to crash our application.

This is not good, and it's not how you would expect a statically typed language to behave.

Addendum: The general principle is that a "narrowing conversion" should never be implicit. An example of a narrowing conversion is float to int, or int to short. In the case of integer types, D already follows this rule.

Nullable!T to T is a narrowing conversion, since T cannot express the null case. Therefor, it must not be implicit.

On Monday, 16 April 2018 at 10:10:56 UTC, FeepingCreature wrote:
> Nullable!T to T is a narrowing conversion, since T cannot express the null case. Therefor, it must not be implicit.

I agree with this. To even reach for a Nullable type in one's codebase incurs a cost -- the need is substantial to oughtweigh this complexity. In this situation, one will design internal interfaces both with and without Nullable. The strict typechecks (prevent assignment of Nullable!A to A) make large codebases of this kind safe to refactor.

If the checks must get more verbose because Nullable isn't part of the language, but rather a library feature, that's certainly a cost to pay for reliability. But it's still better than these runtime crashes. It's sad every time code crashes at runtime with errors that other type systems easily find at compiletime.

An alternative to Phobos's Nullable might be aliak's Optional? It's in the dub registry as "optional", or on github: https://github.com/aliak00/optional

    import optional;
    struct S { int i; }
    void main()
    {
        Optional!S s = some(S(5));
        S t = s;
    }

This produces the desired error at compiletime: source/app.d(6,11): Error: cannot implicitly convert expression s of type Optional!(S) to S.

-- Simon

If nobody objects, I'll make a PR to deprecate it.

On Wednesday, 18 April 2018 at 08:24:14 UTC, FeepingCreature wrote:
> If nobody objects, I'll make a PR to deprecate it.

+1. Nullable has received some improvements lately, it would be great if `get` was no longer implicit. For new code, it is trivial to write `.get` to convert to the original type. For old code, deprecation is a good solution. (Permanent deprecation can solve the problem of breaking code in the longer term).

There is an argument for making `get` *require* a fallback value rather than a default `.init` argument, to make the conversion completely explicit, but this is perhaps a change too far considering how lax we currently are. A third way would be to require a fallback argument only when the original type doesn't have a good invalid value like null. At least null dereference, if triggered, will be detected immediately upon execution. An unexpected zero-value might go undetected for a long time.

On Wednesday, 18 April 2018 at 10:01:17 UTC, Nick Treleaven wrote:
> On Wednesday, 18 April 2018 at 08:24:14 UTC, FeepingCreature wrote:
>> If nobody objects, I'll make a PR to deprecate it.
>
> +1. Nullable has received some improvements lately, it would be great if `get` was no longer implicit. For new code, it is trivial to write `.get` to convert to the original type. For old code, deprecation is a good solution. (Permanent deprecation can solve the problem of breaking code in the longer term).

I agree; the important thing to me is that code that uses the implicit conversion is at least highlighted in some way.

> There is an argument for making `get` *require* a fallback value rather than a default `.init` argument, to make the conversion completely explicit, but this is perhaps a change too far considering how lax we currently are. A third way would be to require a fallback argument only when the original type doesn't have a good invalid value like null. At least null dereference, if triggered, will be detected immediately upon execution. An unexpected zero-value might go undetected for a long time.

An argument against making get require a fallback is that D lacks a good abstraction for customizing control flow, so we don't have a good way to say "if Nullable contains a value, do this with the value, else do this fallback," meaning we have to do if (isNull) { get; } or such. That said, I'm pretty sure that get will throw anyways if invoked without a default value.

I've hit a bit of a snag while implementing the PR.

https://issues.dlang.org/show_bug.cgi?id=18775 seems to make deprecating the implicit conversion basically impossible, because the deprecation will be triggered on every UFCS access to a Nullable, no matter whether it actually uses .get or not. DMD seems to generally have issues isolating failed struct lookups properly.

On Wednesday, 18 April 2018 at 13:36:15 UTC, FeepingCreature wrote:
> That said, I'm pretty sure that get will throw anyways if invoked without a default value.

Not in release mode, but I suppose that doesn't matter. I've made a pull to update the docs to reflect this:
https://github.com/dlang/phobos/pull/6468