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

Discussion: non-null var pattern #306

Closed
alrz opened this issue Mar 21, 2017 · 19 comments
Closed

Discussion: non-null var pattern #306

alrz opened this issue Mar 21, 2017 · 19 comments

Comments

@alrz
Copy link
Member

alrz commented Mar 21, 2017

Originally proposed in dotnet/roslyn#13432, can be used to null-guard a var pattern,

if (obj is X { P: var! p }) // will only match if `P` is not null.

Alternatively, we could do something like "optional patterns" in Swift,

if ( obj is var x? ) // optional `?` token in a SingleVariableDesignation

This allows us to do null check inside deconstruction pattern for each var individually.

if (obj is var (x?, y))

In this example we only null-guard x but y still can be null.

Note: the simplest case obj is var x? wouldn't be ambiguous if we parse ? eagerly as part of the pattern, just like obj is bool? which we parse ? as part of the nullable type rather than expecting a ternary.

@bondsbw
Copy link

bondsbw commented Mar 25, 2017

From a readability perspective, I totally missed the ! in the last example until I analyzed it very closely.

@alrz
Copy link
Member Author

alrz commented Mar 25, 2017

@bondsbw

That's kind of the point. You might prefer var! over postfix ! operator to force dereference a nullable.

@bondsbw
Copy link

bondsbw commented Mar 25, 2017

Ah, I misunderstood, I thought postfix was part of this proposal.

@gafter
Copy link
Member

gafter commented Oct 11, 2017

In the next update to pattern-matching in C#, we will be adding support for a property pattern. For example, if you have a object o, you would be able to test if it is a Point whose X value is 3, and save its Y value into a variable y by writing

if (o is Point {X is 3, Y is var y}) ...

(syntax not necessarily final; we might use : instead of is)

If the input is already of the type you're testing, you can omit it and that type is implied. For example, if you have a variable p of type Point, you would write

if (p is {X is 3, Y is var y}) ...

You can also provide an identifier if you want to refer to the matched entity later:

if (GetPoint() is {X is 3, Y is var y} p) ...

The semantics of a property pattern is that it first tests if the input is non-null, and then if it is non-null it also matches the named properties against the provided patterns. You can have any number of nested property/pattern pairs. In this example, if GetPoint() returns type Point?, then p will be of type Point (stripping nullable).

As a side-effect, you get a non-null pattern "for free", because "zero" satisfies "any number":

if (GetPoint() is {} p) // if the returned point is not null...

Given that there is pattern {} that acts exactly like var but doesn't match null (and conveniently strips Nullable<> from the type when it is present), we are unlikely to add any further way to do the same thing.

@alrz
Copy link
Member Author

alrz commented Oct 12, 2017

@gafter That would make it impossible to use {} for array patterns. {} could match an empty array, on the other hand, property patterns always need to match at least one property e.g. {name is var p}

@DavidArno
Copy link

DavidArno commented Oct 12, 2017

@alrz,

The suggestion of overloading {} as a way of pattern matching collections has always puzzled me. Did using [] get rejected as it seems a more obvious choice (aside from :: cons of course, but that's a different discussion)?

var x = someCollection match (
    case [ 1, 2, var x ]: $"it's 1, 2, {x}",
    case [ 1, .. 20 ]: "it starts with 1 and ends with 20",
    case *: "oh, it's neither of them"
);

@alrz
Copy link
Member Author

alrz commented Oct 12, 2017

The suggestion of overloading {} as a way of pattern matching collections has always puzzled me

We do have object initializers vs. collection initializers with the similar syntax and it wasn't puzzling nor ambiguous in any ways,

Did using [] get rejected as it seems a more obvious choice

Nothing has been told regarding the choice of [] for array patterns. But it would be inconsistent, because we already use {} to create arrays and I think it's good to have similar pattern vs. construction syntaxes for arrays etc (unless perhaps we're going to have dictionary/list literals with that syntax).

It's not just that, IMO {} does not communicate the fact that it has anything to do with null, imagine this in a positional pattern,

expr is Type({}, [])

The former is a null check and the latter is an empty array. wat.

This is highly opinionated but I think obj is BinaryExpression { Left: {} left, Right: {} right } is just eye scraping - where {} used solely for null-checking.

@DavidArno
Copy link

obj is BinaryExpression { Left: {} left, Right: {} right } doesn't look nice. But neither does obj is BinaryExpression { Left: let left, Right: let right }.

I think it unfortunate that the dammit operator is going to be used as a "you, Mr Compiler think that's a null, but I think you are wrong" notation. I'd prefer it were reserved as a null-check feature, eg it could be used in patterns as:

