".?" operator with foreach doesn't creates nullptr check assembly. also no compile-time exception

[ArtBlnd]

(first please understand about my english skills. Im really bad at english)

this is example.

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();

            foreach (int j in foo.bar?.list)
            {
                System.Console.WriteLine(j);
            }
        }
}

this should throws compile-time error because there is no things to handle is foo.bar?. that is nullptr
also. even assembly JIT codegen doesn't even creates nullptr check assembly.

See dump here.

25:             Foo foo = new Foo();
00007FF87C7C4941 48 B9 38 55 80 7C F8 7F 00 00 mov         rcx,7FF87C805538h  
00007FF87C7C494B E8 F0 02 65 5F       call        00007FF8DBE14C40  
00007FF87C7C4950 48 89 45 68          mov         qword ptr [rbp+68h],rax  
00007FF87C7C4954 48 8B 4D 68          mov         rcx,qword ptr [rbp+68h]  
00007FF87C7C4958 E8 F3 C6 FF FF       call        00007FF87C7C1050  
00007FF87C7C495D 48 8B 55 68          mov         rdx,qword ptr [rbp+68h]  
00007FF87C7C4961 48 89 95 90 00 00 00 mov         qword ptr [rbp+90h],rdx  


    27:             foreach (int j in foo.bar?.list)
00007FF87C7C4969 48 8B 95 90 00 00 00 mov         rdx,qword ptr [rbp+90h]  << here is where that exception throws in run-time
00007FF87C7C4970 48 8B 52 08          mov         rdx,qword ptr [rdx+8]  
00007FF87C7C4974 48 89 55 60          mov         qword ptr [rbp+60h],rdx  
00007FF87C7C4978 48 8B 55 60          mov         rdx,qword ptr [rbp+60h]  
00007FF87C7C497C 48 89 55 58          mov         qword ptr [rbp+58h],rdx  
00007FF87C7C4980 48 8B 55 60          mov         rdx,qword ptr [rbp+60h]  
00007FF87C7C4984 48 85 D2             test        rdx,rdx  
00007FF87C7C4987 75 09                jne         00007FF87C7C4992  
00007FF87C7C4989 90                   nop  
00007FF87C7C498A 33 D2                xor         edx,edx  
00007FF87C7C498C 48 89 55 50          mov         qword ptr [rbp+50h],rdx  
00007FF87C7C4990 EB 0C                jmp         00007FF87C7C499E  
00007FF87C7C4992 48 8B 55 58          mov         rdx,qword ptr [rbp+58h]  
00007FF87C7C4996 48 8B 52 08          mov         rdx,qword ptr [rdx+8]  
00007FF87C7C499A 48 89 55 50          mov         qword ptr [rbp+50h],rdx  
00007FF87C7C499E 48 8D 55 38          lea         rdx,[rbp+38h]  
00007FF87C7C49A2 48 8B 4D 50          mov         rcx,qword ptr [rbp+50h]  
00007FF87C7C49A6 39 09                cmp         dword ptr [rcx],ecx  
00007FF87C7C49A8 E8 A3 C4 E8 5E       call        00007FF8DB650E50  
00007FF87C7C49AD C4 E1 7A 6F 45 38    vmovdqu     xmm0,xmmword ptr [rbp+38h]  
00007FF87C7C49B3 C4 E1 7A 7F 45 78    vmovdqu     xmmword ptr [rbp+78h],xmm0  
00007FF87C7C49B9 48 8B 4D 48          mov         rcx,qword ptr [rbp+48h]  
00007FF87C7C49BD 48 89 8D 88 00 00 00 mov         qword ptr [rbp+88h],rcx  
00007FF87C7C49C4 90                   nop  
00007FF87C7C49C5 EB 1D                jmp         00007FF87C7C49E4  
00007FF87C7C49C7 48 8D 4D 78          lea         rcx,[rbp+78h]  
00007FF87C7C49CB E8 D0 D1 E8 5E       call        00007FF8DB651BA0  
00007FF87C7C49D0 89 45 30             mov         dword ptr [rbp+30h],eax  
00007FF87C7C49D3 8B 4D 30             mov         ecx,dword ptr [rbp+30h]  
00007FF87C7C49D6 89 4D 74             mov         dword ptr [rbp+74h],ecx 

00007FF87C7C4969 48 8B 95 90 00 00 00 mov rdx,qword ptr [rbp+90h] << doesn't even check nullptr before. it should be like.

lea rdx, qword ptr[rbp+90h]
test rdx, rdx
jne ~ nullptr handler ~
mov rdx, [rdx]

[mikedn]

I can't reproduce this issue. The null check that ?. generates is at 00007FF87C7C4984. Then there's another null check for list at 00007FF87C7C49A6 before calling GetEnumerator and this null check will generate a NullReferenceException.

it should be like.

lea rdx, qword ptr[rbp+90h]
test rdx, rdx
jne ~ nullptr handler ~
mov rdx, [rdx]

Your code is checking the address of a local variable, that can't ever be null.

