www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Improvements to std.typecons.Nullable

reply "BLM768" <blm768 gmail.com> writes:
I've been working on a project that makes relatively heavy use of 
nullable values. I've been using std.typecons.Nullable, and it 
mostly works well, but there are some improvements that could be 
made to the implementation:

* A toString() method (needed to fix bug #10915)
* An opEquals for comparisons with the type that the Nullable 
wraps
   * Currently, comparing a null Nullable!T with a T produces an 
error,
     but it makes more sense to just return false.
* Making isNull()  property

get() might also make more sense as a property, but not with its 
current name; it would be better to make the name a noun such as 
"value" rather than a verb. If it were to be changed, it could be 
done in a fully backward-compatible way by making "get" an alias 
of "value".

These would all be simple changes, so if someone's willing to 
guide me through the formalities, I could make this my first 
contribution to Phobos.
Oct 08 2013
next sibling parent "Brad Anderson" <eco gnuk.net> writes:
On Tuesday, 8 October 2013 at 19:04:33 UTC, BLM768 wrote:
 I've been working on a project that makes relatively heavy use 
 of nullable values. I've been using std.typecons.Nullable, and 
 it mostly works well, but there are some improvements that 
 could be made to the implementation:

 * A toString() method (needed to fix bug #10915)
 * An opEquals for comparisons with the type that the Nullable 
 wraps
   * Currently, comparing a null Nullable!T with a T produces an 
 error,
     but it makes more sense to just return false.
 * Making isNull()  property

 get() might also make more sense as a property, but not with 
 its current name; it would be better to make the name a noun 
 such as "value" rather than a verb. If it were to be changed, 
 it could be done in a fully backward-compatible way by making 
 "get" an alias of "value".

 These would all be simple changes, so if someone's willing to 
 guide me through the formalities, I could make this my first 
 contribution to Phobos.

The wiki has a pretty good guide of the overall process: http://wiki.dlang.org/Pull_Requests
Oct 08 2013
prev sibling next sibling parent "BLM768" <blm768 gmail.com> writes:
On Tuesday, 8 October 2013 at 19:20:05 UTC, Brad Anderson wrote:
 The wiki has a pretty good guide of the overall process: 
 http://wiki.dlang.org/Pull_Requests

That answers most of my questions, but it seems a little... informal. I guess the formal review process doesn't really apply to minor API changes. If it's really that easy, I'll just go for it.
Oct 08 2013
prev sibling next sibling parent "monarch_dodra" <monarchdodra gmail.com> writes:
On Tuesday, 8 October 2013 at 19:04:33 UTC, BLM768 wrote:
 I've been working on a project that makes relatively heavy use 
 of nullable values. I've been using std.typecons.Nullable, and 
 it mostly works well, but there are some improvements that 
 could be made to the implementation:

 * A toString() method (needed to fix bug #10915)
 * An opEquals for comparisons with the type that the Nullable 
 wraps
   * Currently, comparing a null Nullable!T with a T produces an 
 error,
     but it makes more sense to just return false.
 * Making isNull()  property

 get() might also make more sense as a property, but not with 
 its current name; it would be better to make the name a noun 
 such as "value" rather than a verb. If it were to be changed, 
 it could be done in a fully backward-compatible way by making 
 "get" an alias of "value".

 These would all be simple changes, so if someone's willing to 
 guide me through the formalities, I could make this my first 
 contribution to Phobos.

Or we could just nuke the alias this. A Nullable!T isn't a T. It's a T handler. "alias this" allows implicit cast, which should only happen with a "is a" relation. Using it in a different context (such as nullable) is wrong, and these errors are the price we are paying for it. It's a bit more verbose, but it would solve *all* of these ambiguity and unexpected error problems. I would much rather we push in that direction instead. This also holds true for RefCounted btw. That's my take on it.
Oct 08 2013
prev sibling next sibling parent "BLM768" <blm768 gmail.com> writes:
On Tuesday, 8 October 2013 at 20:55:35 UTC, monarch_dodra wrote:
 Or we could just nuke the alias this. A Nullable!T isn't a T. 
 It's a T handler. "alias this" allows implicit cast, which 
 should only happen with a "is a" relation. Using it in a 
 different context (such as nullable) is wrong, and these errors 
 are the price we are paying for it. It's a bit more verbose, 
 but it would solve *all* of these ambiguity and unexpected 
 error problems.

Maybe... An object reference can be null, but a null "Object" isn't really an Object but the absence of one. With "alias this", the behavior meshes with the behavior of object references. Whether that's good or bad, it does add some consistency. I'm not sure what I'd like to see in the long term, but I'll leave the "alias this" in for my pull request because removing it would be a breaking change.
Oct 08 2013
prev sibling next sibling parent Nick Sabalausky <SeeWebsiteToContactMe semitwist.com> writes:
On Tue, 08 Oct 2013 22:55:34 +0200
"monarch_dodra" <monarchdodra gmail.com> wrote:
 
 A Nullable!T isn't a T. It's a T handler.

I see that as an (unavoidable) implementation detail.
 "alias this" allows implicit cast, which should 
 only happen with a "is a" relation. Using it in a different 
 context (such as nullable) is wrong, and these errors are the 
 price we are paying for it. It's a bit more verbose, but it would 
 solve *all* of these ambiguity and unexpected error problems.
 
 I would much rather we push in that direction instead.
 
 This also holds true for RefCounted btw.
 
 That's my take on it.

Personally, I find Nullable's "alias this" functionality to be a wonderful convenience. FWIW.
Oct 08 2013
prev sibling next sibling parent "BLM768" <blm768 gmail.com> writes:
On Tuesday, 8 October 2013 at 19:04:33 UTC, BLM768 wrote:
 * Making isNull()  property

Hmm... looks like it's already property. I guess this happened after the last update to the Phobos docs. I'll still need to fix the other stuff, though.
Oct 08 2013
prev sibling next sibling parent "Meta" <jared771 gmail.com> writes:
On Wednesday, 9 October 2013 at 03:42:50 UTC, Nick Sabalausky 
wrote:
 Personally, I find Nullable's "alias this" functionality to be a
 wonderful convenience. FWIW.

Yeah, it's convenient to be able to switch out T with Nullable(T) and have it work without breaking the API... Well, it sort of works, when the stars align and the blessing of Donald Knuth is upon you.
Oct 08 2013
prev sibling next sibling parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Tuesday, 8 October 2013 at 19:04:33 UTC, BLM768 wrote:
 I've been working on a project that makes relatively heavy use 
 of nullable values. I've been using std.typecons.Nullable, and 
 it mostly works well, but there are some improvements that 
 could be made to the implementation:

 * A toString() method (needed to fix bug #10915)
 * An opEquals for comparisons with the type that the Nullable 
 wraps
   * Currently, comparing a null Nullable!T with a T produces an 
 error,
     but it makes more sense to just return false.

OK, so that's two functions already. What about opCmp? What about toHash? What if T is a range? Then "Nullable!T.empty" should return true if the Nullable is empty. IF we don't, we'll get a crash in foreach.
 On Tue, 08 Oct 2013 22:55:34 +0200
 "monarch_dodra" <monarchdodra gmail.com> wrote:
 
 A Nullable!T isn't a T. It's a T handler.

I see that as an (unavoidable) implementation detail.

Is it though? C++ has done without it, and is still doing without it. It has "implicit build from" which every one says is mostly an abomination. Then here we are, bashing on their implicit constructors, yet using "implicit cast to" O_o.
 Personally, I find Nullable's "alias this" functionality to be a

I draw the line when convenience gets in the way of my programs not crashing.
Oct 08 2013
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 10/9/13 10:07 AM, monarch_dodra wrote:
 On Wednesday, 9 October 2013 at 16:34:52 UTC, BLM768 wrote:
 On Wednesday, 9 October 2013 at 06:48:31 UTC, monarch_dodra wrote:
 OK, so that's two functions already. What about opCmp? What about
 toHash?

Since ordered comparisons make no sense with null values, opCmp would need to throw an exception when working with null values anyway. That's exactly what it does right now. I've considered the other operators, and the same logic seems to apply to them. As for toHash, that needs to be overloaded whenever opEquals is. That's true for all types.
 What if T is a range? Then "Nullable!T.empty" should return true if
 the Nullable is empty. IF we don't, we'll get a crash in foreach.

It does. Null is not an "empty" range; it's the _absence_ of a range. They are not the same concept, and a null range _cannot_ be empty because the range does not exist. Therefore, it's a logic error and should throw an exception.

That was my point. Writting "Nullable!T == T" is the exact same thing: Comparison of a value with the absence of a value. It's neither equal nor different, it's an error.

I'm confused. I thought Nullable!T == T is well defined to mean "true" if a value is present and equal to the right-hand side, or "false" otherwise (the absence of a value is a singularity unequal with all objects). What's harmful about that? Andrei
Oct 10 2013
prev sibling next sibling parent "Paolo Invernizzi" <paolo.invernizzi gmail.com> writes:
On Wednesday, 9 October 2013 at 06:48:31 UTC, monarch_dodra wrote:
 On Tuesday, 8 October 2013 at 19:04:33 UTC, BLM768 wrote:
 I've been working on a project that makes relatively heavy use 
 of nullable values. I've been using std.typecons.Nullable, and 
 it mostly works well, but there are some improvements that 
 could be made to the implementation:

 * A toString() method (needed to fix bug #10915)
 * An opEquals for comparisons with the type that the Nullable 
 wraps
  * Currently, comparing a null Nullable!T with a T produces an 
 error,
    but it makes more sense to just return false.

OK, so that's two functions already. What about opCmp? What about toHash? What if T is a range? Then "Nullable!T.empty" should return true if the Nullable is empty. IF we don't, we'll get a crash in foreach.
 On Tue, 08 Oct 2013 22:55:34 +0200
 "monarch_dodra" <monarchdodra gmail.com> wrote:
 
 A Nullable!T isn't a T. It's a T handler.

I see that as an (unavoidable) implementation detail.

Is it though? C++ has done without it, and is still doing without it. It has "implicit build from" which every one says is mostly an abomination. Then here we are, bashing on their implicit constructors, yet using "implicit cast to" O_o.
 Personally, I find Nullable's "alias this" functionality to be 
 a

I draw the line when convenience gets in the way of my programs not crashing.

I think that there are some situation where the aliased nullable is just very handy, especially when you are adapting previous written code, but all that considerations are interesting. It would be wonderful to have some sort of linked "rationale", with pro and versus, with suggested use cases and possible pitfall, just in the ddoc section of the module: some sort of community-driven wiki page? -- Paolo Invernizzi
Oct 09 2013
prev sibling next sibling parent "BLM768" <blm768 gmail.com> writes:
On Wednesday, 9 October 2013 at 06:48:31 UTC, monarch_dodra wrote:
 OK, so that's two functions already. What about opCmp? What 
 about toHash?

Since ordered comparisons make no sense with null values, opCmp would need to throw an exception when working with null values anyway. That's exactly what it does right now. I've considered the other operators, and the same logic seems to apply to them. As for toHash, that needs to be overloaded whenever opEquals is. That's true for all types.
 What if T is a range? Then "Nullable!T.empty" should return 
 true if the Nullable is empty. IF we don't, we'll get a crash 
 in foreach.

It does. Null is not an "empty" range; it's the _absence_ of a range. They are not the same concept, and a null range _cannot_ be empty because the range does not exist. Therefore, it's a logic error and should throw an exception. If your range were class-based, you'd have exactly the same issue, except that instead of getting an exception, you'd get a segfault.
 On Tue, 08 Oct 2013 22:55:34 +0200
 "monarch_dodra" <monarchdodra gmail.com> wrote:

without it. It has "implicit build from" which every one says is mostly an abomination. Then here we are, bashing on their implicit constructors, yet using "implicit cast to" O_o.

A major part of the problem in C++ stems from the existence of two different forms of implicit conversion: the overloaded cast operator and copy-constructors. D does not allow implicit conversions with the latter, and it only allows implicit conversion to one type with the former, which is considered acceptable according to every C++ book I've read.
 I draw the line when convenience gets in the way of my programs 
 not crashing.

Whether you have the "alias this" or not, if you try to get a T out of a null Nullable!T, your program will throw an exception. Leaving out the "alias this" would serve only as a reminder to check for null, not as a solution to logic errors. When dealing with a Nullable!T, just like when dealing with a nullable object reference or a nullable pointer, you have to account for the null state.
Oct 09 2013
prev sibling next sibling parent "monarch_dodra" <monarchdodra gmail.com> writes:
On Wednesday, 9 October 2013 at 16:34:52 UTC, BLM768 wrote:
 On Wednesday, 9 October 2013 at 06:48:31 UTC, monarch_dodra 
 wrote:
 OK, so that's two functions already. What about opCmp? What 
 about toHash?

Since ordered comparisons make no sense with null values, opCmp would need to throw an exception when working with null values anyway. That's exactly what it does right now. I've considered the other operators, and the same logic seems to apply to them. As for toHash, that needs to be overloaded whenever opEquals is. That's true for all types.
 What if T is a range? Then "Nullable!T.empty" should return 
 true if the Nullable is empty. IF we don't, we'll get a crash 
 in foreach.

It does. Null is not an "empty" range; it's the _absence_ of a range. They are not the same concept, and a null range _cannot_ be empty because the range does not exist. Therefore, it's a logic error and should throw an exception.

That was my point. Writting "Nullable!T == T" is the exact same thing: Comparison of a value with the absence of a value. It's neither equal nor different, it's an error.
 I draw the line when convenience gets in the way of my 
 programs not crashing.

Whether you have the "alias this" or not, if you try to get a T out of a null Nullable!T, your program will throw an exception. Leaving out the "alias this" would serve only as a reminder to check for null, not as a solution to logic errors. When dealing with a Nullable!T, just like when dealing with a nullable object reference or a nullable pointer, you have to account for the null state.

My point though is that it's implicit nature makes it so that you forget about said state. Adding a simple ".get()" is not that much verbose, and sticks out that you are calling something that can be an error. You should either be operating the handler (Nullable), or the Value itself. Not a bit of "maybe the nullable, maybe the value"
Oct 09 2013
prev sibling next sibling parent "BLM768" <blm768 gmail.com> writes:
On Wednesday, 9 October 2013 at 17:07:18 UTC, monarch_dodra wrote:
 That was my point. Writting "Nullable!T == T" is the exact same 
 thing: Comparison of a value with the absence of a value. It's 
 neither equal nor different, it's an error.

Equality comparison is a bit different from properties such as emptiness. Virtually every language allows comparisons between null and non-null objects. In D and Java, this is the behavior for object references; in Ruby and Lua, this is the behavior for all values. Nullable!T was seemingly designed to emulate that sort of behavior, and overloading opEquals() would provide semantically correct behavior without any exceptions ever being thrown.
 My point though is that it's implicit nature makes it so that 
 you forget about said state.

 Adding a simple ".get()" is not that much verbose, and sticks 
 out that you are calling something that can be an error. You 
 should either be operating the handler (Nullable), or the Value 
 itself. Not a bit of "maybe the nullable, maybe the value"

Ultimately, it seems to boil down to a personal preference: should Nullable!T emulate the behavior of D's existing nullable types, or should it use a more explicit syntax? I personally lean toward consistency, in part because doing otherwise would be a breaking change that doesn't really seem justified unless we can solve the issue with _all_ nullable references. I do see the merits of your proposal, though.
Oct 09 2013
prev sibling next sibling parent "monarch_dodra" <monarchdodra gmail.com> writes:
On Thursday, 10 October 2013 at 10:09:23 UTC, Andrei Alexandrescu
wrote:
 I'm confused. I thought Nullable!T == T is well defined to mean 
 "true" if a value is present and equal to the right-hand side, 
 or "false" otherwise (the absence of a value is a singularity 
 unequal with all objects). What's harmful about that?

 Andrei

That in itself, I think is actually OK. It would be OK, because I think we can agree that a "no-value" is different from any value. In this case, we are making a call to Nullable's opEqual. I'm actually fine with this, because it is a call to Nullable's member function. The point though is that this crashed at runtime, and nobody until now noticed it, because of the "alias this". Ditto for toString. Ditto for to hash. My argument is against the "alias this" itself. It is making a cast when we don't actually expect it. Basically, any time you do a call on said nullable, you have to really think about what you are doing, because your nullable *will* be referenced on the first chance it gets. Basically, passing a Nullable!T to any function that expects a T is a silent runtime danger, which we really shouldn't have to accept.
 Ultimately, it seems to boil down to a personal preference: 
 should Nullable!T emulate the behavior of D's existing nullable 
 types, or should it use a more explicit syntax? I personally 
 lean toward consistency, in part because doing otherwise would 
 be a breaking change that doesn't really seem justified unless 
 we can solve the issue with _all_ nullable references.

Well, not quite, AFAIK, the only two "Nullable" types that exist in D are pointers, and class references. a pointer will *never* implicitly degrade to its pointed type, unless you actually dereference it, or call a function on its member. Watch: struct S{void doit();} void foo(S); S* p; Nullable!S n; p.do_it(); //passes: implicitly dereferences //thanks to an *explicit* call to do it. foo(p); //Nope p.foo(); //Nope n.doit(); //Fine; foo(n); //Fine... n.foo(); //Fine too... In those last to calls, and "unexpected" "dereference" happens: You thought you were passing n to foo()? You were wrong. As for class references, they behave pretty much the same. //---------------- I think opDispatch would have done a *much* better job at emulating a nullable type. I threw this together: //-------- struct Nullable(T) { private T _value; template opDispatch(string s) { enum ss = format(q{ static if (is(typeof({enum tmp = T.%1$s;}))) enum opDispatch = T.%1$s; else { auto ref opDispatch(Args...)(auto ref Args args) { return _value.%1$s(args); } } }, s); pragma(msg, ss); mixin(ss); } property ref inout(T) get()() inout { // 6169 : We avoid any call that might evaluate nullValue's %s, //Because it might messup get's purity and safety inference. enum message = "Called `get' on null Nullable!(" ~ T.stringof ~ ",nullValue)."; assert(!isNull, message); return _value; } //rest of the struct } //-------- And now I get this: //---- struct S { void doit(){} enum a = 1; } void foo(S){}; void main() { Nullable!S p; int i = Nullable!S.a; //OK int j = p.a; //NO problem either; p.doit(); //Fine //foo(p); //Error: function main.foo (S _param_0) is not callable using argument types (Nullable!(S)) //p.foo(); //Error: no property 'foo' for type 'S' foo(p.get); //OK! You want p's *get*. Now I get it. p.get.foo(); //OK! You want p's *get*. Now I get it. } //---- Disclaimer: My opDispatch code is not perfect. In particular, "p.foo()" may compile depending on what is imported in the module that defines the Nullable.
Oct 10 2013
prev sibling parent "Simen Kjaeraas" <simen.kjaras gmail.com> writes:
On 2013-10-10, 13:28, monarch_dodra wrote:

 On Thursday, 10 October 2013 at 10:09:23 UTC, Andrei Alexandrescu
 wrote:
 I'm confused. I thought Nullable!T == T is well defined to mean "true"  
 if a value is present and equal to the right-hand side, or "false"  
 otherwise (the absence of a value is a singularity unequal with all  
 objects). What's harmful about that?

 Andrei

That in itself, I think is actually OK. It would be OK, because I think we can agree that a "no-value" is different from any value. In this case, we are making a call to Nullable's opEqual. I'm actually fine with this, because it is a call to Nullable's member function. The point though is that this crashed at runtime, and nobody until now noticed it, because of the "alias this". Ditto for toString. Ditto for to hash. My argument is against the "alias this" itself. It is making a cast when we don't actually expect it. Basically, any time you do a call on said nullable, you have to really think about what you are doing, because your nullable *will* be referenced on the first chance it gets. Basically, passing a Nullable!T to any function that expects a T is a silent runtime danger, which we really shouldn't have to accept.

Hear hear! I've just played around with implementing a tagged union myself, and opted for explicit everywhere[0], with this being the preferred method for accessing the stored value: TaggedUnion!(string, float, int, Tuple!(long, long)) a; a.match!( (string s) => writeln("It's a string!"), (float f) => writeln("It's a float!"), (Else) => writeln("It's something else!"), ); This way, I'm always forced to handle the other cases. This work also gave me a free Nullable: alias NullableT(T) = TaggedUnion!(T, typeof(null)); I admit I have not tested the latter, so it might in fact not work very well. :p Also, opEquals proved troublesome to implement, as typeof(null) is not comparable to typeof(null). Oh, and for such a generic type, should TaggedUnion!(int, string) be comparable to TaggedUnion(int, float)? [0]: If I want to play unsafe, I can also access the stored value like so: float f = a.as!float; -- Simen
Oct 10 2013