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

Should warn for ?. in expression of foreach #13023

Open
ArtBlnd opened this issue Aug 9, 2016 · 6 comments
Open

Should warn for ?. in expression of foreach #13023

ArtBlnd opened this issue Aug 9, 2016 · 6 comments

Comments

@ArtBlnd
Copy link

ArtBlnd commented Aug 9, 2016

Theres no WAY to handle ?. operator with foreach.
It should emit compile warning or error.
it just thorws NullReferencedException when nullptr object in it(from GetEnumerator)

class Program
{
        class Foo
        {
            public Bar bar = null;
        }
        class Bar
        {
            public List<int> list = new List<int>(2);
        }
        static void Main(string[] args)
        {
            Foo foo = new Foo();
            //It creates ?. check assembly but doesn't handle. also try to send 
            // DWORD PTR[foo.bar] to GetEnumerator so it throws NullReferencedException(nullptr)
            foreach (int j in foo.bar?.list)
            {
                System.Console.WriteLine(j);
            }
        }
}
@ArtBlnd
Copy link
Author

ArtBlnd commented Aug 9, 2016

OR just tacitly handle foreach .? operator from CoreCLR(just passing object if nullptr(loop continue))

@jmarolf
Copy link
Contributor

jmarolf commented Aug 9, 2016

Are you saying that:

static void Main(string[] args)
{
    Foo foo = new Foo();
    foreach (int j in foo.bar?.list)
    {
        System.Console.WriteLine(j);
    }
}

Should translate to:

static void Main(string[] args)
{
    Foo foo = new Foo();
    foreach (int j in foo.bar != null 
        ? foo.bar.list
        : Enumerable.Empty<int>().ToList())
    {
        System.Console.WriteLine(j);
    }
}

?. returns null for the expression instead of throwing a null reference exception. what do you expect the following to do?

static void Main(string[] args)
{
    Foo foo = new Foo();
    foreach (int j in null as List<int>)
    {
        System.Console.WriteLine(j);
    }
}

foreach attempts to get the enumerator from the list. If the expression is null, this will always throw a null reference exception.

A simple fix for this is the ?? operator

static void Main(string[] args)
{
    Foo foo = new Foo();
    foreach (int j in foo.bar?.list ?? Enumerable.Empty<int>())
    {
        System.Console.WriteLine(j);
    }
}

@svick
Copy link
Contributor

svick commented Aug 9, 2016

I think a warning would be appropriate here. By using ?., you say that you want to get null, but foreach won't be able to handle that. Though this would probably have to be a part of a warning wave (#1580).

@gafter gafter added this to the 2.1 milestone Aug 9, 2016
@gafter gafter changed the title ?. operator shouldn't be compiled with foreach. Should warn for ?. in expression of foreach Aug 9, 2016
@ArtBlnd
Copy link
Author

ArtBlnd commented Aug 10, 2016

@svick That is actually what I mean

@HaloFour
Copy link

This would probably automatically get picked up by #5032 as a possible dereference of a nullable reference type.

@ygoe
Copy link

ygoe commented Nov 24, 2016

What if foreach actually could handle null? See #6563

@jaredpar jaredpar modified the milestones: Unknown, 15.1 Mar 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants