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

Proposal: Permit variable declarations under disjunctive patterns #4018

Open
2 of 4 tasks
alrz opened this issue Oct 16, 2020 · 19 comments
Open
2 of 4 tasks

Proposal: Permit variable declarations under disjunctive patterns #4018

alrz opened this issue Oct 16, 2020 · 19 comments

Comments

@alrz
Copy link
Contributor

alrz commented Oct 16, 2020

Permit variable declarations under disjunctive patterns

Design meetings

https://github.com/dotnet/csharplang/blob/master/meetings/2020/LDM-2020-11-11.md#variable-declarations-under-disjunctive-patterns
https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-03-28.md#variable-declarations-under-disjunctive-patterns
https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-09-28.md#ungrouped
https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-10-17.md#permit-variable-declarations-under-disjunctive-patterns
https://github.com/dotnet/csharplang/blob/main/meetings/2024/LDM-2024-09-06.md#permit-variable-declarations-under-disjunctive-patterns

@alrz
Copy link
Contributor Author

alrz commented Oct 16, 2020

cc @333fred

@CyrusNajmabadi
Copy link
Member

I will champ ion this.

@HaloFour
Copy link
Contributor

I imagine we can crib a lot of the rules/spec from F# since they offer this functionality?

@alrz
Copy link
Contributor Author

alrz commented Oct 16, 2020

@HaloFour Definitely. F# specification on the subject did help a lot already.

Both patterns must bind the same set of variables with the same types

Beyond that, we are diverging from F# specification because in F# it's an error if you don't have an identical set on each side, but in C# we could make additional variables unassigned in that case. This is useful in a switch statement with a when clause.

@333fred
Copy link
Member

333fred commented Oct 16, 2020

Lol @CyrusNajmabadi I had already told alrz I'd champion this. It's mine! No fair waking up before me.

@333fred 333fred self-assigned this Oct 16, 2020
@CyrusNajmabadi CyrusNajmabadi removed their assignment Oct 16, 2020
@CyrusNajmabadi
Copy link
Member

WFM :) I was championing in case no one else picked it up. But if you want it, go for it :)

@MadsTorgersen
Copy link
Contributor

I want us to address this scenario! I am a bit concerned about a feature that has multiple declarations of the same variable, so I would want to consider alternatives that didn't do that.

For instance, maybe we have a pattern that can match into an existing variable if the type is right. E.g. (just thinking out loud here):

e switch
{
    (int i, 0) or (0, i) => i,
}

Where specifying an existing variable matches into that variable. Perhaps we would only allow it if the variable is not already assigned, to avoid pattern matching having side effects.

I'm not saying this is better, but it has different trade-offs, and we should consider our options.

@alrz
Copy link
Contributor Author

alrz commented Nov 4, 2020

Some thoughts on that syntax:

  1. It's not clear that we're assigning to a variable or matching a constant. The former is something that we might want to consider at some point but I don't think a sole identifier is suitable. I think it should be reserved for when we want to possibly permit matching with non-constatnt readonly variables.
  2. We depend on the order in which the variable is declared first, so reordering patterns around won't be free.
  3. This doesn't seemlessly apply if we want to do this outside pattern boundaries (see the "further considerations" section)

@HaloFour
Copy link
Contributor

HaloFour commented Nov 4, 2020

Where specifying an existing variable matches into that variable.

Would that mean enabling using existing variables in pattern variables in general?

int i;
e switch {
    (i, 0) or (0, i) => i
}

@alrz
Copy link
Contributor Author

alrz commented Nov 4, 2020

What if I want to match instead of assign?

readonly int i = 0;
e switch {
    (i, 0) or (0, i) => ...
}

And that already works for constants with the exact same syntax.

I've suggested out i for assigning existing variables before but I think it would be mainly useful with user-defined positional patterns.

@HaloFour
Copy link
Contributor

HaloFour commented Nov 4, 2020

