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
18.83k stars 4k forks source link

`allows ref struct` generic can't be tested with `is` #73918

Open stephentoub opened 2 months ago

stephentoub commented 2 months ago

Version Used: Version 17.11.0 Preview 3.0 [35009.13.main]

Steps to Reproduce:

ReadOnlySpan<char> span = "Hello";
Print(span);

static void Print<T>(T item) where T : allows ref struct
{
    if (item is ReadOnlySpan<char>)
    {
        Console.WriteLine("span");
    }
}

Diagnostic Id: CS0019

Expected Behavior: Compiles

Actual Behavior:

 error CS0019: Operator 'is' cannot be applied to operands of type 'T' and 'ReadOnlySpan<char>'
jaredpar commented 2 months ago

The compiler errors here because the underlying runtime doesn't support this type of check. If the runtime provides a path for this check, I think we'd be willing to take advantage of it.

https://github.com/dotnet/runtime/blob/main/docs/design/features/byreflike-generics.md

@AaronRobinsonMSFT

stephentoub commented 2 months ago

Ok, thanks. It's not a big deal, I was just surprised it didn't work. The same thing is possible just with item.GetType() == typeof(ReadOnlySpan<char>) or typeof(T) == typeof(ReadOnlySpan<char>).

AaronRobinsonMSFT commented 2 months ago

@jaredpar Just for my understanding and I accept I could be missing something here, the following IL sequences are optimized by the runtimes - test. This doesn't use the ldnull pattern though, which is what I see is generated via SharpLab.io.

    .method public hidebysig
        instance bool BoxIsinstBranch(!T) cil managed
    {
        ldarg.1
        // Begin sequence
            box !T
            isinst ByRefLikeType
            brfalse.s NEXT_1
        // End sequence
        NEXT_1:
AaronRobinsonMSFT commented 2 months ago

@jaredpar and I chatted and it looks I didn't call out the fact that certain IL sequences are special-cased by the runtimes. See https://github.com/dotnet/runtime/blob/main/docs/design/features/byreflike-generics.md#special-il-sequences. Usage of box, isinst and unbox.any are permitted in narrow use cases.

stephentoub commented 2 months ago

So this is expected to work and should be reopened?

AaronRobinsonMSFT commented 2 months ago

So this is expected to work and should be reopened?

Yes, I believe so.

AlekseyTs commented 2 months ago

@AaronRobinsonMSFT I am a bit confused by your IL example in the https://github.com/dotnet/roslyn/issues/73918#issuecomment-2161330330 comment and by what https://github.com/dotnet/runtime/blob/main/docs/design/features/byreflike-generics.md says.

The https://github.com/dotnet/runtime/blob/main/docs/design/features/byreflike-generics.md#runtime-impact section says:

  • isinst – Will always place null on stack.

The https://github.com/dotnet/runtime/blob/main/docs/design/features/byreflike-generics.md#special-il-sequences says:

box ; isinst ; br_true/false – The box target type is equal to the unboxed target type or the box target type is Nullable<T> and target type equalities can be computed.

In the IL example (https://github.com/dotnet/roslyn/issues/73918#issuecomment-2161330330) we are boxing T and unboxing some concrete ref struct. This doesn't match the requirement for the special IL sequence. The types are not equal. I interpret it as though types of both instructions (box, isinst) should be the same, statically, in IL, not merely at runtime.

And the statement that "isinst always produces null on stack" doesn't help either. Meaning, even if the sequence doesn't cause a runtime failure, the value on the stack will always be null, therefore, even when type matches, the result will imply that it doesn't. The special IL sequence doesn't come with detailed description of its semantics, therefore I am still interpreting its semantics instruction-by-instruction. There is a phrase: "in cases where the result can be computed at JIT time and elided safely." However, this leaves us at the mercy of JIT behavior rather than prescribing/guarantying specific JIT behavior. Since I have no idea what exactly JIT is capable of at the moment, the phrase doesn't tell me much.

To summarize, I think that https://github.com/dotnet/runtime/blob/main/docs/design/features/byreflike-generics.md can benefit from clarification and various examples for special IL sequences. We can sync up on that offline. I would like this to be done before we start making any changes in compiler.