Roslyn: Proposal: Ternary Operator Scoping

[HaloFour]

I propose that if an identifier is introduced within the body of an expression used with the ternary conditional operator (?:) that the scope of said identifier should be narrow. Specifically said identifiers would only be accessible within the first_expression and second_expression expressions.

I give the following non-exhaustive list of reasons as to why I think that this is reasonable:

1. Said identifiers could be considered a "working space" of the current expression and would have no purpose after the final result is calculated.
2. In the case of pattern variables the identifier would almost certainly not be definitely assigned and therefore effectively unusable in the wider scope.
3. The conditional operator can be viewed as a simpler and terser form of match which would likely carry over the same narrow scoping as switch.

I'll update this proposal with additional reasons as enumerated in the comments. I just wanted to get the ball rolling.

[HaloFour]

@alrz @CyrusNajmabadi @DavidArno @Andrew-Hanlon @gafter @sharwell @eyalsk

[alrz]

I think the issue is not specific to the ternary operator, otherwise you could "ditch" if scoping,

if (e is T t && t.IsFoo ? true : false) 

I think it belongs to variable declarations and return (plus other statements e.g. invocation, object creation, etc — basically all statements except for if),

var x = e is T t && t.IsFoo; 
return e is T t && t.IsFoo;
F(e is T t && t.IsFoo);
new C(e is T t && t.IsFoo);

and I'm not even using a ternary operator here.

[CyrusNajmabadi]

One concern i have is how this interacts with declaration expressions. As expressions can be nested, and because you're now proposing a scope be at an expression level, it feels like people could end up with confusing interactions. For example:

```c#
x is Y z ? Foo(out var m) : Bar(out var n)

Another concern is with this statement:

> In the case of pattern variables the identifier would almost certainly not be definitely assigned and therefore effectively unusable in the wider scope.

Just because something is not definitely assigned does not make it unusable.  People would be free to give the value an assignment if they so wanted later on.  Similar to how we can see code that does the following:

```c#
if (!(x is Y z))
{
    // z not assigned here.
    ...
    z = something
    ...
}

Just because a pattern does not match does not make me feel that is necessary that the variable will not be used.

[alrz]

@CyrusNajmabadi As you can see in my previous comment, this is not specific to the ternary operator (expression context). I propose narrow scoping for pattern variables in all statements except for if which per your example, is an exception.

[HaloFour]

@CyrusNajmabadi

One concern i have is how this interacts with declaration expressions. As expressions can be nested, and because you're now proposing a scope be at an expression level, it feels like people could end up with confusing interactions.

This is possible. I saw the condition expression as the best place to start as I see that as being the least likely place to intend to introduce a new variable into the wider scope. I'm more than happy to explore different options.

Just because something is not definitely assigned does not make it unusable.

