Pattern matching order of evaluation

[pinkfloydx33]

The proposal document for recursive pattern matching has a section titled "Order of evaluation in pattern matching" which implies that the compiler is free to optimize and re-order the evaluation of properties. However it doesn't seem to make the guarantee for anything other than ITuple (unless I'm reading it wrong):

confirmed: the compiler is permitted to reorder calls to Deconstruct, property accesses, and invocations of methods in ITuple

The section on positional matching has a sentence at the very end that states:

The order in which subpatterns are matched at runtime is unspecified, and a failed match may not attempt to match all subpatterns.

As this is only a proposal specification, I'm wondering if anything has changed about it or if it's the official stance. The official Patterns documentation only refers to order of evaluation when discussing Logical Patterns using nested and and or:

The order in which patterns are checked is undefined. At run time, the right-hand nested patterns of or and and patterns can be checked first.

Reordering properties in a match where one of the properties throws, seems to work and follow left-to-right evaluation. That may be an implementation detail or perhaps there's nothing to optimize in my pattern. Either way, I'm hoping to get clarification so I don't make any assumptions going forward

Background

I actually hit a case when matching against a property that (unknown to me at the time) threw an exception. I was trying to pull the Version property out of an NpgSqlConnection object, which actually throws unless the connection is connected. So instead of this:

conn is { Version: { } v }

I tried changing it to:

conn is { Version: {} v, State: ConnectionState.Open }

This didn't work, however, and threw an exception if the connection wasn't open. So I changed it to the following, which does work:

conn is { State: ConnectionState.Open, Version: {} v }

This got me wondering about what guarantees exist and whether or not I could rely on the above behavior. I think the documents I linked answers the question for me, but because one is "proposal" and neither seem to make a blanket statement on the topic, I just wanted to be sure.

[333fred]

No such guarantees exist. As the specification says, the ordering is unspecified, and we are free to change it in the future. I do think we would have a very hard time changing it (because code like this will be written and will break on update) but we have done breaking changes before with big enough motivation, so if we find a really good motivation, it is possible pattern evaluation would change.

[333fred]

The problem is something like this:

conn is { State: ConnectionState.Open, Version: {} } or { Version: {}, State: ConnectionState.Closed }

These patterns are in opposite order, but we're only going to evaluate their consistent components once. State is only evaluated once, Version is only evaluated and null checked once, etc. It gets even worse when you start looking at patterns across switch arms. Lexical ordering is already not available today, and if you're depending on it it's likely an issue.

[HaloFour]

I totally agree, for that case it makes sense that State and Version are only evaluated once, but I might expect them to be evaluated in lexical order so that State is evaluated before Version and that the second part of the pattern uses the memoized values.

With the ordering of evaluation within a pattern being entirely undefined it would become impossible to write that pattern without having to resort to case guards, which is messy, especially if that pattern is part of a larger recursive pattern. It's very unintuitive given the expectations of how short circuiting works with normal Boolean expressions.

[pinkfloydx33]

As the specification says, the ordering is unspecified, and

I can accept that. Thanks, exactly what I was looking for.

However...the way I read both the proposal spec and the official patterns documentation, this fact seems to always be qualified by something else and/or hidden away in a sub-section making it seem as if it only applies sometimes (ie. the logical and/or case in the official docs). Perhaps the docs should be updated to make it more obvious that it applies all around?

[DavidArno]

To emphasise @HaloFour's point, consider this code:

Fraction f = default;

var isPositionUnsafe = f switch {
    { IsANumber: true, AsDecimal: > 0.0 } => true,
    _ => false
};

var isPositionLongWindedButSafe = f switch {
    { IsANumber: false } => false,
    { AsDecimal: > 0.0 } => true,
    _ => false
};

record struct Fraction(int Numerator, int Denominator)
{
    public bool IsANumber => Denominator != 0;
    
    public double AsDecimal 
        => Numerator / (double)Denominator is var d && double.IsNaN(d)
            ? throw new Exception("☹️")
            : d;
}

As patterns currently stand, I have to make a choice: I either write my patterns in the first, unsafe but tidier way or I play safe and write it the second way. But if I go down that safe route, I get asked why I wrote it in such an inefficient way and have to explain how theoretically option one might break in a future release. There's obviously therefore the temptation to take a risk and go for the first option and no doubt many will.

Just compromising the "No such guarantees exist" line with "for property patterns, the order of evaluation of properties within a single property pattern is guaranteed to be the order that the properties are first specified" should satisfy most, if not all, cases where the lack of guarantee is currently a problem. So for example,

var x = a switch {
    { P1: "foo", P2: "bar" } and a is { P3: 1, P4: 2 } => "foobar",
    { P2: "baz", P1: var p1 } => p1
};

In this case, it would be guaranteed that P1 is evaluated before P2 in both cases. Likewise, P3 will be evaluated before P4. But no guarantee of when P3 and P4 are evaluated in relation to P1 or P2 is offered as it shouldn't be needed.

[MrJul]

I frequently browse this repository, was aware of the absence of specified ordering for property pattern matching and still just realized by reading this discussion that I've written code with undefined behavior in several places.

The usages reported in the posts above are always in the form { Guard: SomeValue, PropertyValidOnlyWhenGuardIsSomeValue: ... } which seems pretty common. The typical Optional<T>/MayBe<T> implementation you can find in dozens of projects is probably throwing when trying to access the value when there's none, exactly like Nullable<T> does:

public readonly struct Optional<T> {
    private readonly T _value;
    public Optional(T value) => (HasValue, _value) = (true, value);
    public bool HasValue { get; }
    public T Value => HasValue ? _value : throw new InvalidOperationException();
}

(There are of course exceptions, such as Roslyn's own Optional which doesn't throw).

Sample usages (with optional being a local of type Optional<int>):
if (optional.HasValue && optional.Value == 42) ✔️ OK (no pattern matching)
if (optional is { HasValue: true } && optional is { Value: 42 }) ✔️ OK (guaranteed evaluation order)
if (optional is { HasValue: true, Value: 42 }) ❌ Wrong (unspecified evaluation order: might throw in a future compiler version)

I don't think it's unreasonable to believe that those three have the same behavior. The correct pattern matching usage is the less obvious, more verbose version. (I'm definitely guilty of having "simplified" existing code to use pattern matching the wrong way.)

But pattern matching isn't really needed here: the first old school statement is perfectly fine. It gets worse if you're already using pattern matching to verify the type of an expression and want to have some additional checks:

With a method object? GetExpensiveData():
if (GetExpensiveData() is Optional<int> { HasValue: true, Value: 42 }) ❌ Wrong
if (GetExpensiveData() is Optional<int> o && o.HasValue && o.Value == 42) ✔️ OK
if (GetExpensiveData() is Optional<int> { HasValue: true } o && o.Value == 42) ✔️ OK
if (GetExpensiveData() is Optional<int> o && o is { HasValue: true } && o is { Value: 42 }) ✔️ OK
You get the gist: the simplest and most direct form of pattern matching is always the incorrect one.

C# has very few places where there's undefined evaluation order, and that's a good thing. Left to right evaluation is simple to understand and get right.

I understand why the order is undefined when there are multiple patterns referring to the same property in different orders. Even in this probably uncommon case (I don't have any numbers to back this), I would personally prefer a compiler warning instead of having the behavior change at runtime in a future compiler version, which is much worse than a new build error in my eyes.