Discussion on Potential Bugs Due to Operator Precedence in Swift

Hi everyone,

I wanted to open up a discussion around a piece of Swift code that I believe highlights an easy-to-miss source of potential bugs due to operator precedence. Here's the code snippet in question:

var a: Int? = 12
let b = a ?? 0 + 757575

Given that the addition + operator has a higher precedence than the nil-coalescing ?? operator, the actual logic executed by this line is:

let b = a ?? (0 + 757575)

This behavior is correct and in line with how operator precedence works. However, the developer's intention might have been to execute the operation as:

let b = (a ?? 0) + 757575

It's a clear case where understanding and paying attention to operator precedence is vital. Nonetheless, it's equally a spot where developers could easily introduce bugs into their code without realizing it, especially for those who might not be as vigilant about operator precedence.

This brings me to the point of discussion: Should there be a warning issued in such situations to highlight the potential misuse due to operator precedence?

This is, of course, just my take on it, and I'm eager to hear if others have faced similar issues or have suggestions on how to better handle such scenarios. Do you think compiler warnings for these cases would be beneficial, or is there a better approach to mitigate these kinds of bugs?

Looking forward to your thoughts and any advice you might have.

Best, KFKK

4 Likes

I'd either use parentheses, or for longer expressions with more requirement for readability simply transform:

-->

var a: Int? = 12
let b = 0 + 757575
let c = a ?? b
1 Like

IMHO + 757575 does not look like a numeric literal due to the space between + and 757575, so I don't think there should be a warning.

Apologies, I omitted the evolution of the code in my description, which might make it appear confusing. In reality, encountering such issues in a production environment is mostly introduced during project iterations:

Old:

let defaultHeight: CGFloat = 15
var a: CGFloat? // some runtime value
let b = a ?? defaultHeight

New:

let defaultHeight: CGFloat = 15
let padding: CGFloat = 5
var a: CGFloat? // some runtime value
let b = a ?? defaultHeight + padding

In the revised code, the developers intended for the operation order to be: let b = (a ?? defaultHeight) + padding. However, due to not considering the operator precedence or a moment of oversight, It has been written as let b = a ?? defaultHeight + padding, which is equivalent to let b = a ?? (defaultHeight + padding).

Having encountered this issue multiple times, the idea of adding a warning for operator precedence emerged.

2 Likes

What would be incredible, is if the Xcode editor would let you hover over any expression, and show hints below your code to display the order in which the various parts of your expressions are evaluated. That way no one would have to memorize operator precedence rules at all. When in doubt, move your cursor over a line, wait a second of two for the compiler to figure it out, and get a visual representation of what the tree looks like.

It’s hard to make an example using monospaced text, but imagine if the two "lines" below would be thin, and sit right below the code rather than on subsequent lines:

let b = a ?? 0 + 757575
             [--------]
        [------------]
8 Likes

Precedence groups are partially ordered so you can express this by changing the precedence of ?? to be incomparable with the others (swift-evolution/proposals/0077-operator-precedence.md at main · apple/swift-evolution · GitHub). But I think at this point, such a fundamental change would need to be addressing a really serious problem to be considered.

4 Likes

I can’t speak to the Xcode side but the language server part would be a good application for the SwiftOperators library in swift-syntax: swift-syntax/Sources/SwiftOperators at main · apple/swift-syntax · GitHub

3 Likes

A linter warning or project style guide that requires parentheses in this case are the solutions to this issue.
I don't believe I've ever run into a bug caused by operator precedence. Anyone with a question just adds in the parentheses.

I would appreciate having a warning (or even an error) here as it could lead to silent bugs:

func setupHeightIfNeeded(_ height: CGFloat?) {
    guard let height else { return }
    view.heightAnchor.constraint(equalToConstant: height)
}
...
let b = a ?? defaultHeight + padding
setupHeightIfNeeded(b) // oops, no padding when a is non-nil

As Slava pointed out it would be a fundamental change to modify the current precedence order (which is effectively set in stone):

i.e. remove NilCoalescing out of this list and make Casting > Comparison. So if to introduce any warning (at all) it would have to be an adhoc pinpoint change to the compiler itself. Introducing an error is out of question unless we are talking of a language mode.

2 Likes

Isn't ?? precedence a bit too low?

If you feel so this could be used as a workaround:

infix operator .? : MultiplicationPrecedence

func .? <T> (lhs: T?, rhs: @autoclosure () -> T) -> T  {
    lhs ?? rhs()
}
var a: CGFloat? = 100
var defaultHeight: CGFloat = 1
var padding: CGFloat = 2
_ = (a ?? defaultHeight) + padding
_ = a.?defaultHeight + padding // ditto
1 Like

I think one can make a coherent argument that that ?? should have the highest precedence like a postfix operator. I think one can also make a perfectly coherent argument that it should have the same precedence as the ternary operator. This is fundamentally the problem with adding new non-mathematical operators; there really isn't much/any prior art to base precedence on.

If we were adding the operator today, I probably would argue for its precedence to be ordered above comparison, but unordered vs. everything else above comparison, but the operator system didn't allow partial-ordering when the operator was introduced. (This is one of several operator changes¹ I would make for a new language, I should write them up sometime for future reference).

Reordering the precedence of existing operators is never going to be acceptable, because it changes the meaning of expressions without any diagnostic. Relaxing ordering constraints in a major language version might be possible, since it "just" requires adding parentheses (but it would need to go through evolution, and I would expect it to be somewhat controversial, since it would require wide-reaching source changes, even if they could be fully automatic).


¹ The main one is that I would make all bitwise operators (including shift!) unordered vs. all arithmetic operators, so that we'd have two independent precedence chains: (shift > and > (or, xor) > ..., mul > add > ...). There are perfectly reasonable justifications you can pick for ordering these relative to each other, but no matter what you choose someone will be badly surprised.

10 Likes

A consequence of this, is that whatever the priority, it will always be ambiguous for some (most) readers of your code, and so any sane coding style guideline will encourage the use of parentheses for such cases.

To me, this (your last paragraph) would be an extremely welcome change to the language.

I’m sure there are great reasons for not wanting to make source-breaking changes. As for my 2c, I’d be fine with a bit more of that kind of thing to end up with a modern-feeling result. I imagine it’s too late for swift 6 now but maybe 7.0?

1 Like