Champion "permit `t is null` for unconstrained type parameter" (16.3, Core 3)

[gafter]

• [ ] Proposal added
• [x] Discussed in LDM
• [x] Decision in LDM
• [x] Finalized (done, rejected, inactive)
• [ ] Spec'ed

Currently it is an error to write if (t is null) ... when the type of t is a type parameter that is not constrained to be a reference type. We should permit this to allow people to write that instead of if (t == null) ... uniformly when testing for null values. A code fixer to do just that has to have a special case for this and it feels quite irregular. See https://github.com/dotnet/roslyn/pull/24173. See also https://github.com/dotnet/csharplang/issues/1261.

The current spec requires that the constant expression null be converted to the type of the value being tested. That would have to be changed (for type parameters) to make this work.

Related to that, we should also permit t is 3 when the type of t is a type parameter.

More generally, when the object being tested is an open type, we treat it the same as if it is of type object for the purposes of legality of the pattern.

This was implemented in dev16: https://github.com/dotnet/roslyn/pull/25995

[ghost]

Good change, seems that v is null should help to avoid boxing allocations for some logic too. It may be the key difference between v == null.

Compare vs

Thank to JetBrains for the new feature "heap allocator". Actually analyzer suggest a suspicious code optimization for a generic parameter which cause boxing allocations with value types.

Reported to https://youtrack.jetbrains.com/issue/RIDER-13249

[CyrusNajmabadi]

IsNull<T>(this T obj) => obj == null;

If you intend this to only be used on reference types (which would make sense given they're the only things that can be null), you can do IsNull<T>(this T obj) where T : class => obj == null; This will never cause boxing.

If you do want this to work on value types as well (not sure why..., but maybe you have some generic stuff that wants to work on both, but deal with null reference types specially), then you can write it as above and know that the CLR should optimize this such that your code will not box for value types.

[ghost]

@CyrusNajmabadi Yes, thank, I understand that it is possible to use where T : class and suggested logic is very rare. :) I just think about the possible symmetry.

T v = ...

v is T /* no boxing */
v != null /* possible boxing */

v is null /* ? use non-boxing optimization [more expected for me] */
v == null /* possible boxing */

[ghost]

@CyrusNajmabadi In another words there are possible two implementations of IsNull method with the key difference in boxing.

public static bool IsNull<T>(this T o) => o == null; /* may cause boxing */
public static bool IsNull<T>(this T o) => o is T ? false : true; /* non-boxing */

I suspect that the new implementation will be equals to the second.

public static bool IsNull<T>(this T o) => o is null; /* non-boxing */

What do you think about it?

[CyrusNajmabadi]

My point is that public static bool IsNull<T>(this T o) => o == null should never cause boxing. I believe the CLR is smart enough to look for that pattern and just convert it to 'false' immediately when instantiating with value types.

[ghost]

@CyrusNajmabadi It means that JetBrains heap allocator shows invalid hint.

[gafter]

@reinspi (previously known as Makeloft) Please stop posting in this repo. We are long past any interest in your equating extension methods with pattern-matching.

[gafter]

Added:

Related to that, we should also permit t is 3 when the type of t is a type parameter that is not constrained to be a reference type.

[DavidArno]

@gafter,

Related to that, we should also permit t is 3 when the type of t is a type parameter that is not constrained to be a reference type.

What would be the scope of this addition? Would it be limited to numbers, or could I do t is "hello" too, if T isn't constrained to a value type?

[yaakov-h]

So this is interesting... for anyone following along at home:

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Attributes.Jobs;
using BenchmarkDotNet.Running;

namespace NullBenchmark
{
    class Program
    {
        static void Main(string[] args)
        {
            var summary = BenchmarkRunner.Run<IsNullVersusIsT>();
        }
    }

    static class NullCheckExtensions
    {
        public static bool IsNull<T>(this T t) => t == null;
        public static bool IsT<T>(this T t) => t is T;
    }

    [RyuJitX64Job]
    [LegacyJitX86Job]
    [MemoryDiagnoser]
    public class IsNullVersusIsT
    {
        [Benchmark]
        public bool IsNull() => 42.IsNull();

        [Benchmark]
        public bool IsT() => 42.IsT();
    }
}

Results on .NET Framework:

 Method |          Job |       Jit | Platform |      Mean |     Error |    StdDev |  Gen 0 | Allocated |
------- |------------- |---------- |--------- |----------:|----------:|----------:|-------:|----------:|
 IsNull | LegacyJitX86 | LegacyJit |      X86 | 0.0000 ns | 0.0000 ns | 0.0000 ns |      - |       0 B |
    IsT | LegacyJitX86 | LegacyJit |      X86 | 1.4537 ns | 0.0082 ns | 0.0073 ns | 0.0038 |      12 B |
 IsNull |    RyuJitX64 |    RyuJit |      X64 | 0.0087 ns | 0.0021 ns | 0.0018 ns |      - |       0 B |
    IsT |    RyuJitX64 |    RyuJit |      X64 | 0.0053 ns | 0.0039 ns | 0.0036 ns |      - |       0 B |

Results on CoreCLR (and slower CPU):

 Method |      Mean |     Error |    StdDev | Allocated |
------- |----------:|----------:|----------:|----------:|
 IsNull | 0.0416 ns | 0.0316 ns | 0.0280 ns |       0 B |
    IsT | 0.0578 ns | 0.0342 ns | 0.0303 ns |       0 B |

It would appear that t == null never boxes, but t is T does on the old Framework JIT only.

I have no idea why t == null on the old framework JIT is completely flatlined, though.

[gafter]

@DavidArno

What would be the scope of this addition? Would it be limited to numbers, or could I do t is "hello" too, if T isn't constrained to a value type?

This is specifically for the number '3' 😉

Seriously, the proposal is to permit a constant pattern of any type when the value to be matched is an "open type".

[gafter]

Fixed in https://github.com/dotnet/roslyn/pull/25995 for C# 8.0.

[yaakov-h]

Is 8.0 the next expected release after 7.3?

[SIkebe]

@yaakov-h https://github.com/dotnet/csharplang/issues/200#issuecomment-282851600

[dasMulli]

Should this only be allowed in C# 8? The current compiler happily compiles bool IsNull<T>(T v) => v is null with LangVersion 7.2 as reported on StackOverflow: https://stackoverflow.com/questions/56153403/using-the-is-operator-with-unconstrained-generics/56155441#56155441 (tried myself with 2.2.203 SDK and a LangVersion 7.2 netcoreapp2.2).

The problem this created for the OP is that their C# 7.2 program developed in VS 2019 does not build on a 2017 build server.

[333fred]

@dasMulli see https://github.com/dotnet/roslyn/issues/34678

[gafter]

@dasMulli It was only intended to be permitted in C# 8. I will investigate and file a bug and fix VS2019 (to not allow this) if needed.

[gafter]

@dasMulli That was a bug that was fixed in https://github.com/dotnet/roslyn/pull/34695.

[dasMulli]

Thanks! I was only searching for CS0403 and couldn't find the issue or PR.