dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.02k stars 4.03k forks source link

Where is the metadata emitted for `where T : IDisposable?` versus `Disposable` #39546

Closed jnm2 closed 5 years ago

jnm2 commented 5 years ago

Version Used: Master branch

According to https://github.com/dotnet/roslyn/blob/master/docs/features/nullable-reference-types.md#type-parameters, you should get a warning for using a nullable type argument for a non-nullable type parameter, and that where T : IDisposable is a not-null constraint:

using System;

public class C
{
    public void Foo1<T>() where T : IDisposable { }
    public void Foo2<T>() where T : IDisposable? { }

    void M()
    {
        // CS8631: The type 'System.IDisposable?' cannot be used as type parameter 'T' in the
        // generic type or method 'C.Foo<T>()'. Nullability of type argument
        // 'System.IDisposable?' doesn't match constraint type 'System.IDisposable'.
        Foo1<IDisposable?>();   

        // No warning
        Foo2<IDisposable?>();   
    }
}

The problem (sharplab link) is that no nullability attributes are emitted when the assembly is compiled:

// Attributes are not present in this decompiled version of the IL or in the IL itself.

public class C
{
    public void Foo1<T>() where T : IDisposable
    {
    }

    public void Foo2<T>() where T : IDisposable
    {
    }
}

Since compiling loses the distinction between : IDisposable and : IDisposable?, if you reference and use that assembly after compilation, there's no warning for using a nullable type argument:

    void M(OtherAssembly.C c)
    {
        // ❌ Should have a CS8631 warning, but doesn't!
        c.Foo1<IDisposable?>();   

        // ✔ No warning
        c.Foo2<IDisposable?>();   
    }

https://github.com/dotnet/roslyn/blob/master/docs/features/nullable-metadata.md#type-parameters mentions the encoding of notnull, class?, and class, but not the encoding of IDisposable? versus IDisposable.

jnm2 commented 5 years ago

⚠ I might be testing improperly, or maybe this issue will be just a question. When I actually built a separate assembly and added <Reference Include="path/to/assembly.dll" /> in a separate solution, it appears to actually be getting information from somewhere and actually getting this right:

    void M(OtherAssembly.C c)
    {
        // ✔ CS8631 warning
        c.Foo1<IDisposable?>();   

        // ✔ No warning
        c.Foo2<IDisposable?>();   
    }

I decompiled the assembly in dotPeek which also shows the same lack of attributes in IL view as SharpLab. How this information is being transmitted if not through nullability attributes?

jnm2 commented 5 years ago

Ah, I just realized what is happening. There is no C# syntax for attributes on generic parameter constraints (which I knew), and dotPeek and SharpLab's IL views both don't show any custom attribute syntax either. I was really trusting that the IL view would show everything. So maybe there's no ILAsm syntax for them in the first place, or maybe they are both buggy? In any case, I've found the information channel. Sorry for the bother!

dotPeek view:

image