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

Scope changes for patterns and out var #12597

Closed
gafter opened this issue Jul 18, 2016 · 55 comments
Closed

Scope changes for patterns and out var #12597

gafter opened this issue Jul 18, 2016 · 55 comments
Assignees
Labels
Area-Compilers Feature Request New Language Feature - Out Variable Declaration Out Variable Declaration New Language Feature - Pattern Matching Pattern Matching Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented

Comments

@gafter
Copy link
Member

gafter commented Jul 18, 2016

The LDM is reconsidering some scope issues for pattern variables and out variables. The most likely change is that such variables declared in a declaration statement or expression statement that appear in a block are in scope throughout that block.

However, the proposed rule changes need to be settled before they are implemented.

[Addendum 2016-08-11]
The LDM has settled on a revision of the scoping rules. The principal changes from the current status are that (pattern and out) variables that are declared in the following places are in the enclosing scope (e.g. the scope that they would be in if the statement were a declaration statement that declares that variable).

  • in the initializers of a local variable declaration statement
  • in the expression of an expression statement
  • in the condition of an if or while statement
  • the expression of a switch statement
  • in the expression of a return, throw, or yield return statement.

A controlled statement of a compound statement never leaks variables to an enclosing scope. Logically, you can think of a controlled statement as being surrounded by a block for the purposes of scoping.

Because of these changes, there is no longer a special treatment for the else clause of an if statement.

Here are the relevant design notes #12939.

[Addendum 2016-08-24]
The LDM has revised the scoping rules slightly today. Here are the changes:

  • out variables and pattern variables are forbidden to be declared inside field initializers and auto-property initializers (i.e. it is an error to declare an out variable or a pattern variable). The reason for this is that we are considering having a scope that spans the declaration and includes all the initializers (separately, a scope that spans the static initializers). We expect we may do that around the time we introduce records, but unless we forbid them today it would be an incompatible change later.
  • out variables and pattern variables declared inside a constructor-initializer are in scope throughout the body of the constructor. For C# 7, we may forbid them in that location unless time permits us to implement the desired scoping rules.
@HaloFour
Copy link

@gafter

string s = "123";
bool b = int.TryParse(s, out int i);
Console.WriteLine(i); // in scope?

@gafter
Copy link
Member Author

gafter commented Jul 19, 2016

@HaloFour Yes.

@HaloFour
Copy link

@gafter

Curious, particularly after all of the conversations regarding this scoping dating back to CodePlex. It simplifies things but also complicates other things.

What about:

string s = "123";
if (int.TryParse(s, out int i)) {
    ...
}
// i in scope here?

or

object o = ...;
switch (o) {
    case string x: break;
    case int x: break; // can reuse x here?
}

@AdamSpeight2008
Copy link
Contributor

AdamSpeight2008 commented Jul 19, 2016

I like the idea of by default is local to enclosing scope, unless the user specifies otherwise.

if( Int.TryParse( text, local out var value ) ) \\ local to enclosed scope
if( !Int.TryParse( text, guard out var value ) ) \\ not lot local to enclosed scope.

In pattern matching switches in the clauses it is local to enclosed scope, which would enable the out variable to be reused for different types.

