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

Matching nullables with var pattern #30906

Closed
alrz opened this issue Nov 1, 2018 · 7 comments
Closed

Matching nullables with var pattern #30906

alrz opened this issue Nov 1, 2018 · 7 comments
Assignees
Milestone

Comments

@alrz
Copy link
Member

alrz commented Nov 1, 2018

Version Used: features/recursive-patterns

Steps to Reproduce:

using System;
class C {
    extern static (int, int)? Get();   

    static void Test() {
        if (Get() is var (x, y)) {
            Console.Write(x);   
            Console.Write(y);   
        }
    }
}

Expected Behavior: Should check for HasValue and deconstruct.

Actual Behavior: Error.

Note: should work for any deconstructible, not just tuples.
Note: in effect, this should be equivalent to the (var x, var y) pattern.

@Joe4evr
Copy link

Joe4evr commented Nov 1, 2018

Note: in effect, this should be equivalent to the (var x, var y) pattern.

I'm not so sure, the var pattern is supposed to match everything, regardless of the value. Once we get those coveted Property patterns, this might naturally fall out of using the Empty Property pattern as a null-check:

if (Get() is {} (x, y))
{
    //code....
}

@alrz
Copy link
Member Author

alrz commented Nov 1, 2018

the var pattern is supposed to match everything,

A var-pattern with a parenthesized designation cannot possibly match everything. Now, the current behavior comes from deconstruction assignment var (x, y) = e; (you can't deconstruct a nullable tuple). But since here we're talking about patterns (rather than mere deconstruction), we can actually fail the match on null.

if (Get() is {} (x, y))

First off, this is an invalid pattern, unless x and y are constants - in which case the correct syntax is
(x, y) {}. If you want to introduce variables you should use var x instead. The identifiers alone only work in a var-pattern, just like the example above.

Also you don't need to do {} when you're matching with a deconstruction pattern,

if (Get() is (var x, var y))

This check for null (if needed) and deconstruct into variables.

I believe the same behavior is expected with the shorthand form var (x, y).

@gafter
Copy link
Member

gafter commented Nov 1, 2018

This is definitely a bug. It should simply work.

@gafter gafter assigned gafter and unassigned jcouv Nov 1, 2018
@gafter
Copy link
Member

gafter commented Nov 1, 2018

It should work for pattern-matching. It should not work with deconstruction.

@gafter gafter added the Feature - Pattern Matching Pattern Matching label Nov 1, 2018
@alrz
Copy link
Member Author

alrz commented Nov 1, 2018

I think new object() is var (x, y) should match with ITuple, I suspect it would have the same fix.

@gafter
Copy link
Member

gafter commented Nov 1, 2018

Yes, new object() is var (x, y) should yield false.

gafter added a commit to gafter/roslyn that referenced this issue Nov 2, 2018
gafter added a commit to gafter/roslyn that referenced this issue Nov 8, 2018
* Permit 0-element and 1-element tuple patterns
Fixes dotnet#30962
Replaces dotnet#31027

* Handle null and nullable input for a var pattern with a tuple designation
Fixes dotnet#30906
Replaces dotnet#30935

* Don't report missing Deconstruct when there is more than one applicable Deconstruct
Fixes dotnet#31031
gafter pushed a commit that referenced this issue Nov 29, 2018
…31056)

* Permit the use of ITuple when Deconstruct exists but is ambiguous.

* Permit 0-element and 1-element tuple patterns
Fixes #30962
Replaces #31027

* Handle null and nullable input for a var pattern with a tuple designation
Fixes #30906
Replaces #30935

* Don't report missing Deconstruct when there is more than one applicable Deconstruct
Fixes #31031
@gafter
Copy link
Member

gafter commented Nov 29, 2018

Fixed in #31056

@gafter gafter closed this as completed Nov 29, 2018
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

5 participants