Feedback requested: improving inneffective nil coalescing diagnostic feedback

Development Compiler
22

what? the output of step 1 is mechanically a non-optional value. it just has type T. flatMap is an extension on Optional, so that T was promoted to a single-level Optional<T>.

the flatMap contributes nothing to the meaning of the user’s code, even if they were to write more logic within the closure, except by permitting them to reference that (non-optional) value T as an anonymous closure argument.

1 Like
23

Sorry, my mistake. What I had in mind was that the overload of ?? that returned optional value was called. I think it should work too but your suggested change is indeed more straightforward and better.

While this can explain why RHS isn't reached, it can't explain why the transform closure in my test1 wasn't executed. A simpler example:

infix operator !== : ComparisonPrecedence
func !== <T: Equatable>(lhs: T, rhs: T) -> Bool {
    print("lhs: \(lhs)")
    return lhs != rhs
}

let x: Int? = nil
let y = (x ?? 1) !== .some(1) // warning: left side of nil coalescing operator '??' has non-optional type 'Int?', so the right side is never used
print("y: \(y)")

// Output:
// lhs: nil
// y: true

If both sides of ?? is optional-promoted, I think LHS should be displayed as Optional(nil), instead of just nil.

EDIT: More tests:

  • y1 case: LHS of ?? isn't evaluated; no optional promption.
  • y2 case: LHS of ?? isn't evaluated; RHS of ?? is optional-promoted.
  • y3 case: LHS of ?? is evaluted; no optional promption (this is my assumption, because otherwise LHS shouldn't be evaluated).
let x: Int? = nil
let y1 = (x ?? 1) !== .some(1)
print("y1: \(y1)")
let y2 = (x ?? 1) !== .some(nil)
print("y2: \(y2)")
let y3 = (x ?? 1) !== nil
print("y3: \(y3)")

// Output:
// lhs: nil
// y1: true
// lhs: Optional(nil)
// y2: false
// lhs: Optional(1)
// y3: true

PS: all my tests were executed on x86-64 nightly build on godbolt.org.

24

You can write let y = (Optional(x) ?? Optional(1)) !== .some(1) and it will still print nil, not Optional(nil).

The diagnostic tells us that the compiler thinks x is non-optional, and presumably print thinks that too.

25

It's not a display issue. It's because the value passed to print is really nil. You can verify it by printing the value's type too. I think this code works as expected (it doesn't produce diagnostic) and the ?? overload called in this specific test is the one that returns non-optional value:

let x: Int? = nil
let y = (Optional(x) ?? Optional(1)) !== .some(1)

IMO it works this way:

  • Compiler uses the ?? overload that returns non-optional value
  • Optional(x)'s type is Int?? and its value is .some(nil), so RHS of ?? is ignored.
  • The result of ?? operation is nil and its type is Int?.
  • As a result, both sides of !== is Int?. The code compiles.

What's difficult to explain is the following code, which emulates the original example and produces diagnostic:

let x: Int? = nil
let y = (x ?? 1) !== .some(1)

EDIT: I figured it out. I think you're right that ??'s parameters are optional-promoted. So the code let y = (x ?? 1) !== .some(1) in the second example becomes let y = (Optional(x) ?? Optional(1)) !== .some(1) in the first example, which works as explained above. This is unexpected because I think most users (I for one) would expect compiler use the ?? overload that return optional value. Another interesting thing is that while example 1 and example 2 are equivalent because of optional promotion, compiler only produces diagnostic for example 2.

EDIT 2: to answer my own questions in my earlier post:

  1. Test 1: why is flatMap closure not called?

It's because compiler chooses ?? overload that returns non-optional value. Although the original value nil (of Int? type) is promoted to .some(nil) (of Int?? type), the result of ?? operation is "downgraded" to nil (of Int? type). That's why flatMap closure isn't called.

  1. Test 2: why is RHS of ?? evaluated when specifying variable type explicitly?
let a: Int? = nil
let c: Int?? = a ?? 2 // c is .some(.some(2))

I believe it's because compiler chooses ?? overload that return optional value in this case.

  1. Test 3: why are the behaviors of using ?? and using ??? different?

It's because when using ?? compiler chooses its overload that return non-optional value, but when using ??? custom operator compiler chooses the its overload that return optional value.

Summary: It appears in all above scenarios choosing ?? overload that return optional value would greatly simplify the issue. I'll leave It to swift developers to determine if the current behavior in the original example is correct.

