Roslyn: C# 7.0 patterns design concerns

Created on 29 Aug 2016  路  18Comments  路  Source: dotnet/roslyn

Hi Roslyn team!

After observing the C# 7.0 design proposals and trying to imagine future ReSharper features/suggestions/transformations that would help C# developers to adopt all the great language improvements, I have a few design concerns around patterns I wish to express here. I really hope this would be any helpful to anybody.

1. The conditional nature of is expression

Ordinary is expression is actually not only type-checking, but also the null-checking expression in user's intuition...

``` c#
string s = null;
s is string // false


...and 'var' is the "no-op" construct that just enables the type inference...

``` c#
string s = "abc";
var s = "abc"; // exactly the same semantic

Both of those assumptions/intuitions are broken by the x is var y construct:

``` c#
string s = null;
s is var t // true, t == null


Just like `x is *`, the `x is var t` construct has no real use. I can't even use it to "declare" the variable for expression somewhere deep inside other expressions without special method taking dummy boolean `true` from `is` expression like `Foo(Bar( Let(Baz() is var baz, h * h) ))`.

I really like the idea to extend existing `is` and `switch` constructs, but maybe C# 7.0 can save user's intuition by splitting the sets of patterns into conditional/unconditional ones and restricting the use of unconditional patterns inside `is` expressions (and `switch` as well?). This would help preserving the "conditional nature" of `is` expression and avoid it's use to simply introduce some names (we would have better ways to do this, right?).
### 2. The kind of "deconstruction" we can use the most in C# code

In short: what we really need is the "!= null" + variable pattern.

After looking at my daily code in ReSharper (which is quite similar to code you have in Roslyn: lot of null and type checking over trees of closed type hierarchies), I realized I can only rewrite a very limited set of code in terms of the pattern-matching, even with all the reach proposals like positional/property patterns now postponed to C# 7+.

For example, this is the typical "deconstruction" routine:

``` c#
var binaryExpression = expression as IBinaryExpression;
if (binaryExpression == null) return false;

if (binaryExpression.Kind != BinaryExpressionKind.Addition) return false;

var leftOperand = binaryExpression.LeftOperand;
if (leftOperand == null) return false;

var rightOperand = binaryExpression.RightOperand;
if (rightOperand == null) return false;

// do work

I can really turn it into the declarative pattern with property patterns, except the sad part - there is no way to express that leftOperand and rightOperand pattern variables are expected to be non-null, so I have to do manual null-checks after pattern matching:

``` c#
if (expression is IBinaryExpression {
Kind is BinaryExpressionKind.Addition,
LeftOperand is var leftOperand,
RightOperand is var rightOperand
}
&& leftOperand != null // the sad part
&& rightOperand != null)
{
// do work
}


Technically, I can workaround null-checks with the use of type patterns (damn, this looks nearly awesome!):

``` c#
if (expression is IBinaryExpression {
                    Kind is BinaryExpressionKind.Addition,
                    LeftOperand is IExpression leftOperand,
                    RightOperand is IExpression rightOperand
                  })
{
  // do work
}

But I'm not sure this is the desired use of type patterns, since if we change the type of LeftOperand/RightOperand, the pattern code would compile fine but with dramatic semantic change.

I believe it's really important to have "null-checking" pattern from the beginning of "patterns invasion" into C#, since null-checks are really common thing to do during "deconstruction" of the data in C#. Type pattern has implicit null check, I believe property pattern is doing it is well, but there is no simple way to get a fresh variable bound to non-null value.

I can imagine splitting variable pattern into conditional "null-checking variable" and unconditional "nullable variable" patterns, maybe expressed in a x is var notNull and x is var? canBeNull forms respectively. This would preserve our intuition of conditional is expression + preserve type-inference-enabling nature of var:

``` c#
string s = null;
s is var x // false
s is string x // false
s is var? x // true, x == null


So, "null-checking variable pattern" is really a form of type pattern with type-inferred type. And the explicit question mark for nullable variables patterns would notify user for possible null checks needed:

``` c#
if (expression is IBinaryExpression {
                    LeftOperand is var leftOperand,
                    RightOperand is var? rightOperand
                  })
{
  // work with possibly unfinished binary expression
}

I know that would slightly break intuition around var-as-a-thing-that-can-be-null, but we want less nulls in future C# and nullable reference types explicitly expressed in a language syntax, do we?

3. Constness of is expressions with unconditional patterns.

Even if is expression is allowed to have unconditional patterns, I expect is expression to became constant boolean expression with true as a value, so we get the compiler warning on unreachable code:

c# int M(int x) { if (x is var t) { return t; } else { throw null; // unreachable // no return needed } }

Area-Language Design New Language Feature - Pattern Matching Question

Most helpful comment

@HaloFour I think that was initially proposed to make case var x possible which is useful if you are switching on an expression that is not a variable itself. But I agree, is var doesn't make any sense.

if (s1 is var s2)