switch x
{
  case int x:
    // x is in scope
    if( Int.TryParse( text, out var x )
    {
      // reuses x
    }
  default:
    // x not in scope
}
// x not scope

switch x
{  case int x:
    // x is in scope
    if( Int.TryParse( text, out var y )
    {
      // y is in scope
    }
    // y still in scope
  default:
    // x not in scope, y not in scope
}
// x not in scope, y not in scope

switch x
{  case int x:
    // x is in scope
    if( Int.TryParse( text, local out var y )
    {
      // y is in scope
    }
    // y not in scope
  default:
    // x not in scope
}
// x not in scope, y not in scope

switch x
{  case int x:
    // x is in scope
    if( Int.TryParse( text, guard out var y )
    {
      // y is not in scope
    }
    // y is in scope
  default:
    // x not in scope
}
//  x not in scope, y not in scope

@gafter
Copy link
Member Author

gafter commented Jul 19, 2016

@HaloFour These are on the table and being discussed internally. For the switch cases, we're not considering changing that.

@gafter gafter added Need More Info The issue needs more information to proceed. Blocked labels Jul 21, 2016
@gafter
Copy link
Member Author

gafter commented Jul 22, 2016

The revised scoping rules have been edited into the issue description above.

@gafter gafter removed Blocked Need More Info The issue needs more information to proceed. labels Jul 22, 2016
@HaloFour
Copy link

@gafter

So, to clarify:

Fine:

switch (obj) {
    case int x: Console.WriteLine($"obj is an int of value {x}"); break;
    case string x: Console.WriteLine($"obj is a string of value \"{x}\""); break;
}

Compiler Error:

if (obj is int x) {
    Console.WriteLine($"obj is an int of value {x}");
}
else if (obj is string x) { // ERROR CS0136
    Console.WriteLine($"obj is a string of value \"{x}\"");
}

Weirdness:

for (string line = reader.ReadLine(); line != null && int.TryParse(line, out int value); line = reader.ReadLine()) {
    // line and value in scope here
}
// value in scope here?!

@gafter
Copy link
Member Author

gafter commented Jul 22, 2016

@HaloFour Correct. The for loop does not leak anything to the enclosing scope.

@HaloFour
Copy link

@gafter

Correct. The for loop does not leak anything to the enclosing scope.

But the out declaration in the same construct does?

I brought up the for loop case specifically because that and foreach are the only statements that currently allow variable declaration within, and in both cases the scope of those variables is contrary to this decision. My opinion is that having different rules for out declarations will be very confusing.

@gafter
Copy link
Member Author

gafter commented Jul 23, 2016

But the out declaration in the same construct does?

No, the out declaration in the for loop does not leak anything to the enclosing scope. Neither the for loop nor the foreach loop leak their declared variables today, and that will remain true under this revision of the rules.

@HaloFour
Copy link

@gafter

Oh ok, but if I refactored to the following then value would "leak" to the enclosing scope?

string line = reader.ReadLine();
while (line != null && int.TryParse(line, out value)) {
    // do something
}
// is value in scope here?

Sorry for pestering. I did see Mads comment about design notes and how he intends to write up the details on this particular change in the coming weeks and I'm sure that would clarify all of the cases and ramifications.

@gafter
Copy link
Member Author

gafter commented Jul 23, 2016

Yes, if you "refactored" in that way without adding an enclosing block, both line and value would end up in the enclosing scope.

@Eyas
Copy link
Contributor

Eyas commented Jul 29, 2016

@gafter

What happens when someone references a result of a positive pattern match outside of where it is expected to be initialized? Will the compiler complain that this value might not be initialized? Or will it just compile?

e.g.

if (x is string s) {
  // do something
}
Console.WriteLine(s.Length); // s in scope according to new rule. will this compile?

@gafter
Copy link
Member Author

gafter commented Jul 29, 2016

@Eyas It just won't be definitely assigned there. But you could assign it yourself, for example in an else clause!

@paulomorgado
Copy link

Let me see if I got this right.

It works like this:

if(int.TryPars(s, out var i)
{
    // i in scope
}
else
{
    // i in scope
}

// i not in scope

which is the same as this:

{
    int i;
    if(int.TryPars(s, out i)
    {
        // i in scope
    }
    else
    {
        // i in scope
    }
}

// i not in scope

@HaloFour
Copy link

HaloFour commented Jul 29, 2016

@paulomorgado

No:

if (int.TryParse(s, out var i)
{
    // i in scope
}
else
{
    // i in scope
}

// i in scope, too!

@stepanbenes
Copy link

@HaloFour Will i be definitely assigned? I suppose that it will be...

@HaloFour
Copy link

@stepanbenes

Yeah. C# doesn't really know that the out variable and the return value of the function are related. This isn't any different to the situation today:

int i;
int.TryParse(s, out i);
// i is definitely assigned here

@stepanbenes
Copy link

@HaloFour Yes, obviously. But I must say, that it is not intuitive as it differs from the case if (x is string s)

@AlekseyTs
Copy link
Contributor

I believe this issue has been addressed.

@AlekseyTs AlekseyTs assigned gafter and unassigned AlekseyTs Sep 7, 2016
@AlekseyTs AlekseyTs added the Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented label Sep 7, 2016
@SLaks
Copy link
Contributor

SLaks commented Dec 25, 2016

  • out variables and pattern variables are forbidden to be declared inside field initializers and auto-property initializers (i.e. it is an error to declare an out variable or a pattern variable). The reason for this is that we are considering having a scope that spans the declaration and includes all the initializers (separately, a scope that spans the static initializers). We expect we may do that around the time we introduce records, but unless we forbid them today it would be an incompatible change later.

This is annoying; it'd be nice to be able to write

int SomeFlag { get; } = int.TryParse(SomeStatic.String, out int x) ? x : 0;

@gafter
Copy link
Member Author

gafter commented Dec 25, 2016

@SLaks I agree. You'll have to write a helper function.

@paulomorgado
Copy link

@SLaks I agree. You'll have to write a helper function.

It would always have to be inside a method, right? The only thing the compiler could do is generate one. Which wouldn't be such a bad idea.

@SLaks
Copy link
Contributor

SLaks commented Dec 25, 2016

@paulomorgado No; initializers are emitted as part of the (static) constructor; the compiler could generate a local there.

@paulomorgado
Copy link

Static initializers, right @SLaks?

But I think it would be less confusing if a method was generated for each initializer that needs it.

@gafter
Copy link
Member Author

gafter commented Dec 25, 2016

@paulomorgado The reason that the compiler doesn't do this for you is that it would give the wrong scope to the expression variables. We are considering having a single scope for all the expression variables of the static initializers of a class (and another scope for the expression variables of the instance initializers). But until we settle the desired scoping, and implement it, we are blocking off the area so that implementing it later will not be a breaking change.

@paulomorgado
Copy link

paulomorgado commented Dec 25, 2016

@gafter,

With single scope, do you mean that this will be possible?

int SomeFlag { get; } = int.TryParse(SomeStatic.String, out int x) ? x : 0;
int SomeOtherFlag { get; } = x;

@gafter
Copy link
Member Author

gafter commented Dec 25, 2016

Yes

@paulomorgado
Copy link

Why have you considered that? At first, I would never consider any variable leaking out of that initialization. If I wanted that, I would write a proper constructor.

Would that initialization still run before the constructor?

@gafter
Copy link
Member Author

gafter commented Dec 26, 2016

We have considered that because in the cases where it is useful, it is very awkward to work around the feature being absent.

Yes, the initialization would run before the constructor.

@paulomorgado
Copy link

I still can't think of a use case where code that looks like today's code (apart from that temporary variable usage) would benefit from that.

Is something like this expected to work with declaration expressions?

int x = (var I = 1; i);
int y = -i;

Well, if it runs before the constructor, it won't change the meaning that much.

@gafter
Copy link
Member Author

gafter commented Dec 26, 2016 via email

@paulomorgado
Copy link

There are places where we need to compute the value of two properties at the same time and we want to expose them separately. We end up having a single underlying field that holds the pair of values. It would be a bit cleaner if we could store them separately.

Maybe it's a "get off my lawn" attitude, but that's what constructors are for, 😄

@gafter
Copy link
Member Author

gafter commented Dec 26, 2016

Field initializers are where you compute the value of fields that occur before the constructor runs. That is much more compelling with records, where the constructor parameters are in scope in the field initializers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Feature Request New Language Feature - Out Variable Declaration Out Variable Declaration New Language Feature - Pattern Matching Pattern Matching Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Projects
None yet
Development

No branches or pull requests