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

Generic expression of a derived type cannot be handled by a pattern #16195

Closed
alexwiese opened this issue Jan 3, 2017 · 12 comments · Fixed by #18784
Closed

Generic expression of a derived type cannot be handled by a pattern #16195

alexwiese opened this issue Jan 3, 2017 · 12 comments · Fixed by #18784
Assignees
Milestone

Comments

@alexwiese
Copy link

alexwiese commented Jan 3, 2017

Version Used:
Microsoft Visual Studio Professional 2017 RC
Version 15.0.26014.0 D15REL
Microsoft .NET Framework
Version 4.6.01586
Visual C# Compiler version 2.0.0.61213

Steps to Reproduce:

public class Packet
{
}

public class KeepalivePacket : Packet
{
}

public void Send<T>(T packet)
    where T : Packet
{
    if (packet is KeepalivePacket keepalive)
    {
        // Do stuff with keepalive
    }

    switch (packet)
    {
        case KeepalivePacket keepalivePacket:
            // Do stuff with keepalivePacket
            break;
    }
}

I get a compilation error for both the if statement and case statement.

CS8121: An expression of type T cannot be handled by a pattern of type KeepalivePacket

The code compiles successfully if the type is not a derived type of Packet (ie. packet is Packet keepalive or packet is object keepalive works fine).

The code compiles without error if I first cast the parameter to object.

if ((object)packet is KeepalivePacket keepalive)
{
    // This works
}

I couldn't find any mention of unsupported pattern matching with generic parameters/variables in the design notes/blog posts.

Is this expected to fail? Does this compilation error need a more informative message?

Roslyn is recommending pattern matching for the following code with IDE0019. Applying the code fix to the following code results in the compilation error above.

var keepalive = packet as KeepalivePacket;
if (keepalive != null)
{
    // Do stuff with keepalive
}

Does IDE0019 need to be updated to not apply when the variable/parameter is generic?

@HaloFour
Copy link

HaloFour commented Jan 3, 2017

That's interesting. If I had to guess type-switch pattern matching might be disabled on generic variables due to having to emit IL that would support both value and reference types. However, in this case at least, T is constrained to a reference type. If intended I do hope that this is a limitation that could be removed in the future as I see your use case as one that wouldn't be all that uncommon.

@alrz
Copy link
Member

alrz commented Jan 3, 2017

Oddly enough, it works with constrained generic types,

public class C
{
    public void Send<T, U>(T packet)
        /* where U : class */
        where T : /* class, */ U
    {
        if (packet is U keepalive)
        {
            // Do stuff with keepalive
        }

        switch (packet)
        {
            case U keepalivePacket:
                // Do stuff with keepalivePacket
                break;
        }
    }
}

Uncommenting class constraint on either generic types only causes a different code to be generated, but it compiles anyways.

@gafter
Copy link
Member

gafter commented Jan 3, 2017

Looks like a bug.

@gafter gafter added this to the 2.0 (RC.3) milestone Jan 3, 2017
@gafter gafter self-assigned this Jan 3, 2017
@gafter
Copy link
Member

gafter commented Jan 4, 2017

The reason it doesn’t work is that there is no conversion (explicit or implicit) defined from T to KeepalivePacket. Pattern matching requires such a conversion to exist, as it is defined in terms of the cast operator, which requires a conversion exist. The language specification and compiler agree that no conversion exists

        var keepalive = (KeepalivePacket)packet; // error: Cannot convert type ‘T’ to ‘KeepalivePacket’

However, the compiler does allow the following to compile (because there is no requirement that a conversion exists), and in fact it can return true:

        var isKeepAlive = packet is KeepalivePacket;

It seems strange to me that the language specification is defined such that no (explicit) conversion exists here. We'll look at what we can do about that.

@gafter gafter modified the milestones: 2.0 (RTM), 2.0 (RC.3) Jan 4, 2017
gafter added a commit to gafter/roslyn that referenced this issue Jan 5, 2017
Possible approach for fixing dotnet#16195
@gafter
Copy link
Member

gafter commented Jan 5, 2017

We're not going to do anything about this in C# 7. You'll have to add a cast to your code to work around it.

    if ((Packet)packet is KeepalivePacket keepalive)
    {
        // Do stuff with keepalive
    }

    switch ((Packet)packet)
    {
        case KeepalivePacket keepalivePacket:
            // Do stuff with keepalivePacket
            break;
    }

