Ternary type inference failure

[masonwheeler]

Version Used:

Visual Studio 2022 / SharpLab Main (8 Dec 2021)

Steps to Reproduce:

using System;

Console.WriteLine(GetValue(false).GetType().Name);

object GetValue(bool shouldBeLong)
{
    long value = 3;
    return shouldBeLong ? value : (int)value;
}

Expected Behavior:

A human being looking at the code can clearly see that the intent of it is for GetValue to return a boxed Int64 when shouldBeLong is true, and a boxed Int32 when it is false.

Actual Behavior:

GetValue unconditionally returns a boxed Int64. Visual Studio shows a Message-level diagnostic stating that the cast to int is redundant.

[canton7]

The compiler is following the spec here:

The second and third operands, x and y, of the ?: operator control the type of the conditional expression.

• If x has type X and y has type Y then
  • If an implicit conversion (Implicit conversions) exists from X to Y, but not from Y to X, then Y is the type of the conditional expression.
  • If an implicit conversion (Implicit conversions) exists from Y to X, but not from X to Y, then X is the type of the conditional expression.
  • Otherwise, no expression type can be determined, and a compile-time error occurs.

There's an implicit conversion from int to long, so the type of the conditional expression is long. VS is correctly informing you that your cast is doing nothing.

[masonwheeler]

Good catch. But if you didn't know that, and you looked at the code itself, you can easily tell that the intent of the code as-written is not what the spec says.

.NET takes a lot of pride in the concept of "the pit of success," the idea that the system is designed well enough that code will accidentally end up doing the right thing without the developer having to be a language lawyer and know all the fiddly little details to make it turn out that way. This result violates that principle. Perhaps the spec could benefit from an update? It seems unlikely that doing so would break anything, as any existing code following this pattern is currently almost certainly doing the wrong thing anyway.

[erik-kallen]

But the code makes very little sense anyway.

[canton7]

But, to look at it a slightly different way, would you expect your code to differ from:

object GetValue(bool shouldBeLong)
{
    long value = 3;
    var result = shouldBeLong ? value : (int)value;
    return result;
}

I'd expect them to both do the same thing, but you don't?

---

If you're proposing a spec update, I think step 1 is to figure out what the update should be.

[masonwheeler]

I'd expect them to both do the same thing, but you don't?

I don't. The second version has less information available as input to the type inference mechanism, so it's not surprising that it would yield a different result.

[canton7]

Right, but to come to that conclusion you had to reason about how the type inference mechanism works and what information is available to it, which is hardly the Pit of Success.

[masonwheeler]

I look at it as reasoning about treating each Statement as a discrete piece of work.

Version 1: Statement says "compute ternary and return as function return type, which is object."
Version 2: First Statement says "compute ternary and infer type," second Statement says "return value as function return type, which is object."

If instead of var result = shouldBeLong ? value : (int)value; it was object result = shouldBeLong ? value : (int)value;, I would expect that knowledge of the expected return type, which does not exist when you use var, to be taken into account.

[CyrusNajmabadi]

Closing out as the compiler is working as per the language spec.

If you want the language to change here, please open a discussion at csharplang.

Note: a change like what you are asking is for is highly unlikely as it would definitely be a breaking change and it would be certain to affect existing codebases. We know this for certain as we have tried to tweak rules in this space, and every time we have tried we have found it breaks real code.

[CyrusNajmabadi]

I would expect that knowledge of the expected return type, which does not exist when you use var, to be taken into account.

It's reasonable for you to want this. However, until the last release, the language didn't work this way at all. Instead, we would find the common type of the ternary and convert burn branches to that. Types only flowed out of the expression, not into it.

I'm the last release we made it so that if that process failed (e.g. no common type found), then we can look at surrounding context to inform the type. However, that must only happen in the failure case to prevent code from changing meaning and now breaking.

[CyrusNajmabadi]

A human being looking at the code can clearly see that the intent

Note: that's not at all how i read it. I look at a ternary and as a human I determine what it returns independent of what context it appears in. That's a virtuous part of the language for me. That a ternary with compatible 'arm' types can have its type known independent of where it appears.

Your perspective is understandable though. It's just not what the language has ever done. This would, in effect, be relitigating subjective choices in the lang design here made over 20 years ago and which has become baked deeply into the entire ecosystem. Such relitigation is extremely rare and is almost never done when the following is true:

1. it takes existing legal code and changes the meaning of it in a significant way.
2. it is likely to hit a lot of programs that are likely to be depending on the current behavior.

In this case, both of the above are true. Which would make this sort of change one of the worst types of breaks imaginable.

--

Note: if we do create a new language, this would likely be something we'd consider up front as to whether we wanted the "inside out" type semantics we have today, or if we'd want "outside in" where an outer type could contextually feed into things (or some hybrid of both). But for C#, it would be nigh impossible to change this. Def feel free to continue the discussion over at csharplang though if you want!

[CyrusNajmabadi]

Oh. I can both convert to discussion and transfer. Done :)

[masonwheeler]

Such relitigation is extremely rare and is almost never done when the following is true:

1. it takes existing legal code and changes the meaning of it in a significant way.
2. it is likely to hit a lot of programs that are likely to be depending on the current behavior.

