Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

foreach on non-generic should not issue nullable warnings #37918

Closed
stephentoub opened this issue Feb 19, 2020 · 5 comments
Closed

foreach on non-generic should not issue nullable warnings #37918

stephentoub opened this issue Feb 19, 2020 · 5 comments
Assignees
Milestone

Comments

@stephentoub
Copy link
Member

Consider:

#nullable enable
using System.Collections;
using System.Collections.Generic;

public class C {
    public void M()
    {
        var c = new BarCollection();
        foreach (Bar b in c) { } // warning CS8606: Possible null reference assignment to iteration variable
        foreach (Zoo? z in c) { } // ok
    }
}

public class BarCollection : IEnumerable, IEnumerable<Bar>
{
    public IEnumerator GetEnumerator() => null!;
    IEnumerator<Bar> IEnumerable<Bar>.GetEnumerator() => null!;
}

public class Bar{}
public class Zoo{}

The foreach (Bar b in c) will incur a nullable warning, because IEnumerator.Current is typed to return object?. However, foreach over a non-generic IEnumerable allows the iteration variable to be of any type, without warning, as highlighted by the foreach (Zoo? z in c) which has no warnings, even though it'll blow up at run time due to a failed cast.

We should change the nullable heuristics to simply not warn for the iteration variable when foreach binds to the non-generic IEnumerable.

Updated proposal: could we special-case for nullability the case where the type being enumerated implements IEnumerable<T>, and if the dev specifies T as the iteration variable or anything in its inheritance hierarchy, and if T is defined as non-nullable, treating it as non-null?

cc: @agocke, @jcouv

@gafter
Copy link
Member

gafter commented Feb 19, 2020

We could consider modifying the library so the type of IEnumerator.Current is oblivious.

@jnm2
Copy link
Contributor

jnm2 commented Feb 27, 2020

This would solve dotnet/csharplang#3045.

@stephentoub
Copy link
Member Author

stephentoub commented Jun 15, 2020

Decision here is to change IEnumerator.Current to be oblivious in the interface definition.

@stephentoub stephentoub transferred this issue from dotnet/csharplang Jun 15, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Runtime untriaged New issue has not been triaged by the area owner labels Jun 15, 2020
@stephentoub stephentoub added this to the 5.0 milestone Jun 15, 2020
@stephentoub stephentoub removed the untriaged New issue has not been triaged by the area owner label Jun 15, 2020
@stephentoub stephentoub self-assigned this Jun 15, 2020
@danmoseley
Copy link
Member

#37969

@stephentoub
Copy link
Member Author

Fixed by #37969

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants