dotnet / csharplang

The official repo for the design of the C# programming language
11.48k stars 1.02k forks source link

Champion: Target-typed switch expression (16.3, Core 3) #2389

Open gafter opened 5 years ago

gafter commented 5 years ago

@333fred asked me

Given the following code, we cannot determine a best type for the switch expression and the code does not compile. Why can't we determine that the best type is A, given that's the target type of the expression?

class A
{
}

class B : A
{
}

class C : A
{
}

class D
{
    void M()
    {
        A a = null;
        a = a switch
        {
            B b => b,
            C c => c,
            _ => throw new System.Exception(),
        };
    }
}

Casting one branch to A fixes the code

My initial reaction was

We use the same inference algorithm for new []{ b, c } and () => { if (b) return b; else return c; } and c ? b : c and M(b, c) where T M<T>(T x, T y).  The general philosophy is that we avoid inferring a type that wasn't one of the input types.  What if they had a common interface but not a common base class?  What if they had two common interfaces?  We just don't do that. We use "target type" only for certain conversions.  We don't have a "conversion from array creation" or "conversion from conditional expression" and similarly we won't have a "conversion from switch expression".

However, my answer was not satisfactory to either @333fred or myself.

This is a proposal to "do that".

Proposal

Currently, a switch expression is specified to have a type that is the inferred common type of the expressions in the switch arms. It is (currently) required that there be such a common type.

Instead, I propose

  1. We attempt to infer a common type among the arms of the switch expression. If there is such a common type, that is the (natural) type of the switch expression.
  2. There is a switch expression conversion (a new implicit conversion-from-expression) from a switch expression to the type T if there is an implicit conversion from every arm's value expression to T.
  3. Let T be the type of the switch expression, either the target type of the switch expression conversion, or if it was not subject to such a conversion the expression's natural type. It is an error if T does not exist. It is an error if a switch arm's expression cannot be implicitly converted to T.
CyrusNajmabadi commented 5 years ago

I really like this. But i definitely beg of you to do the same for x ? y : z at the same time if you do this :)

YairHalberstadt commented 5 years ago

Is there any reason not to do this generally, for all expressions?

Thaina commented 5 years ago

If I remember correctly this feature was already requested for x ? y : z along with return type of linq and should work with struct/nullable but I don't remember the issue about it

bool b;
var i = b ? 0 : null; // i become Nullable<int>
Joe4evr commented 5 years ago

@Thaina #33.

DavidArno commented 5 years ago

I like this idea, as long as it works with interfaces, not just inherited types:

interface I
{
}

class B : I
{
}

class C : I
{
}

class D
{
    void M()
    {
        I a = null;
        a = a switch
        {
            B b => b,
            C c => c,
            _ => throw new System.Exception(),
        };
    }
}
spydacarnage commented 5 years ago

Would this work the same in a return statement, using the return-type of the method as the target type?

On Thu, 4 Apr 2019 at 08:45, David Arno notifications@github.com wrote:

I like this idea, as long as it works with interfaces, not just inherited types:

interface I { } class B : I { } class C : I { } class D { void M() { I a = null; a = a switch { B b => b, C c => c, _ => throw new System.Exception(), }; } }

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dotnet/csharplang/issues/2389#issuecomment-479788175, or mute the thread https://github.com/notifications/unsubscribe-auth/ANBBwblWPCbRIV_lZowpTgHVYlg5C8Osks5vda2HgaJpZM4cbr7a .

DavidArno commented 5 years ago

An interesting point related to this was raised in #2387. To clarify, @gafter, when you talk of "target typed", are you referring to the type of the target of the expression? For example,

int? x = y switch { … }

would this mean that if that switch were a "target typed switch", then the type of x, int?, would be used? Or would the type be inferred from the arms of the switch? In other words, would the following work:

class D
{
    void M()
    {
        A a = null;
        var a2 = a switch
        {
            B b => b,
            C c => c,
            _ => null,
        };
    }
}

where the type of the switch is inferred from a common type shared by B, C and null and thus the type of a2 is then inferred from the switch? Or would var not work here as the switch needs to know the type of a2 in advance?

alrz commented 5 years ago

the same for x ? y : z at the same time

