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

C# Design Notes for Jul 15, 2016 #12939

Closed
MadsTorgersen opened this issue Aug 4, 2016 · 120 comments
Closed

C# Design Notes for Jul 15, 2016 #12939

MadsTorgersen opened this issue Aug 4, 2016 · 120 comments

Comments

@MadsTorgersen
Copy link
Contributor

C# Design Language Notes for Jul 15, 2016

Agenda

In this meeting we took a look at what the scope rules should be for variables introduced by patterns and out vars.

Scope of locals introduced in expressions

So far in C#, local variables have only been:

  1. Introduced by specific kinds of statements, and
  2. Scoped by specific kinds of statements

for, foreach and using statements are all able to introduce locals, but at the same time also constitute their scope. Declaration statements can introduce local variables into their immediate surroundings, but those surroundings are prevented by grammar from being anything other than a block {...}. So for most statement forms, questions of scope are irrelevant.

Well, not anymore! Expressions can now contain patterns and out arguments that introduce fresh variables. For any statement form that can contain expressions, we therefore need to decide how it relates to the scope of such variables.

Current design

Our default approach has been fairly restrictive:

  • All such variables are scoped to the nearest enclosing statement (even if that is a declaration statement)
  • Variables introduced in the condition of an if statement aren't in scope in the else clause (to allow reuse of variable names in nested else ifs)

This approach caters to the "positive" scenarios of is expressions with patterns and invocations of Try... style methods with out parameters:

if (o is bool b) ...b...; // b is in scope
else if (o is byte b) ...b...; // fine because bool b is out of scope
...; // no b's in scope here

It doesn't handle unconditional uses of out vars, though:

GetCoordinates(out var x, out var y);
...; // x and y not in scope :-(

It also fits poorly with the "negative" scenarios embodied by what is sometimes called the "bouncer pattern", where a method body starts out with a bunch of tests (of parameters etc.) and jumps out if the tests fail. At the end of the test you can write code at the highest level of indentation that can assume that all the tests succeeded:

void M(object o)
{
  if (o == null) throw new ArgumentNullException(nameof(o));
  ...; // o is known not to be null
}

However, the strict scope rules above make it intractable to extend the bouncer pattern to use patterns and out vars:

void M(object o)
{
  if (!(o is int i)) throw new ArgumentException("Not an int", nameof(o));
  ...; // we know o is int, but i is out of scope :-(
}

Guard statements

In Swift, this scenario was found so important that it earned its own language feature, guard, that acts like an inverted if, except that a) variables introduced in the conditions are in scope after the guard statement, and b) there must be an else branch that leaves the enclosing scope. In C# it might look something like this:

void M(object o)
{
  guard (o is int i) else throw new ArgumentException("Not an int", nameof(o)); // else must leave scope
  ...i...; // i is in scope because the guard statement is specially leaky
}

A new statement form seems like a heavy price to pay. And a guard statement wouldn't deal with non-error bouncers that correct the problem instead of bailing out:

void M(object o)
{
  if (!(o is int i)) i = 0;
  ...; // would be nice if i was in scope and definitely assigned here
}

(In the bouncer analogy I guess this is equivalent to the bouncer lending the non-conforming customer a tie instead of throwing them out for not wearing one).

Looser scope rules

It would seem better to address the scenarios and avoid a new statement form by adopting more relaxed scoping rules for these variables.

How relaxed, though?

Option 1: Expression variables are only scoped by blocks

This is as lenient as it gets. It would create some odd situations, though:

for (int i = foo(out int j);;); 
// j in scope but not i?

It seems that these new variables should at least be scoped by the same statements as old ones:

Option 2: Expression variables are scoped by blocks, for, foreach and using statements, just like other locals:

This seems more sane. However, it still leads to possibly confusing and rarely useful situations where a variable "bleeds" out many levels:

if (...)
  if (...)
    if (o is int i) ...i...
...; // i is in scope but almost certainly not definitely assigned 

It is unlikely that the inner if intended i to bleed out so aggressively, since it would almost certainly not be useful at the outer level, and would just pollute the scope.