Looks like a useless declaration expression (#254) because it always return true.

All 18 comments

What if you want to typed stuff with the null?

null string -> string.Empty
null other -> other.Default

etc?

More like generics semantics than is semantics?

if (typeof(T) == typeof(sting)) 
{

}
else if (typeof(T) == typeof(other)) 
{

}
  1. You're right that it doesn't make a lot of sense as a top-level pattern. It exists primarily for recursive patterns where you'd want to deconstruct some child value into an identifier. Is this pattern even permitted in a top-level pattern?
  2. Variable patterns aren't supposed to enforce any additional conditions, including whether or not the value is null. This is handled by type patterns. This is true of pattern matching in any functional language, including Scala and F# that have to coexist in an ecosystem full of nulls. I think that #5032 would alleviate many of the additional concerns as the type of the variable for a nullable reference would itself be nullable leading to warnings on dereference unless flow analysis could demonstrate a proper guard.

@HaloFour

  1. This exactly is why I'm calling it "null-checking variable pattern", not "variable pattern" :) To be fair, var x pattern in C# already looks different from Scala/F#'s "name patterns". But I understand your concern, maybe "null-checking pattern" deserves it's own different syntax. Maybe x is var! t or x is let t or something...

By the way, I missed this "null-checked name pattern" in F# too... you forced to write ugly match e with null -> ... | notNull -> ....

@controlflow

I understand, and I agree that it would be useful to have a quick way to pattern-match away null. Personally I'm kind of hoping that "custom is" or "active patterns" will allow for writing helper patterns that will fill that gap and allow for writing something like the following:

var person = new Person() { FirstName = "John", LastName = "Smith" };
// later
if (person is Person { LastName is Some(var lastName) }) {
    Console.WriteLine($"LastName is not null and is \"{lastName}\".");
}

Alternately maybe a not pattern:

if (person is not null) // feels like VB?

@HaloFour

Variable patterns aren't supposed to enforce any additional conditions, including whether or not the value is null.

var never change the semantics of the code. The current implementation is actually inconsistent with rules that we have today.

I'll note that in property patterns you will be able to elide the type and obviously, check for nulls. It'd be unfortunate to lose this ability in the simplest form of the patterns.

@alrz

The inconsistency comes from blending type declaration and matching into one expression and it's kind of unavoidable. The only way to really fix that would be through the following, which I don't think anybody wants:

string s1 = null;
Debug.Assert(s1 is string s2 && object.ReferenceEquals(s1, s2));

Which puts var into the awkward position, but var can implicitly be anything so it only makes sense that it can match anything.

That said, I totally don't think that it should be legal to include var_pattern under pattern, but instead only allow it under subpattern or property_subpattern. That way you avoid the whole:

string s1 = null;
if (s1 is var s2) { ... } // what's the point of this?

@HaloFour I think that was initially proposed to make case var x possible which is useful if you are switching on an expression that is not a variable itself. But I agree, is var doesn't make any sense.

if (s1 is var s2)

Looks like a useless declaration expression (#254) because it always return true.

cc @gafter

@alrz

That's true, it does make sense with switch (and match) expressions as a catch-all that happens to capture the value of the matching expression. Still feels wrong to allow it with is expressions but I don't know that it would be worth it to separate the grammar out to disallow it in that situation. A compiler warning seems to be in order, though.

I've probably seen the current syntax for so long that I've grown accustomed to it. Having var be akin to a capturing wildcard just feels right at this point. I don't disagree that a non-nullable form would be useful but I can't think of a syntax for it that would also fit in with other syntax in C#.

if (Foo() is var foo && foo.bar) { /* Use foo */ } is reasonable, although with the new scope rule it leaks to outside of if, as few people want.

I'm always wanting a is not b when I write !(a is b).

@CnSimonChan do we really think, that turning conditional language construct used to do type testing today... into unconditional variable introduction construct tomorrow... is "reasonable"? I'm not sure :)

I am convinced that var patterns should not do any null check; if it would ever be allowed to use recursive patterns in the context of out parameters in addition to tuple deconstructions (#11293), the simplest form will be a var pattern which must not do any null check.

Note that var already infers nullabiliy of a variable e.g. var x = new int?(2) so if we treat var as an inferred non-nullable type and var? as an inferred nullable type in patterns , it wouldn't be consistent.

However, I do believe something akin to implicitly unwrapped optionals in Swift would be useful when non-nullable references are introduced in the language. So I think var! syntax would be a good candidate for this purpose.

Maybe just disallow is var in C# 7?

I must admit: this was a bad idea to suggest var x as a "not-null variable pattern" syntax. var! x or something else would be a better choice. I was thinking of this syntax only because it some kind of duality between var s = ...;/string s = ...; declarations and t is T x/t is var x expressions.

Anyway, I think two core problems must be addressed in some way:

  1. Unconditional patterns under is expression,
  2. Missing not-null variable pattern.

We already discussed this at great length, and found the current state to be the least offensive overall solution.

What about an "optional pattern" (from Swift),

case var x?:
// shorthand for 
case var x when x != null:

I think this would be very useful, specifically in scenarios mentioned by OP.

@alrz The OP's use cases are recursive patterns, which are not part of C# 7.

We'll be tracking language design issues in the csharplang repo, so I'm closing this in the Roslyn repo. If there are still issues here that interest you, you are most welcome to open issues there.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

marler8997 picture marler8997  路  3Comments

SolalPirelli picture SolalPirelli  路  3Comments

MadsTorgersen picture MadsTorgersen  路  3Comments

codingonHP picture codingonHP  路  3Comments

glennblock picture glennblock  路  3Comments