Also useful in null-coalescing operator (from https://github.com/dotnet/csharplang/issues/877)

SyntaxNode n = expressionSyntax ?? statementSyntax;

Casting either of operands in any of these should be unnecessary.

CyrusNajmabadi commented 5 years ago

@alrz good point. I think the general idea of like to see investigated is that if we're in a position where there is some Target-type, and if you're using one of the 'branchy' expressions (i.e. switch-exoression, ?:, ??), and if that expression doesn't produce a common type, then Target-type the branches.

Both ?: and ?? can be thought of as abbreviations of the generalized switch-expression, so it makes sense to me to apply this to them. This would also hold for me in the future for any other construct like this

gafter commented 5 years ago

@DavidArno That example would not work. Either there has to be a common type (as currently specified), or there has to be a target type. That example has neither.

@spydacarnage Yes, this would work in a return statement, on the right-hand-side of an assignment statement, as an argument to a method call (even if there are different overloads with different parameter types), and even for a switch expression that is inside another switch expression. There are lots of places where there is a target type.

spydacarnage commented 5 years ago

@gafter That's excellent. I can't wait :-)

DavidArno commented 5 years ago

@gafter,

That's a great shame. I'm struggling to see why people are getting excited over it therefore as surely it only works in inheritance scenarios and so seems of limited appeal.

EDIT

Actually I partially take that back. It does have a small benefit in that:

var x = a switch
{
    B b => (I)b,
    C c => c,
     _ => throw new System.Exception(),
};

can be rewritten as:

var x = (I)(a switch
{
    B b => b,
    C c => c,
     _ => throw new System.Exception(),
});

There's still a cast in there to make it "target typed" but it's in a less obscure location. And if it does work with returns, that could be rewritten to this, I suppose:

var x = SomeSwitch(a);
I SomeSwitch(I a) => a switch
{
    B b => b,
    C c => c,
     _ => throw new System.Exception(),
};

So it's a very slight improvement on what we currently have.

spydacarnage commented 5 years ago

@DavidArno One of the main uses I can see is in MVC actions (this currently does not work without casting one of the ?: branches as IActionResult):

public IActionResult Index(int id) { User? user = GetUserFromDatabase(id); return user != null ? Ok(user) : NotFound(); }

DavidArno commented 5 years ago

@spydacarnage,

Sure, there are use-cases where the target type can be determined and used. And this feature would be useful for that.

But this doesn't address the underlying problem for me, namely that interfaces can't be used as the "common type (as currently specified)" (as per @gafter's comment). It masks the problem instead of fixing it.

spydacarnage commented 5 years ago

@DavidArno,

We may be reading the same thing differently. I assumed that @gafter's response of "that won't work" was directed to your "var a2 = a switch ..."example, because neither the switch block had a common type, and var is not a valid target type for the purposes of determining the result of the switch.

On the other hand, "IExample a2 = a switch ..." has a defined target type, and as both branches of the switch block could be represented as an IExample, it would work.

Or, at least, that's how I understood the proposal. I would certainly be less interested if it didn't work for interfaces, too...

gafter commented 5 years ago

I really like this. But i definitely beg of you to do the same for x ? y : z at the same time if you do this :)

I'm not sure that's possible. Here's the issue.

I would like this to work

    byte M(bool b) => b switch { false => 0, true => 1 };

Since the switch expression is new, we can make it target-typed (i.e. have a conversion-from-expression that is defined even if the switch expression has a natural common type of its arms) to make that work. Note that the originally proposed spec above would not make this work: you would be required to cast all arms of this switch to byte. But we cannot make this work

    byte M(bool b) => b ? 1 : 0;

because that conditional expression has type int where an expression of type byte is required. Allowing this to work would change the result of overload resolution, for example in a case like this

    void M(byte b) {}
    void M(long l) {}

    void Test(bool b) => M(b ? 1 : 0);

This calls M(long) today. If we permitted target-typing, we would see that the conditional expression can be converted to byte and that is a better target, therefore the result of this overload resolution would change to M(byte). That would be a breaking change.

So I'm thinking it may make more sense to do it for the switch expression only.

YairHalberstadt commented 5 years ago

@gafter I think the benefit of having this work for most potential conditional expressions far outweighs the benefits of having this work for all potential switch expressions

alrz commented 5 years ago

@gafter

I think the idea here is that we use the target type only if the compiler could not determine the type of :? or ?? based on current rules (which is an error at the moment).

HaloFour commented 5 years ago

Agreed, only change the behavior where it would be a compiler error today.

alrz commented 5 years ago

In fact, I think we should do the same for switch expressions as well (try to find a common type, if failed, then do the target-typing).

Consider this:

AbstractBase x = e switch { P1 => new Derived(1, 2), P2 => new(1, 2) };