One could say that this can easily be avoided by the guidance of using curly brace blocks in all branches and bodies, but it is unfortunate if that changes from being a style guideline to having semantic meaning.

Option 3: Expression variables are scoped by blocks, for, foreach and using statements, as well as all embedded statements:

What is meant by an embedded statement here, is one that is used as a nested statement in another statement - except inside a block. Thus the branches of an if statement, the bodies of while, foreach, etc. would all be considered embedded.

The consequence is that variables would always escape the condition of an if, but never its branches. It's as if you put curlies in all the places you were "supposed to".

Conclusion

While a little subtle, we will adopt option 3. It strikes a good balance:

  • It enables key scenarios, including out vars for non-Try methods, as well as patterns and out vars in bouncer if-statements.
  • It doesn't lead to egregious and counter-intuitive multi-level "spilling".

It does mean that you will get more variables in scope than the current restrictive regime. This does not seem dangerous, because definite assignment analysis will prevent uninitialized use. However, it prevents the variable names from being reused, and leads to more names showing in completion lists. This seems like a reasonable tradeoff.

@HaloFour
Copy link

HaloFour commented Aug 4, 2016

Yay, design notes!

Boo, changed scoping rules! I'm beating a dead horse, but I still think that it introduces inconsistencies into the language that aren't worth it. Why should an out declaration in a while leak but not an out declaration in a for? Even if the spec is written to be explicit and technically correct I guarantee that it will still feel unintuitive and unexpected.

If we kept with the previous scoping rules for out declaration variables there wouldn't need to be any new syntax introduced. We already have syntax to explicitly allow that variable to be defined in the enclosing scope:

int value;
if (int.TryParse(s, out value)) {
}

Now I do recognize that variable/type-switch patterns are a different case since they are implicitly readonly and must be declared where they are assigned. I don't have a good answer to this, short of saying just treat the pattern variable like any run-of-the-mill out variable (or use the out keyword to assign that value to said variable). But I think that something better can be proposed than deciding that half of the loop statements do one thing and the other half do something completely different.

And, to reiterate, yay design notes! 🎉 🍻

@AdamSpeight2008
Copy link
Contributor

AdamSpeight2008 commented Aug 4, 2016

The default should be that it is available in the surround scope. unless explicitly changed by the user.
eg
default

int value;
if (int.TryParse(s, out value)) {
 // value is in scope
} else
{
 // value is in scope
}
// value is in scope

local enclosing scope

if (int.TryParse(s, local out value)) {
 // value is in scope
} else
{
 // value is not in scope
}
// value is not in scope
int value;
if (int.TryParse(s, local out value)) { // error value already exists.
 // value is in scope
} else
{
 // value is not in scope
}
// value is not in scope

using @MadsTorgersen example

for (int i = foo(local out int j);;); 
// j in scope
// i in scope
}

guard

if (int.TryParse(s, guard out value)) {
 // value isnot in scope
} else
{
 // value is in scope
}
// value is not in scope

@HaloFour
Copy link

HaloFour commented Aug 4, 2016

@AdamSpeight2008

I don't see how that is an improvement. It's still internally inconsistent, but now adds the complication of all of these new keywords. Can you apply local or guard to variables declared within a for loop? Does that even make sense? I don't think so.

@AdamSpeight2008
Copy link
Contributor

@HaloFour They are contextual only applying to out var, saying which scope to introduce the variable into. The default makes sense as it simplifies the existing pattern.

if( int.tryparse( text, out value ) 

If you're going change the scope the make it explicit rather than implicit.

@MadsTorgersen
Copy link
Contributor Author

@AdamSpeight2008, @HaloFour:

A principle that the new design manages to maintain is that when it comes to scopes, all local variables are the same. It doesn't matter how they are introduced, only where they are introduced. This is very valuable in order for folks to build intuition: You can visualize the scopes as physical boundaries in the code.

It then merely becomes a question of "what establishes a scope". And yes, the weakest part of the chosen approach probably is the intuition around the fact that an if or while statement does not establish a scope for variables in its condition. Believe me, we had looong arguments around it! :-)