In this case, both of the above are true. Which would make this sort of change one of the worst types of breaks imaginable.

I find point 2 doubtful. As I noted above, it seems clear to me that any code where this pattern exists is almost certainly not doing what its writer intended. (It definitely was not in the code where I found this! I'm not the one who wrote it, but it's clear from the context that it was meant to return a boxed long in one case and a boxed int in the other, and in an IL code generator, that's a potentially significant mistake.) Therefore, it feels to me that such a change would be more likely to accidentally fix existing code than to accidentally break it.

Can you give examples of where real-world code might be intentionally depending on the current semantics?

[CyrusNajmabadi]

Therefore, it feels to me that such a change would be more likely to accidentally fix existing code than to accidentally break it.

This is very unlikely as if that code exists that means it was either untested, or very broken and no one noticed. In practice this happens rarely. Instead, of the code exists it is precisely because it is tested, does what is expected, and now cannot change without significant issue. Again, this is not speculative. We've looked into this before and created prototypes to see what effect different conditional logic would have. We did this in the past and even in c#9 when we added the new logic to handle a prior error case. In all times that we've looked into changing the behavior of existing legal code, it absolutely broke things in many repos immediately.

[CyrusNajmabadi]

I find point 2 doubtful. As I noted above, it seems clear to me that any code where this pattern exists is almost certainly not doing what its writer intended

This is absolutely incorrect. This has been the logic for c# for 20 years and we have ample experience playing in this space and seeing that if we change this it breaks things immediately.

[masonwheeler]

In all times that we've looked into changing the behavior of existing legal code, it absolutely broke things in many repos immediately.

Then you must have plenty of real-world examples you could share to support your point...

[333fred]

dotnet/roslyn#49598 is the main example from the last time we changed anything here, where we shipped a bug that broke code that compiled in C# 1. Fixing that bug then revealed everyone who had already taken a dependency on that bug in the few months since we shipped 16.8 (you can see this by the number of issues that mention this one). This area is an extremely fertile ground for intentional and unintentional dependencies, and any changes need to be very carefully considered. Breaks are pretty much never accepted.

[CyrusNajmabadi]

as fred mentions, whenever we've changed things here (either intentionally, or accidentally), we immediately find out because of how many partners are impacted and who let us know.

That said, if you want examples, roslyn alone has them (and many more can be found in our other first party teams).

As examples, but certainly not limited to, there are cases like:

https://github.com/dotnet/roslyn/blob/main/src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs#L2106

Here we do not want the '0' int to be returned as an int. It is very intentional that this is widened to a long, and we are boxing that long into an object. This was intentionally written this way and would break immediately (and only be caught by tests) if it changed meaning. This was literally just with a couple files looked at that this was found.

Note: this gets much worse with constructs like default. Currently, if you have anything like:

SomeType t = x ? willConvert : default;

then this could dramatically change meaning. Currently, default there means "will have the value of whatever default 'willConvert's type has" and then the final result is converted to SomeType. However, if you change this meaning, default will now have the value default(SomeType). You can trivially see how this can and will change values across compiles.

Consider something as simple as MyEnum? e = x ? MyEnum.SomeVal : default. You will change the above from meaning MyEnum? e = x ? MyEnum.SomeVal : default(MyEnum) to MyEnum? e = x ? MyEnum.SomeVal : default(MyEnum?). This would be bad baddy bad bad.

[masonwheeler]

You can trivially see how this can and will change values across compiles.

Sure, but that's an issue with the naked default keyword in general. IIRC the possibility of types changing elsewhere causing its semantics to change was brought up various times when it was originally being discussed as a reason not to include naked default as a language feature. You're basically arguing that "this change could cause to break a feature that is well-known to be dangerous and prone to breakage when things change."

[CyrusNajmabadi]

You're basically arguing that "this change could cause to break a feature that is well-known to be dangerous and prone to breakage when things change."

No. I'm providing examples of what you asked for. 'default' has shipped and is widely used and very effective. Your suggestion would break cases using it (as well as the other cases i mentioned). That would be a breaking change.

You don't get to ignore the impact of specific changes you are suggesting just because they overlap with other parts of the language you have a specific disagreement with.

[CyrusNajmabadi]

IIRC the possibility of types changing elsewhere causing its semantics to change was brought up various times when it was originally being discussed as a reason not to include naked default as a language feature

You recall incorrectly. There's no reason for the type to change here, or for semantics to change. :)

This is the same with 'null' as well. Each get their type contextually from their surrounding context. 'default' is simply the equivalent for 'null', except that it works with non-reference types (like value types and type parameters). When discussing 'default' we had no belief that tehre would be any possibility of the type changing, and that belief persists today. defaults type does not change because we do not break the rules of hte language.

--

And, as above, even ignoring default, this would obviously be an issue. Like i showed, roslyn itself contains code like this. Our first party partners contain code like this. The ecosystem contains code like this. This code behaves exactly as intended and spec'ed since C# 1.0. You can argue that the intent of C# 1.0 was a poor decision, and you might find those who agree with that. I, personally, woudl certainly consider revisiting this if or when we work on a new language.

However, the plain fact of hte matter is that this behavior is deeply part of C# and we absolutely will not change it and cause such breaks (esp. as it's trivially debatable if the new logic is actually desirable versus the existing logic).