Update pattern-variables.md

[alrz]

Prototype: dotnet/roslyn#59702
Rendered: pattern-variables.md

[alrz]

@jcouv Does this address your comment #4592 (comment)?

[alrz]

Could we relax the redeclaration requirement in a switch section?

Note: I think that could be done if we enforce the redeclaration in terms of definite-assignment instead of relying on a declarator. This way the switch example works as presented. That would also result in no error for something like var x or 1, but x will be left unassigned. I'll add this in a separate PR later for review.

[333fred]

I think that could be done if we enforce the redeclaration in terms of definite-assignment instead of relying on a declarator.

I had a similar thought earlier, I think it's a good change that would be well received by LDM.

[alrz]

I went ahead and made the change in this PR. @333fred @jcouv please take a look, thanks.

[333fred]

This all seems much more complicated than I was envisioning for rules, particularly the in-pattern ones. What I was envisioning was simply: if a variable is not currently definitely assigned, it may be redeclared in a pattern. This would permit a is T t || b is T t because t is not definitely assigned in the false case, but would not permit _ = a is T t; _ = b is T t;

[alrz]

a is T t || b is T t

If I recall correctly, LDM didn't support redeclaration across expression boundaries. I can readd the open question on that.

This all seems much more complicated than I was envisioning for rules

I only reverted the initial PR and made some changes (this currently mirrors the prototype).
I'm not sure how this could be specced in simpler terms while maintaining the same behavior. Suggestions?

[333fred]

If I recall correctly, LDM didn't support redeclaration across expression boundaries. I can readd the open question on that.

Right, we were leary of the complex rules involved and concerned about double assignments. I think that my proposed simplification, where it's just "you can assign if it's not definitely assigned", is much simpler and alleviates the double assignment concerns.

I'm not sure how this could be specced in simpler terms while maintaining the same behavior. Suggestions?

My main thought is to simply remove all the things about where variables are allowed to be redeclared in patterns, and simply state that variables are allowed to be redeclared by patterns if they are not definitely assigned and the types all line up. We might have to define what definite assignment means inside a pattern, but I think that's much more straightforward than the existing wording of the rules around where in patterns variables are allowed to be redeclared.

[alrz]

Just to make sure we're on the same page, "you can assign if it's not definitely assigned" means pattern variables can be effectively redeclared anywhere within their scope; permitting if (a is C c) ; else if (b is C c) ; for example.

For binary patterns, we need to somehow flow the def assignment state from LHS so that the variables are considered assigned in the RHS (similar to expressions) to catch both not (0, var x) or not (var x, 0) or (0, var x) and (var x, 0) as errors.

That doesn't prevent _ = a is T t; _ = b is T t; though, t could be unassigned when we enter the second statement.
Same for case not var t: case var _ when e is var t: because we've relaxed the redeclaration rule in the entire scope.

[333fred]

Just to make sure we're on the same page, "you can assign if it's not definitely assigned" means pattern variables can be effectively redeclared anywhere within their scope; permitting if (a is C c) ; else if (b is C c) ; for example.

Yes.

For binary patterns, we need to somehow flow the def assignment state from LHS so that the variables are considered assigned in the RHS (similar to expressions) to catch both not (0, var x) or not (var x, 0) or (0, var x) and (var x, 0) as errors.

Yes

That doesn't prevent _ = a is T t; _ = b is T t; though, t could be unassigned when we enter the second statement. Same for case not var t: case var _ when e is var t: because we've relaxed the redeclaration rule in the entire scope.

Indeed. I don't especially care about that scenario: you can't use t anyway.

[333fred]

I like this version a lot more :). Just a couple small comments.

[333fred]

@jcouv please take a look

[jcouv]

Good to merge.
@alrz We were planning to review this morning, but ended up not having time. Likely will get discussed next week.