However, at the end of the day, the current proposal wins on having full expressiveness for the important scenarios, while having only slightly surprising scopes.

@HaloFour
Copy link

HaloFour commented Aug 5, 2016

@MadsTorgersen

I don't know, I imagine more complaints here about people not being able to reuse their variable names. Especially if they need to convert from a switch statement to a series of if/else statements and all of a sudden, again, the rules change. Which is even weirder given that case labels don't introduce scope.

Believe me, we had looong arguments around it! :-)

Not the first time: https://roslyn.codeplex.com/discussions/565640

That's probably the most jarring aspect. This has been argued out before, long since settled, and all of a sudden we get this big 180. And as this decision came out of a particularly quiet time from the team it feels even more out of the blue.

@DerpMcDerp
Copy link

DerpMcDerp commented Aug 5, 2016

I had a prior proposal (#10083) suggesting that semi-colons should be able to appear in the boolean-condition part of an if statement which gives the programmer explicit over which variables get to leak outside the condition expression. The proposal didn't support the "bouncer pattern" (because I think it's a bad idea for a mainstream programming language to leak scope like that. e.g. Pre-Standard C++'s for (int i = 0; used to leak i's scope to outside of the for loop and most people hated it) but it could be tweaked slightly to support it. i.e. the proposal could be modified to have the following semantics instead:

if (foo(out int one)) {
} else {
}

// insert a dummy "if (false) {}" to suppress scope leakage outside the if chain
if (false) {
} else if (foo(out int two)) {
} else {
    two; // two's scope ends here
}

// insert a dummy "if (true;" to suppress scope leakage to other branches
if (true; foo(out int three)) {
    three; // three's scope ends here
} else {
}

one; // one's scope ends here

I'll mention it here as a way to enhance "Option 3" with granular control of how scopes are leaked.

@bradphelan
Copy link

bradphelan commented Aug 5, 2016

The canonical examples always seem to be a bool return and an out. This is much better handled by an Option type and a pattern match

if(foo() is Some x){
   Console.WriteLine(x);
}else{
   Console.WriteLine("Got nothing");
}

though I'm not sure if the current pattern matching proposal for C# can handle the above type of patterns.

The guard pattern as described above.

void M(object o)
{
  guard (o is int i) else throw new ArgumentException("Not an int", nameof(o)); // else must leave scope
  ...i...; // i is in scope because the guard statement is specially leaky
}

again works better as an extension method on Nullable

T IfNull(this T? o, Action a){
    if(o == null) a();
    return o.Value;
}

used liked

void M(object o)
{
    int i = (o as int ).IfNull(()=>throw new ArgumentException("Not an int", nameof(o));
}

and a language extension like ruby blocks would get rid of the lambda

T IfNull(this T? o, Block &a){
    if(o == null) a();
    return o.Value;
}

void M(object o)
{
    int i = (o as int ).IfNull {
        throw new ArgumentException("Not an int", nameof(o))
    }
}

No need for weird scoping rules.

@MadsTorgersen
Copy link
Contributor Author

@HaloFour: Thanks for digging out the design notes where the previous design came from. We went back and looked at those arguments and found that we no longer believed in them. I acknowledge that this came "out of the blue" in the sense that it's a late design change; in fact (knock on wood) the last one for C# 7.0.

The fact is that we've been mulling it for a couple of months, but only had the extreme options on the table (the restrictive design vs. what is "Option 1" above). Both were really unacceptable. Only a couple of weeks ago did we come up with the compromise that is option 3. I think it retains the full upside of option 1 (supporting meaningful use of out vars in top level expression statements, supporting the bouncer pattern for patterns and out vars). At the same time it limits "variable spilling" to a reasonably intuitive set that are declared "near the top level".

Sometimes you want those variables in scope, sometimes you don't.

  • If you don't and they are there anyway, what's the harm? a) some variable names are taken and not available, and b) it may be a bit confusing until you get the hang of it
  • If you do and they aren't there? c) Significant and clunky rewriting of your code or even d) you have no way of using the new features.