Exactly, that syntax wades into the territory of requests to allow existing variables to be used in patterns instead of just constants, as well as being able to assign patterns into existing variables.

@alrz
Copy link
Contributor Author

alrz commented Nov 4, 2020

as well as being able to assign patterns into existing variables

And to be clear, the said variables are usually defined outside of the pattern itself and likely unassigned (e.g. out parameters)

I'd even say that if it ever become a thing, it should be disallowed to reassign pattern variables, that'll make the pattern itself difficult to reason about and hard to follow where such variables are assigned and possibly reassigned.

So I favor "multiply declared variables" syntax where we explicitly re-declare variables, rather than implicitly re-assigning them.

@333fred 333fred added this to the Working Set milestone Nov 11, 2020
@alrz
Copy link
Contributor Author

alrz commented Nov 12, 2020

From LDM:

We need a rule that says when you are allowed to redeclare existing variables. It needs to cover multiple switch case labels, while also not permitting things declared outside the switch label to be redeclared.

It seems like to allow this we only need to relax the existing scoping rules around pattern variables to enable multi-declarations and handle definite-assignment accordingly. i.e. ExpressionVariableBinder would need to handle pattern variables differently.

In other words, the rules that flag those name conflicts are already there, we only need to relax those existing scoping rules.

Having said that, the support for overlapping variables across separate patterns just falls out of the fact that they share the same scope. Unless we want to disallow it explicitly.

How identical do the types need to be? Are nullability differences permitted? ie, are (object?, object) and (object, object?) the same for the purposes of this feature? It seems like they may have to be.

Pattern variables start with not-null state, so I think this is a moot point.

@333fred
Copy link
Member

333fred commented Nov 12, 2020

In other words, the rules that flag those name conflicts are already there, we only need to relax those existing scoping rules.

We still need the specese.

Pattern variables start with not-null state, so I think this is a moot point.

Consider:

if (o is (not null, null) a or (null, not null) a) {} 

Also remember the var pattern.

@Thaina
Copy link

Thaina commented Mar 5, 2021

I want to add that I wish I could reassign variable. So that I could chain it in if block

Instead of

var a = GetFrom0();
if(a == null)
    a = GetFrom1();
if(a == null)
    a = GetFrom2();
if(a == null)
    return;

// DoSomething with a

I could just

if(GetFrom0() is var a || GetFrom1() is {} a || GetFrom2() is {} a) // Should it require reassign to be {} only ?
{
    // DoSomething with a
}

@spydacarnage
Copy link

if(GetFrom0() is var a || GetFrom1() is {} a || GetFrom2() is {} a) // Should it require reassign to be {} only ?
{
    // DoSomething with a
}

Can't you use:

if ((GetFrom0() ?? GetFrom1() ?? GetFrom2()) is var a)
{
    // DoSomething with a
}

@alrz
Copy link
Contributor Author

alrz commented Mar 5, 2021

So that I could chain it in if block

That's the second part of the proposal where you could possibly redeclare pattern variables and they assign to the same local

if (M() is int x ||
    N() is int x)
{
    Use(x); 
}

@Thaina
Copy link

Thaina commented Mar 5, 2021

@alrz Oh then sorry I miss that part

@Thaina
Copy link

Thaina commented Mar 5, 2021

@spydacarnage Not only that sometime I want to do something like this

void DoSomething(SomeObject obj)
{
    if(obj != null || GetFrom0() is {} obj || GetFrom1() is {} obj || GetFrom2() is {} obj) // reassign variable
    {
        // DoSomething with obj
    }
}

And there could be more complex logic in between

void DoSomething(SomeObject obj)
{
    if(obj != null || (GetFrom0() is {} obj && obj.Val0 != null) || (GetFrom1() is {} obj && obj.Val1 != null))
    {
        // DoSomething with obj
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Language/design
Development

No branches or pull requests

8 participants