[ArtBlnd]

I don't think its checking foo.bar object. it seems like checking foo.bar.list is null.

you are saying RDX on 00007FF87C7C4969 thrown is foo.bar.list right? so it throws NullReferenceException on GetEnumerator. but after that code. its testing [rdx + 8]. so RDX cannot be foo.bar.list or something is wrong.

00007FF87C7C4970 48 8B 52 08 mov rdx,qword ptr [rdx+8] <<

00007FF87C7C4969 48 8B 95 90 00 00 00 mov         rdx,qword ptr [rbp+90h] << exception throws here
00007FF87C7C4970 48 8B 52 08          mov         rdx,qword ptr [rdx+8]  << rdx = [rdx + 8]
00007FF87C7C4974 48 89 55 60          mov         qword ptr [rbp+60h],rdx  << [rbp+60] = rdx
00007FF87C7C4978 48 8B 55 60          mov         rdx,qword ptr [rbp+60h]  << rdx = [rbp+60]
00007FF87C7C497C 48 89 55 58          mov         qword ptr [rbp+58h],rdx  << [rbp+58] = rdx
00007FF87C7C4980 48 8B 55 60          mov         rdx,qword ptr [rbp+60h] << rdx = [rbp+60]
00007FF87C7C4984 48 85 D2             test        rdx,rdx << so its testing [rdx + 8], [rdx + 8]

RBP => 00000075E54FE4C0

RBP + 90h => 0x00000075E54FE51A 00000000 00000000 00000000

[ArtBlnd]

nvm It was GetEnumerator problom.
I thought call stoped at 00007FF87C7C4969 because VS did.

[mikedn]

I thought call stoped at 00007FF87C7C4969 becouse VS did.

Yes, it seems that the managed debugger points to the wrong instruction. The native debugger does point to cmp dword ptr [rcx],ecx as expected.

[ArtBlnd]

it still looks like glitch. then it should throw compile-time exception when compile it. because it cannot be handled

[mikedn]

then it should throw compile-time exception when compile it. becouse it cannot be handled

That's a C# compiler issue, it's not related to the CLR. And the C# language doesn't require a compile time error to be generated in this case so it's not a C# compiler bug.

I suppose that the C# compiler could generate a warning but with the exception of trivial cases such as

string s = null;
Console.WriteLine(s.Lenght);

it's difficult or even impossible to prove that a variable is always null.

[ArtBlnd]

still cannot handle "?." operator in foreach.
it shouldn't be compilable.

[mikedn]

still cannot handle "?." operator in foreach.

Well, you can if do the appropriate null check:

foreach (int j in foo.bar?.list ?? new List<int>())
    System.Console.WriteLine(j);

If you would like the compiler to generate a warning for this kind of code or if you would like foreach to somehow work with nulls you should create an issue in the Roslyn repository.

[RussKeldorph]

@ArtBlnd Do you have any additional concerns beyond what @mikedn has addressed? If not, could you re-close this issue?

[whoisj]

An allocating null check seems wrong here. Use of foreach (int j in foo.bar?.list ?? new List<int>()) to simply not NRE is ridiculous. I agree with @ArtBlnd here, either the compiler should emit a warning or actually handle the GetEnumerator() -> null issue correctly.

IMO the compiler should do the equivalent of if (iter != null) { while (iter.MoveNext()) { ... } } when the enumerable is referenced via ?..

The suggestion that developers should know the "secret handshake" or that the operator doesn't work as advertised so developers should know better is incorrect. Any time "tribal knowledge" is required, we've failed the consumers of the C# language.

Well, that's my two-cents.

[mikedn]

IMO the compiler should do the equivalent of if (iter != null) { while (iter.MoveNext()) { ... } } when the enumerable is referenced via ?..

That compiler being the C# compiler so it doesn't belong in this repository. And the C# compiler does what the specification says that it should be doing so it's no a bug.

The suggestion that developers should know the "secret handshake" or that the operator doesn't work as advertised so developers should know better is incorrect. Any time "tribal knowledge" is required, we've failed the consumers of the C# language.

I don't understand what "secret handshake" or "tribal knowledge" you're talking about. The ?. operator can produce a null reference. foreach never operated on null references.

[whoisj]

I don't understand what "secret handshake" or "tribal knowledge" you're talking about. The ?. operator can produce a null reference. foreach never operated on null references.

The ?. operator offers the functionality of not performing an action if the reference is null. This is a built in nicety of the C# compiler, as is foreach. The two should work together and behave as expected, or the use-case of not playing nice with foreach should be well documented and well known.

And the C# compiler does what the specification says that it should be doing so it's no a bug.

Being spec compliant and being bug free are completely separate issues. Are you suggesting the spec is 100% perfect and free from omissions, errors, and logical mistakes? Given that there were humans involved in its crafting (even very smart and talented humans) I'm willing to bet there are issue in it.

This issue doesn't need to be a bug in the CoreClr, but it is (imo) something the Roslyn team should consider.