-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Comments
string s = "123";
bool b = int.TryParse(s, out int i);
Console.WriteLine(i); // in scope? |
@HaloFour Yes. |
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?
} |
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 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 |
@HaloFour These are on the table and being discussed internally. For the switch cases, we're not considering changing that. |
The revised scoping rules have been edited into the issue description above. |
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?! |
@HaloFour Correct. The |
But the I brought up the |
No, the |
Oh ok, but if I refactored to the following then 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. |
Yes, if you "refactored" in that way without adding an enclosing block, both |
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? |
@Eyas It just won't be definitely assigned there. But you could assign it yourself, for example in an else clause! |
Let me see if I got this right. It works like this:
which is the same as this:
|
No: if (int.TryParse(s, out var i)
{
// i in scope
}
else
{
// i in scope
}
// i in scope, too! |
@HaloFour Will |
Yeah. C# doesn't really know that the int i;
int.TryParse(s, out i);
// i is definitely assigned here |
@HaloFour Yes, obviously. But I must say, that it is not intuitive as it differs from the case |
…rSyntax.ArgumentList in ‘fixed’, ‘for’ and ‘using’ statements. Related to dotnet#12597. Fixes dotnet#13459, dotnet#13460.
…rSyntax.ArgumentList in field declarations. Related to dotnet#12597 and # 13417.
I believe this issue has been addressed. |
This is annoying; it'd be nice to be able to write int SomeFlag { get; } = int.TryParse(SomeStatic.String, out int x) ? x : 0; |
@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. |
@paulomorgado No; initializers are emitted as part of the (static) constructor; the compiler could generate a local there. |
Static initializers, right @SLaks? But I think it would be less confusing if a method was generated for each initializer that needs it. |
@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. |
With single scope, do you mean that this will be possible?
|
Yes |
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? |
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. |
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?
Well, if it runs before the constructor, it won't change the meaning that much. |
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.
Declaration expressions are still on the drawing board, so I cannot say exactly how they would work.
…-Neal
On Dec 25, 2016, at 4:29 PM, Paulo Morgado <[email protected]<mailto:[email protected]>> wrote:
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.
-
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Froslyn%2Fissues%2F12597%23issuecomment-269143416&data=02%7C01%7Cneal.gafter%40microsoft.com%7C32b77791d0b649db4e8108d42d26237c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636183089204408297&sdata=kp6sHrXrmR1%2B8Vm0pAE4NxZdFP70EEw3RDK6edNRtjE%3D&reserved=0>, or mute the thread<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FADoMuqOBeMuBPM3o_drlCB4uBITr63Zmks5rLwo1gaJpZM4JPQEG&data=02%7C01%7Cneal.gafter%40microsoft.com%7C32b77791d0b649db4e8108d42d26237c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636183089204408297&sdata=CkY218GouXgo05F%2BTis8mbazDj%2FaWmsXLpIn2EkXyJI%3D&reserved=0>.
|
Maybe it's a "get off my lawn" attitude, but that's what constructors are for, 😄 |
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. |
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).
if
orwhile
statementswitch
statementreturn
,throw
, oryield 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 anif
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:
The text was updated successfully, but these errors were encountered: