https://issues.dlang.org/show_bug.cgi?id=17440

--- Comment #10 from hsteoh@quickfur.ath.cx ---
P.S. Actually, Nullable does have another overload that takes a null value to be used in lieu of a boolean flag.  So conceivably, you could use Nullable!(T, null) instead of Nullable!T and you will have the requisite semantics.

--

https://issues.dlang.org/show_bug.cgi?id=17440

--- Comment #11 from Marenz <dmdtracker@supradigital.org> ---
I can only repeat what I said before. The principle of the least surprise
should apply. No one expects their object to be destroyed when a reference to
it is set to null, be it through .nullify(); or through = null;
It's not difficult to fix and it saves lots of people debugging time, figuring
out why the hell their object is gone.

Note that already two persons have stumbled over this AND have reported it. God knows how many more this happened to who haven't given this valuable feedback.

--

https://issues.dlang.org/show_bug.cgi?id=17440

--- Comment #12 from hsteoh@quickfur.ath.cx ---
I think you misunderstood my comment.  I meant that one way to fix this bug is to change Nullable, or at least the overload that takes a single type parameter, so that it does not instantiate for reference types, or redirects the instantiation to Nullable!(T, null), so that when you call .nullify on it, it will just set the reference to null instead of attempting to destroy the object by calling .destroy on it.

It really does not make sense to use Nullable!T, as currently implemented, for a class type T, since making that means there will be *two* null states, one with .nullify / .isNull, and the other with `= null` and `is null`, and the two null states will not be equivalent. That will only lead to more confusion.  The fact that Nullable!T's dtor calls .destroy is further proof that the original intent was to use it with by-value types, not with class references.  I'd say either the docs should recommend using Nullable!(T, null) for class types T, or else Nullable!T in that case should just internally redirect to Nullable!(T, null).

--

https://issues.dlang.org/show_bug.cgi?id=17440

--- Comment #13 from Chris Paulson-Ellis <chris@edesix.com> ---
Perhaps it would be helpful to reiterate one of my comments from my original forum thread linked in comment 8...

<snip>
For those confused as to why you'd want to wrap a Nullable around something
that already has nullable semantics, it's mostly about making APIs that
explicitly declare their optional return values. See:
http://www.oracle.com/technetwork/articles/java/java8-optional-2175753.html
</snip>

If Phobos needs a new Maybe/Optional template - perhaps specialised for class types - then so be it, but at the moment Nullable is it.

--

https://issues.dlang.org/show_bug.cgi?id=17440

--- Comment #14 from Chris Paulson-Ellis <chris@edesix.com> ---
Also - to emphasise what was said in comment 11 - let me just say that it took me *3 days* to find that the cause of a mysterious crash in my code was the destroy in nullify().

At the very least, the documentation for Nullable should contain a prominent warning that nullify() will invalidate your class references.

--

https://issues.dlang.org/show_bug.cgi?id=17440

hsteoh@quickfur.ath.cx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |pull

--- Comment #15 from hsteoh@quickfur.ath.cx ---
https://github.com/dlang/phobos/pull/6038

--

https://issues.dlang.org/show_bug.cgi?id=17440

Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy@yahoo.com

--

https://issues.dlang.org/show_bug.cgi?id=17440

--- Comment #16 from hsteoh@quickfur.ath.cx ---
Rebooted: https://github.com/dlang/phobos/pull/6043

Due to design issues with the current implementation of Nullable, it's not possible to fix the problem of having two distinct null states without extensive changes and breaking backward-compatibility.  So for now, we'll just make it so that .nullify doesn't call .destroy on class references.

--

https://issues.dlang.org/show_bug.cgi?id=17440

--- Comment #17 from github-bugzilla@puremagic.com ---
Commits pushed to master at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/5b04f455711a517cec273dad5eb4e4fda193ebff Fix issue 17440: do not call .destroy on class instances in .nullify.

https://github.com/dlang/phobos/commit/0666fc6e21b594321fe638d30865b0eef472f7a5 Merge pull request #6043 from quickfur/issue17440b

Fix issue 17440: do not call .destroy on class instances in Nullable.nullify merged-on-behalf-of: Steven Schveighoffer <schveiguy@users.noreply.github.com>

--

https://issues.dlang.org/show_bug.cgi?id=17440

github-bugzilla@puremagic.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|---                         |FIXED

--