Runtime: Simplify Nullable<T> usage with deconstruction semantics

[IEvangelist]

Recently I upgraded to using C# 7.* and I finally got to do more with tuples. I love the .Deconstruct capabilities and how they extend into anything. I have found this to be particularly useful for Nullable<T>. I tweeted about it here and it caught a lot of positive attention from the developer community. In fact, I was inspired to make a pull request as @jaredpar and @VSadov seemed to think it "should just be added to Nullable".

Rationale and Usage

This feature will simplify the usage of Nullable<T> access, as the consumer will have two variables namely hasValue and value - instead of an instance of the Nullable<T> that they would then have to . into.

public void TakeActionOnNullable<T>(Nullable<T> nullable)
{
    // Leveraging the generic nullable deconstruction extension method
   var (hasValue, value) = nullable;
}

Proposed API

This is my first PR in coreclr / corefx, is this what you're looking for?

[EditorBrowsable(EditorBrowsableState.Never)]
public static void Deconstruct<T>(this T? nullable, out bool hasValue, out T value)
    where T : struct
{
    hasValue = nullable.HasValue;
    value = nullable ?? default;
}

Details

This extension method is generic and will support all variations of Nullable<T>, this means that any Nullable<T> can easily be deconstructed. This is obviously an opt-in, no developer will be forced to use this. It is however an option that could be considered a more terse approach.

Pull Request

Here is the PR: https://github.com/dotnet/coreclr/pull/15605

[JonHanna]

I don't see much of an advantage here. Generally one takes one path when HasValue and another otherwise, and only cares about the value in the former case.

There are plenty of places where either a TryXXX or Deconstruct offers a performance benefit, but with nullables, in the path for HasValue is true we can use GetValueOrDefault() which is generally inlined and pretty cheap (simple field access of a value type), and not use it otherwise (by the defined semantics we'd use Value, but since we know GetValueOrDefault() will give the same result we can use it instead as an optimisation). It's not clear that the desconstruction accessing the value field could be optimised out so this would be a minor performance loss for the path where HasValue is false.

A loss I'd be happy with if I could see a case where this led to clearer code, but all I can think of so far is that it further separates the HasValue from the path that depends on it, which makes the code less clear. Can you think of a place where deconstructing would add clarity or otherwise aid readability?

[JonHanna]

If we were to have it, I would say it should definitely be an instance method on Nullable<T> rather than an extension method. (And if we really did have to have it as an extension method for some reason I'm not grasping, then on Nullable rather than a new class).

[JonHanna]

Hmm. I've thought of a counter to my first comment above. I can find a few places where there are several branches on HasValue in the same method, which means there is indeed an advantage in pulling both HasValue and Value local before the first such branch.

[karelz]

Can you share few examples where it would show the benefit?

[jkotas]

there is indeed an advantage in pulling both HasValue and Value local before the first such branch.

Both Nullable and ValueTuple are structs with two fields. Nullable will work just fine for caching Nullable in a local. You won't get much benefit by caching it in ValueTuple instead.

This proposal is a pure syntax sugar that some people find attractive. It makes things bigger and slower because of extra generic type and method instantiations need to be loaded. It does not help to make things faster or smaller.

[JonHanna]

I was thinking that it would be a readability win, but after playing with it I'm not seeing it helping.

[madelson]

It seems like a TryGetValue method would be more useful in practice. Most of the time when I would use this I’m going to immediately branch off the HasValue result. With TryGetValue we can cleanly inline by doing something like:

expression.TryGetValue(out var value) ? DoStuff(value) : ...

[IEvangelist]

@JonHanna

If we were to have it, I would say it should definitely be an instance method on Nullable rather than an extension method.

I prefer this being an extension. I am not sure I like the idea of having a Deconstruct method show up on intellisense for any nullable instance. This would mean that this code would be possible:

var nullable = new DateTime?();
nullable.Deconstruct(out bool hasValue, out DateTime value);

With an extension method this is at least more hidden away from the consumer, yeah?

[GrabYourPitchforks]

There doesn't seem to be a large audience clamoring for this. Any reason it can't be implemented as an extension method in the projects which need it? Here's a somewhat more optimized version of what was proposed earlier in the issue:

[EditorBrowsable(EditorBrowsableState.Never)]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void Deconstruct<T>(in this T? nullable, out bool hasValue, out T value)
    where T : struct
{
    hasValue = nullable.HasValue;
    value = nullable.GetValueOrDefault();
}

[ericstj]

Closing for now due to lack of recent interest as well as a good workaround from @GrabYourPitchforks.