if (obj is BinaryExpression { Left: var left!, Right: var right! }) ...

as well as an earlier suggestion (that I'm not sure still applies) of having the compiler auto-generate null checks on parameters:

C1 Foo(C2 bar!) => ...
// being lowered to:
C1 Foo(C2? unsafeBar)
{
    if (!(unSafeBar is C2 bar)) throw new ArgumentNullException("bar");
    ...
}

@MkazemAkhgary
Copy link

MkazemAkhgary commented Oct 25, 2017

the whole is thing inside {} isn't right. why not have normal conditions inside? like

if (o is Point {X == 3, Y is var y}) ...

instead of

if (o is Point {X is 3, Y is var y}) ...

also could expression declarations be used too? (instead of is var x) @gafter

@gafter
Copy link
Member

gafter commented Oct 26, 2017

@MkazemAkhgary We definitely do not intend to have expression semantics, such as overload resolution to find an applicable operator == and conversions applied to its operands. We also do not intend to specify a particular order of evaluation for the nested patterns. In summary, patterns are not like expressions.

@MkazemAkhgary
Copy link

@gafter thats too much different from what we always had, see my proposal.

Im against property pattern,because its too much limited, very unnatural, c# is becoming too complex

Thats pile of notations that i hate to use. Please take a step back and see from wider perspective.

Sorry for my bad english, but dont throw me away just because i cant talk good.

Best regards

@MkazemAkhgary
Copy link

@gafter anyway, with current syntax, how would you want to say if X is between two numbers? for X being equal to 3 its like this. how would that go for ranges?

 if (o is Point {X : 3, Y is var y}) ...

@MkazemAkhgary
Copy link

MkazemAkhgary commented Oct 26, 2017

@gafter never mind, I've closed that proposal. I get your point that its about pattern matching not usual expressions, would something like this for ranges work?

also Y is var y should not be valid, I think : is better. declaration expressions could be used instead.

if(p is {X: [0, 10), Y: var y = Y})

if X is between 0 inclusive and 10 exclusive. also set Y to y

@DavidArno
Copy link

@MkazemAkhgary,

@gafter anyway, with current syntax, how would you want to say if X is between two numbers?

One possibility for that is active patterns (see #277). So you might define something like:

public static bool BetweenPattern(this int value, int min, int max) => 
    value >= min && value <= max;

then use it like:

if (o is Point {X  is Between(3, 9), Y is var y}) ...

@MkazemAkhgary
Copy link

very cool, thats something that can be used in normal expressions too. Hmmm @DavidArno

@MgSam
Copy link

MgSam commented Nov 2, 2017

if (GetPoint() is {} p) // if the returned point is not null...
Given that there is pattern {} that acts exactly like var but doesn't match null (and conveniently strips Nullable<> from the type when it is present), we are unlikely to add any further way to do the same thing.

@gafter Notice how you had to put a comment in your code there to explain what it was doing. Because such a pattern completely obfuscates your intent, which is to check for nullness without actually having the word "null" or a "?" anywhere in the expression.

I think expecting such a null check to become idiomatic is awful and certain to lead to bugs because people misunderstand what the code is supposed to be doing.

Please let's not turn this language into Perl where secret incantations happen to have non-obvious side effects that your code relies on for correctness.

@MkazemAkhgary
Copy link

MkazemAkhgary commented Nov 2, 2017

@MgSam not to mention that is var x is already something that people may abuse to inline variable declaration, while exact same thing can be achieved by c# 8 candidate #973 (in a better way of course)

allowing is {} x with such meaning will be second mistake after allowing is var x.

I second abuse of pattern-matching because that's what is called by @gafter himself. here

@alrz
Copy link
Member Author

alrz commented Dec 1, 2017

@gafter

This is the syntax I'm imagining we could use here:

single_variable_designation
  	: identifier
  	| identifier '?'
	;

Which cleanly fit to the proposed extension to the var_pattern to use a designation,

var_pattern
  	: 'var' variable_designation
	;
case var y?: 
case var (x, y?):
case (var x, var y?):

// equivalent to

case var y when y != null: 
case var (x, y)  when y != null:
case (var x, var y)  when y != null:

Then we could safely disallow the empty property pattern {} and require at least one subpattern there.

There is an open issue here though, how we should null check a discarded value? Maybe either of,

case var _?:
case var ?:
case _?:
case ?:

@gafter
Copy link
Member

gafter commented Dec 1, 2017

@alrz That seems backwards. We use ? to identify things that might be null, not things that ought not be null.

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

7 participants
@DavidArno @bondsbw @MgSam @alrz @gafter @MkazemAkhgary and others