Runtime: Add WhereNotNull to help with nullability tracking

Created on 26 Jul 2019  路  7Comments  路  Source: dotnet/runtime

See https://github.com/dotnet/roslyn/issues/37468

Currently the C# compiler can't track how the lambda in a Where method effects nullability.

For example

using System;
using System.Collections.Generic;
using System.Linq;

#nullable enable
public class C {
    public IEnumerable<string> WhereNotNull(IEnumerable<string?> strings)
    {
        return strings.Where(x => x != null);
    }
}

Warns with:

warning CS8619: Nullability of reference types in value of type 'IEnumerable<string?>' doesn't match target type 'IEnumerable'.

This could definitely be solved by improved compiler tooling, but doing so would be non-trivial.

A workaround would be to introduce a WhereNotNull method into CoreFX, so that consumers won't need to use the suppression operator themselves.

A further advantage would be that using WhereNotNull instead of Where would avoid allocating a lambda for an extremely common Linq method. This could be done either by caching the lambda as a Func<object, bool>, which avoids the need to update the rest of Linq to be aware of a new Linq method, or by creating a custom enumerator for WhereNotNull which avoids the overhead of a virtual function call, but requires updating the rest of Linq.

api-suggestion area-System.Linq needs further triage

Most helpful comment

This could definitely be solved by improved compiler tooling, but doing so would be non-trivial.

It might be non-trivial, but there are a myriad number of places this same kind of issue can show up, and I don't think we should address each as a one-off solved by adding additional methods for each such case. My preference is to wait and see what the actual broad impact is (we don't even have LINQ annotated yet), and then ideally come up with a holistic solution around such cases. If it turns out that this specific case is literally the only one that matters, then it could be relevant, but we should not rush it.

All 7 comments

A further advantage would be that using WhereNotNull instead of Where would avoid allocating a lambda for an extremely common Linq method. This could be done either by caching the lambda as a Func

If you write:
C# Where(x => x != null)
the compiler is already going to cache the delegate automatically.

This could definitely be solved by improved compiler tooling, but doing so would be non-trivial.

It might be non-trivial, but there are a myriad number of places this same kind of issue can show up, and I don't think we should address each as a one-off solved by adding additional methods for each such case. My preference is to wait and see what the actual broad impact is (we don't even have LINQ annotated yet), and then ideally come up with a holistic solution around such cases. If it turns out that this specific case is literally the only one that matters, then it could be relevant, but we should not rush it.

the compiler is already going to cache the delegate automatically.

The compiler will cache one delegate per type parameter, per use site. There are likely to be hundreds of identical such delegates across a large code base - a quick search just for "x => x != null" turned up 97 in ours. It's a small benefit, but perhaps not irrelevant.

My preference is to wait and see

Agreed. I was just bringing this up here as a point of discussion. Hopefully the Roslyn and CoreFx teams can work something out on this together. Thanks for your input!

The compiler will cache one delegate per type parameter, per use site.

This is something Roslyn could have an optimization pass to consolidate if deemed impactful. Developers shouldn't be required to manually do such folding by hand.

Maybe WhereNotDefault

Linq is now annotated and the issue remains.

41363 has some good comments:

why isn't OfType<object> good enough? Avoids the lambda, isn't particularly cryptic (imo, given the fact null is T is never true), and works great with nullability

Was this page helpful?
0 / 5 - 0 ratings

Related issues

bencz picture bencz  路  3Comments

matty-hall picture matty-hall  路  3Comments

chunseoklee picture chunseoklee  路  3Comments

btecu picture btecu  路  3Comments

jamesqo picture jamesqo  路  3Comments