1

I just got bit by a surprising result from a switch statement that wasn't matching the case block I expected. The minimum reproducer is as follows:

func check(_ arg1: Int?, _ arg2: Int?) -> Bool {
    switch (arg1, arg2) {
    case (.some(let val1), .some(let val2)):
        return val1 > val2
    case nil, nil:
        return true
    default:
        return false
    }
}

print(check(nil, nil))

Can you tell right off the bat what this will print? Perhaps it's more obvious when isolated, but looking at this in context it was very surprising to me that the switch statement would execute the default block, printing false.

Does this seem worth offering a diagnostic for? We already have a "case is already handled by previous patterns; consider removing it" warning—is there a reason that it isn't semantically valid here? I could also envision some sort of "treating this as separate patterns" when the list of patterns could match match as a tuple or match as a list of separate cases. In the second situation, I would expect fix-its which take the form:

case nil:
    fallthrough
case nil:
   ...

if the user wants to silence the warning, or

case (nil, nil):
    ...

if they want to match as a tuple.

6 Likes
2

Just in case anyone else is very confused looking at this example (like I was), what makes it behave "as expected" is

func check(_ arg1: Int?, _ arg2: Int?) -> Bool {
    switch (arg1, arg2) {
    case (.some(let val1), .some(let val2)):
        return val1 > val2
    case (nil, nil): // added parens
        return true
    default:
        return false
    }
}

print(check(nil, nil))

What is happening with the case nil, nil is that it is being treated as if:

case (arg1, arg2) ~= nil, (arg1, arg2) ~= nil:

Adding a warning for this is a bit uncanny-valley territory since essentially we have an expression pattern and we don't have a general way of comparing expressions apart from doing a syntactic check.

That said, I think given the sharp edge here, having the tuple fix-it would be a good improvement.

I don't think the case nil: fallthough fix-it is correct through (maybe you had some more complex example in mind where it would be applicable), I would certainly be very surprised if I came across code like:

case nil:
    fallthrough
case nil:
    ...
5 Likes
3

Heh, thanks for elaborating on what's going on in the example—it took me a while to figure it out as well so I should have explained better. :sweat_smile:

Yeah, this is what I'm worried about. Off the top of my head, the rule would look something like:

  • Look at each case statement.
  • If the case statement has n patterns, for some n greater than 1, and the value we're switching over has tuple type (T1, ..., Tn), offer the fix it.

I'm sure there's nuances here I haven't thought of, though...

EDIT: for example, immediately after posting I realized that in this case, the value we're switching over has type Optional<(T1, ..., Tn)>.

Right, I think in the particular example I have here, it's exceedingly unlikely that the author intended to have nil, nil specify two different patterns, but I'm also wary of introducing a warning with no way to silence it. A slightly more reasonable switch statement might look like:

    switch (arg1, arg2) {
    case (.some(let val1), .some(let val2)):
        return val1 > val2
    case nil, _:
        return true
    }

for which offering the warning would be erroneous, IMO, without some way to silence it.

4

Honestly, my first impression was that round brackets were missing. What really got me surprised was instead not having an error on the second case since (Int?, Int?) can never be nil. I didn't know you can use == nil with non optional values. TIL :slight_smile:

That being said, for this particular instance, since nil is a literal, I would appreciate seeing the warning

Literal value is already handled by previous pattern; consider removing it

instead of having the compiler to suggest a bracket addition.

Schermata 2021-02-11 alle 19.22.59
Schermata 2021-02-11 alle 19.22.591046×166 27.2 KB

4 Likes
5

Yeah, huh, looks like we just don't do that check at all for nil literals?

let x: Int = 0
switch x {
case 0, 0: // Warning: literal value is already handled...
    print("0")
case nil, nil: // OK
    print("nil")
default:
    print("some")
}
6

I think this makes a good case for adding the nil check in the same place for where the literal value pattern warning is present. :+1:

5 Likes
7

Filed as SR-14188! Though I don't think it's quite sufficient for all cases—the existing diagnostic wouldn't, for instance, cover the case nil, _ situation from above.

1 Like
8

Filed SR-14189 to track the possibility of adding an additional diagnostic.

9

I can't see any valid reason to allow a case nil: when the switch expression is non-optional. It's a type mismatch, pure and simple.

The behavior of == with mixed optional/non-optional types ought to have nothing to do with switch.

3 Likes
10

Yeah, it definitely seems like we should offer a similar diagnostic to what we do for ==:

var x: (Int?, Int?) = (nil, nil)

if x == nil { // Warning: comparing non-optional value of type '(Int?, Int?)' to 'nil' always returns false
    print("???")
}

There are even more contrived cases (without any optional promotion) that might still deserve a special diagnostic, but perhaps these sorts of edge cases are too rare...

func check(_ arg1: (Int?, Int?)?, _ arg2: Int?) -> Bool {
    switch (arg1, arg2) {
    case (nil, nil), _:
        return true
    }
}
1 Like