EDIT 3: FWIW, now that I have a better understanding of the issue, I find the first few posts actually explained it well. I'll rephrase @taylorswift's explanation and modify it a bit:

  • For some reason compiler chooses the overload that returns non-optional value: (T? ?? T) -> T
  • flapMap requires an optional type
  • From generic function's perspective T isn't an optional type, so it needs to be converted to T? somehow. The approach is to convert (T? ?? T) -> T to (T?? ?? T?) -> T? (if compiler converts the result of the function separately, that's effectively the same as calling the overload that returns optional value).
  • As part of that convertion, T? is converted to T??, so RHS value is always .some(<value of T?>) and thus LHS is ignored.

So a verbose warning might be like this:

flatMap expects an optional type, so (T? ?? T) is converted to (T?? ?? T?), which returns a value of T?. Converting a value of T? to T?? always produces a non-nil value, so RHS of ?? is ignored.

I think it would avoid the entire issue if compiler used ?? overload that returns optional value, but it perhaps isn't worth the effort for such a corner case.

26

A more thorough test here.

Results on Swift 6.3:

✅ test `== 1`
✅ test `!= 1`
✅ test `== one`
✅ test `!= one`
✅ test `== .some(1)`
❌ test `!= .some(1)`
✅ test `== .none`
❌ test `!= .none`
✅ test `== someVar`
✅ test `!= someVar`
✅ test `== noneVar`
✅ test `!= noneVar`
❌ test `.foo`

Results on previous Swift versions - all :white_check_mark:.

Will file this shortly.

1 Like
27

While I appreciate all the feedback here, and am myself intrigued[1] by the behavioral differences pointed out, I somewhat regret my choice of example since, as noted above, the use of flatMap isn't really relevant to the underlying question about what the diagnostics should say. This sort of optional injection is something that can be reproduced on earlier compilers – for instance, this example does it:

extension Optional where Wrapped == Int? {
    func foo() {}
}

let i: Int? = nil
(i ?? 42).foo()
// `- warning: left side of nil coalescing operator '??' has non-optional type 'Int?', so the right side is never used

So still wondering if anyone has further thoughts on how the diagnostic could be improved here. I think as a short-term improvement, just special-casing the optional-injection scenario to have slightly different wording would help a bit.

  1. I mentioned in the motivating issue that it looked like changes to overload resolution in 6.3 seem like a likely cause of these. ↩︎

28

Can we detect the option-injection at all? If we can detect the levels of optionality, anything above ? could have something like "left side of single nil coalescing operator '??' has multiply-optional type 'Int??', due to the promotion required by <cause>, so the right side is never used, as there will always be at least one level of optionality remaining". That's really overly verbose, but seems to contain the correct bits of info. If we had an optional-callapsing-coalescing operator, we could suggest that, but until then I don't think there's any action to suggest.

1 Like
29

How about this?

warning: left side of nil coalescing operator '??' is converted from 'Int?' to `Int??' and always non-nil, so the right side is never used

I think the diagnostic should indciate the paramter of ?? is converted. That's the key to understand why RHS is non-nil. I omit the reason for the conversion for brevity, because that's is easier to figure out. However, if users are aware that ?? have two overloads and aren't sure which one is being used, they might still find it difficult to understand. In this sense the diagnostic isn't complete, but it isn't as self-contradictory as the current one.

2 Likes
30

Tangentially related, in Xcode I find it frustrating that option-clicking on an operator does nothing.

Option-clicking a function name pops up its documentation, which lets me easily see which overload of the function is being called. Sometimes I want to do the same on an operator to see which overload of the operator is being used.

But option-clicking on the operator does nothing.

4 Likes
31

That's not necessarily true either:

let x = Int??.none
let y: Int? = x ?? nil
32

Yes, in that case you could drop the "due to the promotion..." bit. We may want a different diagnostic depending on whether the multiple optional was implicit or not.

33

My point was that double optional doesn't mean the RHS is unused.

34

True, but in the context here we'd presumably only be mentioning this in a diagnostic that occurs when the RHS is known to be unused (due to implicit optional injection of the LHS).

Thanks for all the input here. I've suggested to the author of the aforementioned PR that their approach seems like a good near-term improvement. It would re-word the diagnostic in the multiply-optional case to add some additional context and no longer be self-contradictory:

left side of nil coalescing operator '??' adds an additional level of optional to 'Int?', making it always non-nil, so the right side is never used

1 Like