While this isn't a slam-dunk trade-off, in the end we think a and b are lesser evils than c and d.

@DerpMcDerp: It's an explicit goal not to allow granular control of the scope of a given variable. A given construct should be a scope, and all variables contained inside it are scoped by it, regardless of how they are declared.

(This principle is violated by dubious legacy behavior inside switch statements that unfortunately cannot be fixed in a non-breaking fashion. We'll consider this a wart and live with it).

@bradphelan: If we could start over we might have avoided the TryFoo(out ...) pattern, or maybe even out parameters altogether. But the state of the world is that there are lots of out parameters out there, and as long as we're introducing variables in expressions anyway (with patterns) it seems good and right to use that to also improve the consumption experience of those out parameters. For Try methods, you can now think of them almost as a way to write your own active pattern.

@bradphelan
Copy link

@madstorgesson yeah that's a nice way to think of out params as being
active pattern. Thanks for the insight.

On Fri, 5 Aug 2016, 18:20 Mads Torgersen, notifications@github.com wrote:

@HaloFour https://github.com/HaloFour: Thanks for digging out the
design notes where the previous design came from. We went back and looked
at those arguments and found that we no longer believed in them. I
acknowledge that this came "out of the blue" in the sense that it's a late
design change; in fact (knock on wood) the last one for C# 7.0.

The fact is that we've been mulling it for a couple of months, but only
had the extreme options on the table (the restrictive design vs. what is
"Option 1" above). Both were really unacceptable. Only a couple of weeks
ago did we come up with the compromise that is option 3. I think it retains
the full upside of option 1 (supporting meaningful use of out vars in top
level expression statements, supporting the bouncer pattern for patterns
and out vars). At the same time it limits "variable spilling" to a
reasonably intuitive set that are declared "near the top level".

Sometimes you want those variables in scope, sometimes you don't.

  • If you don't and they are there anyway, what's the harm? a) some
    variable names are taken and not available, and b) it may be a bit
    confusing until you get the hang of it
  • If you do and they aren't there? c) Significant and clunky rewriting
    of your code or even d) you have no way of using the new features.

While this isn't a slam-dunk trade-off, in the end we think a and b are
lesser evils than c and d.

@DerpMcDerp https://github.com/DerpMcDerp: It's an explicit goal not
to allow granular control of the scope of a given variable. A given
construct should be a scope, and all variables contained inside it are
scoped by it, regardless of how they are declared.