if we target-type the whole expression from get-go this would fail to compile, but the compiler could infer Derived as it does today.

This also makes it consistent with :?, ?? if we ever implement target-typing for those.

edit: this is actually what is being proposed:

We attempt to infer a common type among the arms of the switch expression. If there is such a common type, that is the type of the switch expression. Otherwise, the switch expression is called a target-typed switch expression.

So if I'm not missing something, @gafter's example above would infer int not byte.

CyrusNajmabadi commented 5 years ago

because that conditional expression has type int where an expression of type byte is required. Allowing this to work would change the result of overload resolution, for example in a case like this

Hey neal, sorry if i was unclear here. My design would not be "if the conditional expression's type cannot be converted to the target type, then try to target-type the branches". Instead, it would be: "if no common type can be found between teh conditional branches and there is a target-type for the conditional expressoin, then attempt to target type the branches".

Does that make more sense? Or would there still be problems with that approach?

Thanks!

gafter commented 5 years ago

if we target-type the whole expression from get-go this would fail to compile, but the compiler could infer Derived as it does today.

Not so. There is not a conversion-from-expression from this switch expression to the target type by virtue of the proposed switch conversion, but there is a conversion from the switch's natural type to the target type, and that would therefore be used.

@CyrusNajmabadi That would work for the conditional expression, but to fix this error

    byte M(bool b) => b switch { false => 0, true => 1 }; // error: no implicit conversion from int to byte

you'd have to write this

    byte M(bool b) => b switch { false => (byte)0, true => (byte)1 };

Of course I could have made it much worse, but this is a problem I would like to solve before putting it into the language that way.

Mads suggests possibly treating switch and ?: differently. The latter would use target-typing as a fallback only when we cannot infer a common type (which is an error today), but switch would have the conversion-from-expression defined even if it has an inferred type available. We've already done something like this for tuples and interpolated strings (which have a natural type of string but a conversion-from-expression to FormattableString), so we know the compiler is wired to handle it gracefully.

CyrusNajmabadi commented 5 years ago

I'm happy if we just make ?: better :) i leave it to you guys to pick a solution you find the most acceptable given the constraints you're under. Thanks!

gafter commented 5 years ago

I've updated the proposal to make this work:

    byte M(bool b) => b switch { false => 0, true => 1 };
gulshan commented 5 years ago

I think Union/Or types proposed in my #399 will be useful to determine the "common type" and then targeting another type, be it an interface or something else. Actually the Union type would be the "common type". Something similar was suggested by @HaloFour in https://github.com/dotnet/csharplang/issues/33#issuecomment-279284400

The way it works is simple-

var result = x switch { true => new A(), false => new B() };
// the type of result here will be- A|B

If all participating types of an Union/Or type individualy inplements an interface (or their "most common type" in inheritance tree inplements that interface), target typing to that interface is possible, otherwise error-

interface IAB { }
class A : IAB { }
class B : IAB { }

// the expression
IAB result = x switch { true => new A(), false => new B() };
lkomanetz commented 5 years ago

I see the same thing with delegates as well.

private Action Test(string name) => name switch {
    "John" => () => Console.WriteLine("Hello John"),
    "Jane" => () => Console.WriteLine("Hello Jane"),
    _ => () => Console.WriteLine("Who are you?")
};

This gives me the compiler error "No best type was found for the switch expression." If I make one of them something like this

() => new Action(() => Console.WriteLine("Hello John"),

then everything is fine. It seems like the compiler should be able to deduce that I'm only returning actions with no parameters.

Joe4evr commented 5 years ago

@lkomanetz #150

gafter commented 5 years ago

This was approved today in the LDM.

We also approved the change to the conditional expression, and that is being treated as a separate feature and tracked at https://github.com/dotnet/csharplang/issues/2460. This change to the switch expression must be done in C# 8 (because it would be a breaking change to do it later). The change to the conditional expression is approved for C# 8 but could be done later if we are pressed for time.

CyrusNajmabadi commented 5 years ago

Thanks @gafter You rock!

gafter commented 5 years ago

As a practical matter we are pressed for time. We will get this change into C# 8 but we did not have time to make the corresponding change to conditional expressions (#2460) for C# 8.

gafter commented 5 years ago

An implementation for this is now in Roslyn master.

Thaina commented 5 years ago

@gafter Is this also include #33 ?

gafter commented 5 years ago

No. We are tracking that in #33 ;)