Indeed not. At least with if statements the block used as the embedded statement can manually assign the variable. That's not the case with the conditional operator. The only way the pattern variable would be assigned is if it is subsequently used as an out argument somewhere within the second_expression. Otherwise while the variable could eventually be assigned that value would more than likely be unrelated to anything pertaining to that original expression. Sure, it's perfectly legal to reuse variables for different purposes within the same scope, but that is definitely a code smell. (I am fully aware that this is how C# and IL assembly manage block-level scoping under the hood.)

@alrz

I'd probably agree, but it seems that trying to have different scopes for out declarations and pattern variables is a non-starter, and that the advocates for out declarations wish it to satisfy the following use case:

int.TryParse(s, out int i);
// use i here, even if parse failed

[alrz]

@HaloFour out parameters have nothing to do with "failure", TryX is just a usage pattern as long as the compiler concerned. On the other hand, the is operator is all about failure and the compiler can reason about it, hence the "definitely not assigned" error. So it would totally make sense to maintain a narrow scope around its validity. The use case @CyrusNajmabadi mentioned is specific to if statements and no more. Your example certainly does not make sense for is operator and we can safely take advantage of narrow scoping without limiting any potential use case. In fact, outside if statement, any usage of the leaked variable can be considered a code smell and should not be encouraged because it certainly hampers readability and increases complexity of the code (plus, as you said, the variable would be definitely unrelated to any subsequent code), not to mention that all those use cases involve mutation and I think we all agree that it's not a good thing (again, if is an exception, and the use cases are not that unreasonable). Furthermore, I don't see any reason for consistent scoping for is and out var as the two don't have anything in common in terms of usage and semantics.

[HaloFour]

@alrz

Furthermore, I don't see any reason for consistent scoping for is and out var as the two don't have anything in common in terms of usage and semantics.

I'd agree. So would @CyrusNajmabadi it would seem. But it sounds like the LDM decided the two would behave identically.

I've also made the "definitely not assigned" argument multiple times to no avail.

[alrz]

[CyrusNajmabadi]

I propose narrow scoping for pattern variables in all statements except for if which per your example, is an exception.

I was just using 'if' as an example of how something not-definite-assigned could still be usable.

The same is true of patterns. Someone may do a pattern test that leaves the value not-definitely assigned. However, they may still end up definitely assigning it, making is usable. Hence why i take issue with:

In the case of pattern variables the identifier would almost certainly not be definitely assigned and therefore effectively unusable in the wider scope.

The first part of the sentence is correct (the variable would not be definitely assigned), but the second part is the part that i don't agree with. It is not "effectively unusable". It simply cannot be read until it has been definitely assigned. That's something that people are completely capable of doing in their code.

[aluanhaddad]

The first part of the sentence is correct (the variable would not be definitely assigned), but the second part is the part that i don't agree with. It is not "effectively unusable". It simply cannot be read until it has been definitely assigned. That's something that people are completely capable of doing in their code.

I definitely agree with this. However, I find @HaloFour's assertion that

Sure, it's perfectly legal to reuse variables for different purposes within the same scope, but that is definitely a code smell.

to be highly compelling. For example, var declarations are mutable bindings but declaring a fresh variable instead of reassigning to an existing one is widely considered good practice<sup>(citation needed)</sup>. I think this is related to reassigning standard, by value, parameters.

[gafter]

@MadsTorgersen I think this is a suitable topic for some upcoming LDM.

[alrz]

I did a quick scan on roslyn and corefx repos, turns out, the subsequent code of all negated is operators that did a cast afterwards, is unreachable. In other words:

if (!(e is T))
{
   // return, throw, break, continue
}

var x = (T)e; // or F((T)e);

And it actually makes sense. because otherwise it's a potential bug!

So, they all can be rewritten to the following to avoid double type check (#14239):

var x = e as T ?? return; // or throw, break, continue

@CyrusNajmabadi's example if (!(x is Y z)) { z = something; } is no exception:

var z = x is Y ?? something;

Perhaps, if should also maintain narrow scoping for is var among other statements.

[paulomorgado]

I don't support this proposal in isolation. Unless the same happens with if, while and such, Like variables introduced in the for statement.

If not, then it will just be confusing!

[alrz]

@paulomorgado

I think so. A scope at an expression level is confusing. As I said. this issue is not specific to ternary operators. For example, there is absolutely no point for t in return e is T t && t.Foo; to leak.

[gafter]

The LDM discussed this yesterday, and we considered the example

c# var state = TryFoo(out int i) ? States.Found : States.NotFound; // use i;

The proposal would disallow this, but we can find nothing so wrong with this style of code that we would want to make it illegal. Confirm the status quo.

[alrz]

@gafter I think the proposal is¹ about pattern variables, not out var.

var state = e is T t ? /* use i */ : otherwise;

---

¹ was.

[gafter]

@alrz Pattern variables can be definitely assigned after the conditional expression, and useful later.

var state = complexExpression is var x && x.Something ? a : b;
// use x here

[DavidArno]

@alrz,

The team have unfortunately made the decision that is T expressions should have the same scope as out var's, so weird scoping is temporary is T bars looks like it's here to stay.

[alrz]

@gafter I think sequence/declaration expressions are more suitable than that.

var state = (var x = complexExression; x.Something) ? a : b;

In my opinion is var is just redundant since it always returns true.

[DavidArno]

@gafter,

In the absolutely vast majority of cases, they won't be useful as they will not be definitely assigned. Sure, they can be (re)assigned, but they could be declared and assigned if they didn't leak, so that offers no advantage either.

But you want consistency over usefulness and so we'll have to live with that weirdness.

[aluanhaddad]

@gafter sorry for beating a dead horse but I just find it very unfortunate that scoping rules for patterns are being defined for a version of the language lacking so many of the critical aspects of pattern-matching which were actually implemented in earlier previews. The match expression you had implemented was immediately applicable to my code and offered dramatic simplifications, improved readability, it was just a great feature overall. Sure, it was missing a couple of things, but it was a game-changingly expressive feature and it was working. I want it back now! :heart:

[eyalsk]

@aluanhaddad AFAIK the match expression is still on the table but needs more discussions, no? or they completely ditched the idea and I didn't notice?

[aluanhaddad]

@eyalsk thanks, I thought it had been removed pending reintroduction in the next version and that only the switch version was being retained for C# 7.0.

[eyalsk]

@aluanhaddad I really hope we wouldn't have to wait for C# 8 for this. :)

[CyrusNajmabadi]

match isn't on the table for C# 7. I hope it will come soon afterwards. AFAICT, everyone likes it. There just isn't enough runway for it.

[CyrusNajmabadi]

@alrz

@CyrusNajmabadi's example if (!(x is Y z)) { z = something; } is no exception:
var z = x is Y ?? something;

That wasn't my example. My example was:

c# if (!(x is Y z)) { // z not assigned here. ... z = something... ... }

[CyrusNajmabadi]

But you want consistency over usefulness and so we'll have to live with that weirdness.

Not to belabor the point, but as has been pointed out a few times "weirdness" is in the eye of the beholder. More people (than even i expected) found inconsistency between out-var and pattern scoping to be super "weird".

We felt there would be more confusion and difficulty if we ended up having to explain two entirely different sets of scoping rules between these different variables.

Once again, a situation where any solution has problems, and you have to weigh out a lot of stuff when making the decision.

[alrz]

@CyrusNajmabadi Ok, if is an exception, but it changes nothing about other statements e.g. a variable declaration, return, object creation and invocation, in all those cases any usage of the pattern variables is, more or less, a code smell, because the variable would be definitely unrelated to any subsequent code. I don't know if defining a new variable for this is too much, immutability is just a joke.

[gafter]

Ok, if is an exception, but it changes nothing about other statements e.g. a variable declaration, return, object creation and invocation, in all those cases any usage of the pattern variables is, more or less, a code smell, because the variable would be definitely unrelated to any subsequent code.

That's why the compiler complains if you attempt to read the variable. It diagnoses the code smell. But it might be definitely assigned, too:

var x = M(e is int y ? y : y=0);
// x and y are definitely assigned here

[alrz]

@gafter How about:

var y = e is int? ?? 0;
var x = M(y);
// or with declaration expressions,
var x = M(var y = e is int? ?? 0);
// use y

(I believe variable declarations should also follow out var scoping as they are always assigned.) But is is different. I'm saying that it's just about code style. If you're going to define something as fundamental as variable scoping based on some scary code to make it useful, sure, do as you please. Same goes to is var. When we have #254 and #6182, there is no need to sacrifice is scoping just to be able to do a somersault.

[gafter]

@alrz The decision to make pattern variables mutable is an assertion that assigning to them isn't "scary". So it would not make sense to base other language design decisions on an inconsistent position about that.

[DavidArno]

@CyrusNajmabadi,

Not to belabor the point, but as has been pointed out a few times "weirdness" is in the eye of the beholder.

Absolutely agree:

If you come from my position as seeing out parameters as at least a code smell, if not an outright anti-pattern, that you don't use, then is T scoping will appear ridiculous.

If you like out parameters and are excited by out var and that's your focus, then is T behaving the same way will make sense. Such folk presumably are also likely to re-assign variables, rather than treating them as immutable, so will struggle less with the strangeness of an unusable (without (re)assignment) variable being in scope.

Of course, these days, "best practice" says head down the pit of success by eg avoiding mutability, so that second position is one that only folk who do not follow the latest ideas in good practice are likely to take.

It could (and has) been argued that the language team therefore ought to be guiding folk toward good practices. But the language team have made it clear that they take an actively neutral stance on good practice: they provide what people ask for; not what would be good for them. As such, we've ended up with vast amounts of time and energy being put into out var, and scoping rules that a great many C# developers will not thank you for, rather than eg putting the time and effort into the universally popular match expression. But we are where we are...

[DavidArno]

@gafter,

Re:
cs var x = M(e is int y ? y : y=0); // x and y are definitely assigned here
Sure, y is definitely assigned here, but can you think of a genuine situation where you'd want to use y obtained in such a way? If y were genuinely useful, then that "y=0" statement shouts "code smell" to me. I'd want to follow the decades-old-but-still-good "avoid assignment in expressions" advice from my C days, and would rewrite it like the following to make it clearer:
cs var y = e is int i ? i : 0; var x = M(y); // x and y are definitely assigned here; // i is no longer useful, but sadly is still in scope here. Grr!

I really do not see any real (good practice) use for this is T leakage. Its only reason for being this way, surely, is to be consistent with out var?

[alrz]

"weirdness" is in the eye of the beholder.

In my eyes, compiler's job is to disallow any "bad" code, and it's not just about syntax and semantics, it's also about preventing accidental bugs and that's where language designers should help. I think it should try to produce compile-time errors to prevent potential runtime errors and that's just the necessary and sufficient reason to do so. I believe the more compile-time errors a compiler produces, the more powerful it is. Advertising mutability and such as a "feature" and saying that there is nothing that you cannot do with a language, whether it is shooting yourself in the foot or not, doesn't seem to be the way of designing a language. It's how you sell it.

[qrli]

c# var x = M(e is int y ? y : y=0); // x and y are definitely assigned here

This is a bad example to me. So, to use the leaky matched variable, people have to use this hack. I guess it would become a stackoverflow question&answer. Some hacks are useful, e.g. for performance, but this hack has no good reason to exist except to obfuscate the code IMO.

And unlike if, which can use {if() ... } to reduce the scope, {var x = M(e is int y ? y : 0); } would render x useless.

[alrz]

Oh, "hack" you say, it's not always bad. In case folks want to use if but do not want the variable to leak:

while (e is T t)
{
  // ...
  break;
}

easy peasy.

[DavidArno]

Inspired by @alrz, it occurs to me that local functions can be used to prevent is T variables leaking. So taking @gafter's example:
cs var x = M(e is int y ? y : y=0); // x and y are definitely assigned here
That can be rewritten as:
cs int getY(object o) => o is int y ? y : 0; var x = M(getY(e)); // y isn't in scope here. Evil leaking variable feature defeated! Yay! :)

😁

[orthoxerox]

@DavidArno, be careful, next LDM meeting notes might change that too ;)

[alrz]

@DavidArno This is so going to be an idiom. :laughing:

[DavidArno]

Further thoughts on this, again completely inspired by @alrz's if/while hack. I'm guessing most folk would probably not want to use it directly to work around variable scope leakage. However, when (if?) source generators get added to the language, then the while scope hack, along with eg using local functions to encapsulate is T expressions to stop them leaking too, could be indirectly used. A source generator could detect, eg:
cs public void F() { if (e is int x) { do something with x; } }
and transform it into:
cs replace public void F() { when (e is int x) { do something with x; break; } }
According to #15595, the generator will replace the if syntax tree with a while one during compilation, thus any attempt to use x after the if block will then generate an out-of-scope error. Coupled with analysers to eg enforce immutability by default, such generators could then form a tool set that turns C# into a "pit of success" language.

Or am I deluding myself and we'll all just learn to live with variable scope leakage?

[qrli]

@alrz I mean that specific hack to get y definitely assigned is bad. Not others.
@alrz @DavidArno Why use while when you can simply use { }?
c# { if (e is int x) { DoSomethingWith(x); } } // x is not in scope here

[qrli]

I just created #15651 to modify sequence expression a bit to allow
c# var x = { e is int y ? y : 0 } // x in scope but y is not here

[eyalsk]

@DavidArno

Or am I deluding myself and we'll all just learn to live with variable scope leakage?

There is only one question here why we need to use heavy machinery at all? this needs to somehow be baked into the language.

It doesn't make sense to me that people will use hacks/tricks or use this feature as is and then in C# 7.X or C# Y they will introduce a new way or a way to deal with scopes and then people will need to revise their code because they really wanted a narrow scope.

Now, I wish I had an elegant way to deal with it, I'd write a proposal for it but atm I don't. 😛

[aluanhaddad]

Honestly if we had match expressions people wouldn't even want to use the pattern variable outside of the scope of the pattern. Since match is an expression and therefore has a value, any declaratively builtup state would be available for projection into the result of the case clause.

[alrz]

@gafter

I found a gem in #6182

The scope of any variable declared in a parenthesized-expression is the body of that parenthesized-expression.

I understand that is not being considered at this time, just curious how that would end up being.

var x = e is T t ? t : foo;
// t is in scope

var x = (e is T t ? t : foo);
// t is not in scope

wow.

Although, it would be a breaking change if done later, since currently parentheses don't maintain their own scope. Can parenthesized-expression enable optional narrow scoping for is var and out var?

[eyalsk]

@alrz Nice! :D

Wait, will it allow us to control the scope of expressions inside if statements too?

[orthoxerox]

@eyalsk
c# if ((e is T t)) { //t is not in scope here }
:stuck_out_tongue:

[alrz]

@orthoxerox Yeah that's unfortunate. But if we want #6182, it should be decided now, otherwise it'll be a breaking change and probably would be a real concern in sequence expressions' design.

[orthoxerox]

@alrz I don't know, I think not leaking to any other scope is a good feature of sequence expressions. I think {if (e is T t) { ... }} is the only currently viable answer for scope control.

[eyalsk]

@orthoxerox Yeah, currently but that's extremely ugly and odd that for one thing we use one approach and for the other we need to use another approach just to express the same thing, I'd also argue that there's a difference between the two, I mean one narrows the scope and the other creates a new scope.

[HaloFour]

@alrz

I found a gem in #6182.

I'm going to make the giant assumption that the scoping rules mentioned there are no longer on the table. Who knows if/how sequence expressions would work now, but I imagine simply wrapping an expression in parenthesis would not be sufficient in affecting scoping elsewhere.

[CyrusNajmabadi]

@HaloFour Correct.

[DavidArno]

@CyrusNajmabadi,

Is it also correct to assume that @qrli's proposal (#15651) of using {} instead of () to contain scope will not ever happen too?

Also, is it fair to assume that based on what you have been saying on #5561, that generators won't happen any time soon too as they are too difficult, or am I jumping to conclusions there?

[eyalsk]

@CyrusNajmabadi Can you at least tell us how we _might_ narrow the scope in the future? do you guys have some ideas? I hope {if (...) {}} isn't the direction, well, we can do that now but what I meant is I hope there will be a _better_ way.

[CyrusNajmabadi]

generators won't happen any time soon too

Define "any time soon" :)

It's certainly not happening for VS17 RTM. And it's large enough that i doubt it could make it into the first update for VS17. But after that it's certainly possible. But that's also far enough out that many many many things could happen.

[CyrusNajmabadi]

Can you at least tell us how we might narrow the scope in the future?

First, i'd want to see how the C# 7 stuff is actually adopted in the wild. And if in the wild this is actually problematic and garners enough feedback to warrant addressing.

[CyrusNajmabadi]

Is it also correct to assume that @qrli's proposal (#15651) of using {} instead of () to contain scope will not ever happen too?

There's certainly no effort to make that happen for C#7. For a future release, who knows? But, as mentioned above, we're definitely going to be observing how people are using C#7 and evaluating if such work is appropriate.

[DavidArno]

@CyrusNajmabadi,

When you talk of VS17, I take it you mean VS2017?

[CyrusNajmabadi]

Yes.