(This principle is violated by dubious legacy behavior inside switch
statements that unfortunately cannot be fixed in a non-breaking fashion.
We'll consider this a wart and live with it).

@bradphelan https://github.com/bradphelan: If we could start over we
might have avoided the TryFoo(out ...) pattern, or maybe even out
parameters altogether. But the state of the world is that there are lots of
out parameters out there, and as long as we're introducing variables in
expressions anyway (with patterns) it seems good and right to use that to
also improve the consumption experience of those out parameters. For Try
methods, you can now think of them almost as a way to write your own active
pattern.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#12939 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABE8gP2mrMuByixMNg8ypD61ZypD_cKks5qc2LMgaJpZM4JdLzM
.

@mungojam
Copy link

mungojam commented Aug 5, 2016

I'm confused how option 3 supports the bouncer pattern. In your example. isn't i still out of scope because of the implicit braces around the if branch?

void M(object o)
{
  if (!(o is int i)) throw new ArgumentException("Not an int", nameof(o));
  ...; // we know o is int, but i is still out of scope with option 3 isn't it?
}

seems to be equivalent to:

void M(object o)
{
  if (!(o is int i)) 
  { 
      throw new ArgumentException("Not an int", nameof(o)); 
  }
  ...; // i is still out of scope due to block?
}

@Eyas
Copy link
Contributor

Eyas commented Aug 7, 2016

@mungojam Implicit braces around a branch affect patterns, declarations, etc. inside of the branch (between the implicit braces), the declaration inside of the condition part of the if statement continues to 'bleed'.

Since C# provides us with definite assignment analysis, I agree this sounds like a clean approach.

@mungojam
Copy link

mungojam commented Aug 7, 2016

Thanks @Eyas, yes, I follow now, the variables are escaping to outside the if block, not just into each branch, and definite assignment analysis is doing its magic.

Sounds like a very reasonable compromise to me, in current code the variable had to be declared outside the if block anyway, e.g. when using Try...(out result)

@Shiney
Copy link

Shiney commented Aug 8, 2016

This might be enough of an edge case for the bouncer pattern that it probably doesn't matter, but what happens if boolean operators that cause short circuiting are used in the conditional in the if block?

E.g.

if(dictionary != null  && dictionary.TryGetValue("key", out int j)
{  
    //Stuff
}
// Is j in scope here? What happens if the dictionary was null?

@stepanbenes
Copy link

@Shiney I understand that the variable j will be in scope after if, but it won't be definitely assigned.

@HaloFour
Copy link

HaloFour commented Aug 8, 2016

@Shiney

It's no different than the following that you can write today:

int j;
if(dictionary != null  && dictionary.TryGetValue("key", out j)
{  
    // j is in scope and is definitely assigned
}
// j is in scope but is not definitely assigned

@HaloFour
Copy link

HaloFour commented Aug 8, 2016

@MadsTorgersen

What about the scope of out declaration variables within LINQ queries? Allowing them to leak throughout the LINQ query would be very useful, like an automatic range variable. But if they leak to the enclosing scope they could potentially produce concurrency issues if the LINQ query is parallel.

I imagine that people will expect the following to "Just Work"™:

IEnumerable<string> strings = GetSomeStrings();
var query = from s in strings
    where int.TryParse(s, out int i)
    select i;

// is i in scope here?

@HaloFour
Copy link

HaloFour commented Aug 8, 2016

@MadsTorgersen

Actually, thinking about it some more, specifically with pattern variables, the scope changes make little sense. As long as pattern variables remain implicitly readonly, the scope is leaking into scopes where the identifier could never be definitely assigned. As such, it's just a wasted identifier.

@Eyas
Copy link
Contributor

Eyas commented Aug 8, 2016

@HaloFour doesn't seem wasted in general-- as you said in the different thread:

It seems much of the point of these scoping rules is to avoid the tons of extra syntax that Swift requires in order to influence the scope.

That seems worth having a wasted identifier in some cases, while allowing the identifier to be useful in the opposite case:

if (!x is Type t) throw new IllegalArugmentException(nameof(x));
// can still use t

Are you saying it is worth having special statements or special scoping rules just to get this set of use cases to work?

@HaloFour
Copy link

HaloFour commented Aug 8, 2016

@Eyas

For the bouncer case it's probably fine. t would be definitely assigned at least where you'd expect to use it. But in any other situation you'd be left with t where 2 out of the 3 scopes where it exists it cannot be used nor can it be updated. At least out variables can be updated so you can assign some fallback value.

@MadsTorgersen
Copy link
Contributor Author

@HaloFour and others,

I should clarify a few things that weren't in the notes above.

_Pattern variables are mutable_. For instance you can write:

if (o is int i || (o is string s && int.TryParse(s, out i)))
{
    // i is definitely assigned and o is either an int or a string with an int in it
}

There are a couple of things that also already establish scopes, that I forgot to mention, because they aren't statements:

_lambda expressions already establish scopes_ to carry their parameters. These scopes also contain any variables defined inside the body of even expression bodied lambdas.

_Query clauses already establish scopes_ because query expressions are defined in terms of syntactic translation to lambda expressions which constitute scopes.

It is true that it would be nice to allow them the same scope as that of variables introduced with from and let clauses (which turn into separate lambda parameters for each clause in the translation). We don't know how to do that in practice, though.

_Catch clauses already establish scopes_ for the exception specifiers, and any filter expression on the catch clause is contained within that scope.

Hope that helps!

Mads

@svick
Copy link
Contributor

svick commented Aug 8, 2016

@HaloFour What would that query compile into? A combination of Select() and Where(), like this?

var query = strings
    .Select(s => { int i; bool res = int.TryParse(s, out i); return new { s, i, res }; })
    .Where(x => x.res)
    .Select(x => x.i);

I'm not sure this kind of translation would be expected. How the translation looks like is especially important for IQueryable and custom LINQ providers.

@HaloFour
Copy link

HaloFour commented Aug 8, 2016

@svick

In short, yes. The compiler could emit tuples to make it more efficient, which was/is on the board for LINQ expressions which project temporary anonymous types for range variables today like let. The translation above should work fine with expression trees, although I agree that most queryable providers probably wouldn't understand it, but how many of them would understand methods with out parameters today regardless of what you did with the scope?

Basically an extension of #6877 where the implementation concerns were already mentioned. But with let seeming less and less necessary there likely wouldn't be another opportunity to add said functionality to LINQ, which means that pattern matching wouldn't have a place in stream processing, which in my opinion would be a pretty big fail.

@paulomorgado
Copy link

I really think that it's a bad idea that a variable introduced in the condition of a if or while leak that statement.

In Try... methods, if the user wants the variable to leak, then just write the code the way it can be written today.

Patterns are another issue easily solved by using another variable declared outside the statement:

int j;
if (o is int i || (o is string s && int.TryParse(s, out i)))
{
    j = I;
}
else
{
    j = -1;
}
// I should not be in scope here!!!

@alrz
Copy link
Member

alrz commented Oct 27, 2016

Whatever is decided upon, will definitely affect other features like declaration expressions (#254) which can't be super useful compared to its statement counterpart if it doesn't restrict its own scope. In my opinion, when a language introduce a new idiom, it should make it preferable over other alternatives wherever it can; something that did not happened with string interpolation. We already have the "leaky" version of this: just declare the variable beforehand. Now, if we strive for readability and conciseness I think it's better to reconsider other mechanisms to help with that while we also take advantage of the scoping, like guard or let to see if they address said issues instead of accepting the current situation as permanent (because it's not). Or just postpone it until to the next version when we see the clear picture of all features that need to be consistent with each other together. However, that would make the current next version a point release.

@DavidArno
Copy link

@vbcodec,

Whilst it's being nice having hugely disparate voices united in condemnation of this badly thought through feature, I have to utterly disagree with your ideas on variable scope. The following is more wrong than @MadsTorgersen's daft leakage ideas:

object n = 1;
if (n is int n)
{
    // n here as int
}
// n here as object

The above goes completely against the way scope currently works in that an inner-scoped var cannot be delared with the same name as an existing one. The same rules should apply to out var and is var.

@alrz
Copy link
Member

alrz commented Oct 27, 2016

@DavidArno @vbcodec To me, it looks like variable shadowing but just like existing out arguments I think it's useful to be able to bind to existing variables also in patterns (#13400) which happen to address widening scope of the said variables into the enclosing block, though with a little more typing.

@AdamSpeight2008
Copy link
Contributor

Alternative #14777

@vbcodec
Copy link

vbcodec commented Oct 27, 2016

@DavidArno
This retype of variables is new concept, as currently variables have static types, regardless of context. Really there will be second hidden variable (int for this example), that will be used inside new scope, and original variable must be updated, when hidden copy was changed.. But this retyping may be helpful, as determining new non-conflicting name for variable, very often is laborious and irritating.

@alrz

Or just postpone it until to the next version when we see the clear picture of all features

This is best case if they won'f fix it (and their POV BTW). But there are other factors. New VS is upcoming, and they must enumerate some new significant features into marketing media, and binary literals or digit separators do not look like holy grail.
Team also said that they want implement features in piecemeal fashion, where feature is maturing over multiple language versions, than in one big step. This is risky strategy, as for many features beginning is determined by the end, if feature should be working right.

@iam3yal
Copy link

iam3yal commented Oct 27, 2016

@vbcodec In my opinion this sort of shadowing can be confusing and introduce even more bugs.

@vbcodec
Copy link

vbcodec commented Oct 28, 2016

@eyalsk
As for implementation, better way is to use hidden variable defined similar to ByRef / ref.
Do you mean implementation or conceptually troubles ? Do not think there are any problems, except short time adaptation by users.
Can you provide examples how it is wrong ?

@iam3yal
Copy link

iam3yal commented Oct 28, 2016

@vbcodec

What I wrote was just a general statement but I see at least three problems here:

  1. It's unlikely that n is used for the same reason, thus, a different name would be more suitable.
  2. It's likely that you might want to access n the object inside the scope of the condition due to what I described in paragraph 1.
  3. Finally, it might not be easy to follow the flow of logic, especially with multiple shadowed variables , hence, why it's going to be confusing.

@vbcodec
Copy link

vbcodec commented Oct 28, 2016

@eyalsk

It's unlikely that n is used for the same reason, thus, a different name would be more suitable.

Yes, outside I want object, inside I want integer. What I want is different type, not different name. Such retyping is like function overloading or overriding, which is pretty good idea.

It's likely that you might want to access n the object

This is real issue, but easy to remove, as you can use different name for this case. Retyping is optional.

Finally, it might not be easy to follow the flow of logic, especially with multiple shadowed variables , hence, why it's going to be confusing.

This is pretty relative, and heavily depend how well and clear code is created. Multiple retyping can create mess, and multiple random names also can create mess. But retyping has two advantages:
1 You always know where is source of retyped variable. This may be improved by intellisense that show both original and current type.
2. Retyping allow to update structures, because single object is always used. Without it there is need to use explicit cast, which is bit messier.

@iam3yal
Copy link

iam3yal commented Oct 28, 2016

@vbcodec

Yes, outside I want object, inside I want integer. What I want is different type, not different name. Such retyping is like function overloading or overriding, which is pretty good idea.

Well, if that's what you want then I'm not gonna argue about it but to me it doesn't make sense, I'd probably want to give it a more specialized name.

I don't think that function overloading or overriding or even members that hides through inheritance and were redeclared has anything to do with it nor they are applicable to your argument.

This is real issue, but easy to remove, as you can use different name for this case. Retyping is optional.

So it's optional 20% of the time because after some processing in most cases I'd imagine that you may want to assign a value back to the original variable.

Multiple retyping can create mess, and multiple random names also can create mess.

So if one thing can create a mess and another already creates a mess, do you want to create a chaos? 😄

1 You always know where is source of retyped variable. This may be improved by intellisense that show both original and current type.

If you can't reach the variable inside the scope because it's being shadowed, why exactly do we need the intellisense to show any information about it?

  1. Retyping allow to update structures, because single object is always used. Without it there is need to use explicit cast, which is bit messier.

I'm not sure I understand this part, for it to be used as an int, it needs to be unboxed first so you must cast it, the name doesn't really matter.

@vbcodec
Copy link

vbcodec commented Oct 28, 2016

@eyalsk
Maybe I didn't stated it clearly. Retyped variable isn't separate variable with the same name as 'external' variable and different type. It is just pointer (like those in C++), linked to 'external' variable. So, it has consequences:

  • changing value of retyped variable (pointer), changes value of original variable
  • structures and primitive types are not copied to new variable (as new variable is pointer)
  • intelisense know type of pointer and type of original variable, so can display both types
  • retyping retyped variable do not create new pointer to previous pointer, but new pointer to original variable

Currently pointers can be defined only as input parameters defined with ByRef / ref keywords, I suggest to be created also in typeswitch expressions, if names of variables (checked and created) are identical.

@HaloFour
Copy link

@vbcodec Pointer or not that would still represent a variable that is shadowing another variable. The fact that they might "point" to the same thing doesn't matter. Also, such a proposal does nothing to address this problem in the greater picture of recursive pattern matching as there would be no original variable to shadow.

@DerpMcDerp
Copy link

@vbcodec Your idea can't work because it would make the type system unsound as C# allows write access to the shadowed variable via variable capture:

object str = "asdf";
System.Action bar = () => { str = 3; };
if (str is string) {
    bar();
}

@Andrew-Hanlon
Copy link
Contributor

Andrew-Hanlon commented Nov 18, 2016

The decision should not be based around 'reducing code lines' but instead about reducing explicit variable typing while maintaining scoping sanity. For me the trade off should be having to create a condition variable when necessary to have wider scope:

var condition = s.TryParse(out var i, out var j);
if(!condition) return;
//i, j are in scope

The same logic applies to pattern matching variables: Choose consistent scoping as default, and fall back to outer declaration for wider scoping.

...Or create new keywords that make leakage an explicit choice. It is a shame that there is an assumptive justification around 'new keywords' being somehow worse than unintuitive and contradictory semantics.

@iam3yal
Copy link

iam3yal commented Nov 19, 2016

@Andrew-Hanlon Because this ship has sailed we have two options here drown or adapt. 😆

@Andrew-Hanlon
Copy link
Contributor

Andrew-Hanlon commented Nov 19, 2016

@eyalsk Begrudgingly adapt...

For not wanting to add keywords, the new C# 7 {if} and {while} statements are an odd inclusion:

{if(n is int i && i > 5) return;}

😉

@alrz
Copy link
Member

alrz commented Nov 19, 2016

This is probably mentioned before, but looking into the future, for this specific use case, I personally prefer tuples and pattern matching over out params when it comes to returning multiple values from a method.

// TryX returns a nullable tuple
let (result1, result2) = TryX(arguments, moreArguments) else return;
// TryX returns bool
if (!TryX(arguments, moreArguments, out var result1, out var result2)) return;

let was proposed exactly to introduce pattern variables in a broader scope without requiring an additional indention in the first place. The downside to this approuch is that this needs modifications on the declaration-site rather than just the use-site, but in my opinion it wouldn't be an issue when you actually want to refactor to use new features.

EDIT: F# special cased this and you can use this pattern without touching the method declaration.

match Int32.TryParse str with
| true, value -> // succeed
| _ -> // failed

@tamlin-mike
Copy link

This is the largest disaster I've ever seen when it comes to computer language design.

@CyrusNajmabadi
Copy link
Member

@Kaelum Messaged you offline about some additional questions i have. Please let me know if you get the message or not. Thanks!

@CyrusNajmabadi
Copy link
Member

Hrmm. i tried your @me.com email address. Is there a better one i can try? You can reach me at cyrusn@microsoft.com. Thanks!

@timadamson
Copy link

timadamson commented Nov 24, 2016

void M(object o)
{
  if (!(o is int i)) i = 0;
  ...; // would be nice if i was in scope and definitely assigned here
}

My god this is horrid, please don't let this become best practice. What's wrong with:

void M(object o)
{
  int i = o as int ?? 0;
}

@CyrusNajmabadi
Copy link
Member

What's wrong with: ...

It doesn't work for more than simple expressions. For example, any code that performs multi-statement logic.

@jcouv
Copy link
Member

jcouv commented Jul 29, 2017

LDM notes for Jul 15 2016 are available at https://github.com/dotnet/csharplang/blob/master/meetings/2016/LDM-2016-07-15.md
I'll close the present issue. Thanks

@jcouv jcouv closed this as completed Jul 29, 2017
@mlaukala
Copy link

mlaukala commented Aug 8, 2019

I do understand the arguments for this but I just wanted to point out that going against a fundamental concept for convenience is a very terrible idea and it opens the door to other really really bad implementations for the sake of convenience. This leads to very dirty and convoluted concepts that are hard to grasp and makes learning it that much harder. Further, it's not even a new feature at this point. It's simply a convenience tool. If I were to guess I'd say that it generates the exact same IL as declaring the variable before the function call. In any case, that's exactly how it behaves and it's unexpected.

@gafter
Copy link
Member

gafter commented Aug 8, 2019

@mlaukala I don't know what "fundamental concept" you're referring to.

In any case, this feature was introduced in C# 7.0. Since then we've release 7.1, 7.2, and 8.0 is about to be released. I don't think we can revisit long completed design decisions. I have no idea how you imagine pattern-matching would work without this.

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

No branches or pull requests