Once we have recursive patterns, this may be more difficult to work around. Moreover, the awkward language rule that underlies this issue (i.e. that there is no conversion from T to KeepalivePacket) doesn't make a lot of sense.

@gafter gafter modified the milestones: 2.1, 2.0 (RTM) Jan 5, 2017
@gafter
Copy link
Member

gafter commented Jan 6, 2017

The "as type" operator has special dispensation for open types. Applying that same dispensation to the "is pattern" operator may make this work.

7.10.11 The as operator

The as operator is used to explicitly convert a value to a given reference type or nullable type. Unlike a cast expression (§7.7.7), the as operator never throws an exception. Instead, if the indicated conversion is not possible, the resulting value is null.

In an operation of the form E as T, E must be an expression and T must be a reference type, a type parameter known to be a reference type, or a nullable type. Furthermore, at least one of the following must be true, or otherwise a compile-time error occurs:

  • An identity (§6.1.1), implicit nullable (§6.1.4), implicit reference (§6.1.6), boxing (§6.1.7), explicit nullable (§6.2.3), explicit reference (§6.2.4), or unboxing (§6.2.5) conversion exists from E to T.
  • The type of E or T is an open type.
  • E is the null literal.

@simonbuchan
Copy link

It seems strange to me that the language specification is defined such that no (explicit) conversion exists here. We'll look at what we can do about that.

@gafter Did you find a design note or something about this? It's quite surprising that (T)e errors in the case where the type of e and T are known to share a base, but I suppose it doesn't come up often enough to warrant the spec + compiler special-casing it.

@jaredpar jaredpar modified the milestones: 15.3, 15.1 Mar 9, 2017
@gafter gafter added this to Backlog in Compiler: Gafter Mar 10, 2017
@gafter gafter moved this from Soon to First Priority in Compiler: Gafter Mar 16, 2017
@MichaelPuckett2
Copy link

Having the same problem, different code. Visual Studio tells me it can be simplified and simplifies it for me, but I get the error as well.

gafter added a commit to gafter/roslyn that referenced this issue Apr 18, 2017
…ameters.

Fixes dotnet#16195
This is a language change for 7.1. See dotnet/csharplang#154.
Some tests may fail until dotnet#18756 is integrated.
@gafter gafter added 4 - In Review A fix for the issue is submitted for review. and removed 2 - Ready labels Apr 18, 2017
@fanoI
Copy link

fanoI commented Apr 26, 2017

@gafter so this fix will be in C# 7.1? But how the update of the compiler work? We will get an update of VS2017?

@gafter
Copy link
Member

gafter commented Apr 26, 2017

@fanol Yes, we're hoping to include this feature with C# 7.1, which we expect to ship as an update of VS2017.

gafter added a commit that referenced this issue May 8, 2017
@gafter gafter removed this from First Priority in Compiler: Gafter May 9, 2017
gafter added a commit to gafter/roslyn that referenced this issue May 18, 2017
AdamSpeight2008 pushed a commit to AdamSpeight2008/roslyn-AdamSpeight2008 that referenced this issue Jun 4, 2017
@nvmkpk
Copy link

nvmkpk commented Jun 30, 2017

If it takes longer to fix this, please update the analyzer to not suggest using pattern matching in this case.

@sjb-sjb
Copy link

sjb-sjb commented Nov 8, 2023

This seems to be an issue even for some non-generic types.

I had an interface defined which I implemented on certain exceptions:

public interface IAcceptablePrimitiveException
{
    ...
}

Then I passed the interface to a method and tested it against two exception types:

    private void Test( this IAcceptablePrimitiveException e)
    {
        bool doesntCompile = (e is not AggregateException || e is not TargetInvocationException);
        bool doesCompile = (e is AggregateException || e is TargetInvocationException);
    }

In the doesntCompile line, the message was CS8121, an expression of type 'IAcceptablePrimitiveException' cannot be handled by a pattern of type 'TargetInvocationException'. It certainly seems weird that the interface expression can be handled by a target of type AggregateException but not one of of type TargetInvocationException.

In the doesCompile line, a warning CS0184 is generated saying the expression is never of type TargetInvocationException.

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

Successfully merging a pull request may close this issue.