Problematic `== true/false` boolean comparisons

Danny · January 21, 2025, 8:30pm

It is logged as an issue, but it's not a bug.
It apparently trips people up sometimes.

The compiler isn't as good at seeing exhaustiveness as programers are.

enum Enum {
  case theOnlyCase
  init() { self = .theOnlyCase }
}

switch Enum.theOnlyCase { // Error: Switch must be exhaustive
case .init(): break
}

Sort of. My noisy brain can't switch context away from other switch cases nearly as easily as it can focus on an if condition. I think this is the general case with brains, and strongly related to why this thread exists.

If we didn't have better, I might use a constant, because that extra ({} ()) above is also a lot of noise.

let optionalBoolIsNotTrue = switch optionalBool {
case nil, false?: true
case true?: false
}

if optionalBoolIsNotTrue { // …

But I think

if optionalBool != true { // …

is fine, and if I'm wrong to think that, I wouldn't have a problem having to be explicit. I'm not even convinced Optional promotion is ever a good thing.

if optionalBool != .some(true) { // …

phoneyDev · January 22, 2025, 12:26am

I've used isNilOrEmpty quite a bit.

extension Optional where Wrapped == String {
    public var isNilOrEmpty: Bool {
        return self?.isEmpty ?? true
    }
}

I think they use this a lot in Java. It could conceivably be in the Swift Standard Library.

Danny · January 22, 2025, 1:55am

I feel like this is better expressed as?

public extension Optional {
  func isNil(or isDefault: (Wrapped) -> Bool) -> Bool {
    map(isDefault) ?? true
  }
}

public extension Optional where Wrapped: Equatable {
  func isNil(or default: Wrapped) -> Bool {
    isNil(or: { $0 == `default` })
  }
}

Optional("strang").isNil(or: \.isEmpty)
_?(true).isNil(or: false)

sspringer · January 22, 2025, 8:54am

I warning should not be emitted for code that many might see as a valid option, be it for terseness. Also do not introduce constructs that are actually harder to grasp. The problem is (in the boolean case) that you have to fit the four cases == true, != true, == false, and != false (which all make sense in certain situations) into one easy syntax, I guess this is hard to achieve.

There might be a good syntax for a subset of those cases, e.g.

if optionalValue?.isEmpty? { … } meaning if let optionalValue?.isEmpty == true

So if !optionalValue?.isEmpty? { … } would mean if optionalValue?.isEmpty != true { … }.

Instead of if optionalValue.isNilOrEmpty { … }:

if !optionalValue?.!isEmpty? { … }

Hmm. Let‘s see if people have fun with that

alexhunsley · January 23, 2025, 4:51pm

I actually coded something like this recently - isEmptyNil I called it (for code completion purposes) -- and added a marker protocol Emptyness and then conformed all the things with .isEmpty to that protocol (Data, String, Collections, etc).

protocol Emptyness {
    var isEmpty: Bool { get }
}

extension Optional where Wrapped: Emptyness {
    var isEmptyNil: Bool {
        switch self {
        case .none:
            true
        case let .some(wrapped):
            wrapped.isEmpty
        }
    }
    var hasData: Bool { !isEmptyNil }
}

/// Swift API conformances to Emptyness
extension String: Emptyness {}
extension Array: Emptyness {}
extension ArraySlice: Emptyness {}
extension Set: Emptyness {}
extension Dictionary: Emptyness {}
extension Data: Emptyness {}
extension Range: Emptyness {}
extension ClosedRange: Emptyness {}
extension Substring: Emptyness {}
extension UnsafeBufferPointer: Emptyness {}

protocol Collection: Emptyness {}
protocol Sequence: Emptyness {}

(And it contains .hasData for reverse of .isEmpty. Not strictly necessary of course, but can be useful to tidy up some expressions.)

alexhunsley · January 23, 2025, 5:04pm

Danny:

It would not make sense* to require .some for Bool but not all other Equatable types.
let bool: _? = Bool.random()
#expect(bool == .some(true))

let int: _? = Int.random(in: 0...2)
#expect(int == 1)
Edit: Maybe that's not true. Seems worthy of discussion; it's not immediately obvious to me exactly what makes Bool a problem, but it does seem somewhat special in this regard. I think it's something like, "Optional promotion can make people think Bool?'s == only needs to consider two values, when the real number is three".

Yeah, I think the fundamental issue obviously isn't just Bool, but any type that has finite possible states (enums, most typically). The fewer states, the more likely a reader is to fall into the "it's not A or B, therefore it must be C" kind of thinking.

But Bool is kind of special, given its fundamental importance in the language around checking booleans expressions or values.

alexhunsley · January 23, 2025, 5:05pm

Personally I wish they'd not put the Optional<Thing> comparable to Thing in Swift (like Java and Rust, etc), and instead perhaps added a convenience to explicitly use in those cases where that behaviour was desirable (or you just code around it).

I don't see the convenience/footgun trade-off as useful here.

alexhunsley · January 23, 2025, 5:24pm

Writing == false is fair enough, if you disregard the increased compile time, and if you're not doing it on an optional (compare btw to ! which can't be applied to an optional). If it's used on an optional, I'd argue that's not helping the code readability, due to cognitive load of "what's going on with the optional there?" And see also the 'hidden' optional stuff, where you call e.g. [].last != true on a Bool array -- easy to forget last returns optional and the logic might not be what you intended.

vns · January 23, 2025, 5:52pm

Can you clarify how this affects compile time? I haven’t heard that such thing might make an impact. I can imagine that in optimized builds it might add some time, but these are optimized builds anyway.

Optional has nothing to do with this, readability does; for any negation — you don’t need to looks with eyes for the small leading exclamation. Anyway, I would be happy to eliminate general code style preferences from the equation here.

For optionals though I would consider almost any boolean comparison as harmful. I believe comparing a Bool? type should emit a compiler warning.

AlexisQapa · January 23, 2025, 7:26pm

It took me a while to pinpoint why I don’t think this is an issue and it’s simply because I don’t use ˋ== true/false’ unless I’m in the optional situation. I see this as being overly explicit so I must be only interest in a specific value/situation instead of a simple Boolean check.

Optional promotion is a useful and general feature of swift, I’m definitively against adding a warning there as this would be adding a special case.

As Stephen said a couple messages above there are too many combinations, with I think too little use for that to go into standard lib.

Fortunately its easy to lint for or make an extension in you project if its common in you project.

bbrk24 · January 23, 2025, 7:43pm

The thing is, even if there's no overload of ==, the fact that you can implicitly convert T to T? will make it behave like there is. The only real solution is to also remove the behavior that you can say var x: String? = "" and instead require var x: String? = .some(""), which is a much bigger change.

ibex · January 24, 2025, 12:17am

@alexhunsley can probably explain better, but I checked these functions with godbold:

func f () -> Bool {
    let u: Bool? = h ()
    if u == false {
        return false
    }
    else {
        return true
    }
}

func g () -> Bool {
   let u: Bool = h ()
   if u == false {
        return false
   }
   else {
        return true
   }
}

func h () -> Bool {
    return true
}

Assembly Code

output.f() -> Swift.Bool:
        push    rbp
        mov     rbp, rsp
        sub     rsp, 48
        lea     rdi, [rbp - 8]
        xor     esi, esi
        mov     edx, 1
        call    memset@PLT
        call    (output.h() -> Swift.Bool)
        and     al, 1
        mov     byte ptr [rbp - 8], al
        lea     rcx, [rbp - 10]
        mov     qword ptr [rbp - 32], rcx
        lea     rcx, [rbp - 10]
        add     rcx, 1
        mov     qword ptr [rbp - 24], rcx
        mov     byte ptr [rbp - 10], al
        mov     byte ptr [rbp - 9], 0
        cmp     byte ptr [rbp - 10], 2
        je      .LBB1_2
        mov     rdi, qword ptr [rbp - 32]
        lea     rsi, [rbp - 11]
        call    (outlined init with copy of Swift.Bool?)
        mov     rax, qword ptr [rbp - 24]
        cmp     byte ptr [rax], 2
        je      .LBB1_4
        jmp     .LBB1_3
.LBB1_2:
        mov     rax, qword ptr [rbp - 24]
        cmp     byte ptr [rax], 2
        je      .LBB1_7
        jmp     .LBB1_8
.LBB1_3:
        mov     rcx, qword ptr [rbp - 24]
        mov     al, byte ptr [rbp - 11]
        mov     cl, byte ptr [rcx]
        xor     al, cl
        xor     al, 1
        mov     byte ptr [rbp - 33], al
        jmp     .LBB1_5
.LBB1_4:
        jmp     .LBB1_6
.LBB1_5:
        mov     al, byte ptr [rbp - 33]
        test    al, 1
        jne     .LBB1_9
        jmp     .LBB1_10
.LBB1_6:
        xor     eax, eax
        mov     byte ptr [rbp - 33], al
        jmp     .LBB1_5
.LBB1_7:
        mov     al, 1
        mov     byte ptr [rbp - 33], al
        jmp     .LBB1_5
.LBB1_8:
        jmp     .LBB1_6
.LBB1_9:
        xor     eax, eax
        mov     byte ptr [rbp - 34], al
        jmp     .LBB1_11
.LBB1_10:
        mov     al, 1
        mov     byte ptr [rbp - 34], al
        jmp     .LBB1_11
.LBB1_11:
        mov     al, byte ptr [rbp - 34]
        add     rsp, 48
        pop     rbp
        ret

output.h() -> Swift.Bool:
        push    rbp
        mov     rbp, rsp
        mov     al, 1
        pop     rbp
        ret

outlined init with copy of Swift.Bool?:
        push    rbp
        mov     rbp, rsp
        mov     rax, rsi
        mov     cl, byte ptr [rdi]
        mov     byte ptr [rax], cl
        pop     rbp
        ret

output.g() -> Swift.Bool:
        push    rbp
        mov     rbp, rsp
        sub     rsp, 16
        mov     byte ptr [rbp - 8], 0
        call    (output.h() -> Swift.Bool)
        mov     cl, al
        mov     al, cl
        and     al, 1
        mov     byte ptr [rbp - 8], al
        test    al, al
        jne     .LBB4_2
        jmp     .LBB4_1
.LBB4_1:
        xor     eax, eax
        mov     byte ptr [rbp - 9], al
        jmp     .LBB4_3
.LBB4_2:
        mov     al, 1
        mov     byte ptr [rbp - 9], al
        jmp     .LBB4_3
.LBB4_3:
        mov     al, byte ptr [rbp - 9]
        add     rsp, 16
        pop     rbp
        ret

__swift_reflection_version:
        .short  3

Assembly code for g looks shorter.

AlexisQapa · January 24, 2025, 12:38am

They were talking about compile time not run time.

I don’t follow your conclusion, the issue arise when comparing Bool to Bool? Your g implementation simply skip the issue.

If we were to deprecate the comparaison I guess we would often write ‘if let b, !b’ instead so maybe it makes more sens to compare with this ? I’d expect any of this to be optimised away. Only calling non inlined helper would have any impact.

ibex · January 24, 2025, 12:48am

But, the assembly code is generated towards the end of the compilation. no?

vns · January 24, 2025, 8:06am

@alexhunsley was saying that explicit == false comparison itself increases, so even though assembly not representative of a compile time, that's was interesting to take a look:

func g () -> Bool {
   let u: Bool = h ()
   if u == false {
        return false
   }
   else {
        return true
   }
}

func f () -> Bool {
    let u: Bool = h ()
    if !u {
        return false
    }
    else {
        return true
    }
}

func h () -> Bool {
    return true
}

Assembly

main:
        push    rbp
        mov     rbp, rsp
        xor     eax, eax
        pop     rbp
        ret

output.g() -> Swift.Bool:
        push    rbp
        mov     rbp, rsp
        sub     rsp, 16
        mov     byte ptr [rbp - 8], 0
        call    (output.h() -> Swift.Bool)
        mov     cl, al
        mov     al, cl
        and     al, 1
        mov     byte ptr [rbp - 8], al
        test    al, al
        jne     .LBB1_2
        jmp     .LBB1_1
.LBB1_1:
        xor     eax, eax
        mov     byte ptr [rbp - 9], al
        jmp     .LBB1_3
.LBB1_2:
        mov     al, 1
        mov     byte ptr [rbp - 9], al
        jmp     .LBB1_3
.LBB1_3:
        mov     al, byte ptr [rbp - 9]
        add     rsp, 16
        pop     rbp
        ret

output.h() -> Swift.Bool:
        push    rbp
        mov     rbp, rsp
        mov     al, 1
        pop     rbp
        ret

output.f() -> Swift.Bool:
        push    rbp
        mov     rbp, rsp
        sub     rsp, 16
        lea     rdi, [rbp - 8]
        xor     esi, esi
        mov     edx, 1
        call    memset@PLT
        call    (output.h() -> Swift.Bool)
        mov     cl, al
        and     cl, 1
        mov     byte ptr [rbp - 8], cl
        test    al, 1
        jne     .LBB3_1
        jmp     .LBB3_2
.LBB3_1:
        xor     eax, eax
        mov     byte ptr [rbp - 9], al
        jmp     .LBB3_3
.LBB3_2:
        mov     al, 1
        mov     byte ptr [rbp - 9], al
        jmp     .LBB3_3
.LBB3_3:
        mov     al, byte ptr [rbp - 9]
        add     rsp, 16
        pop     rbp
        ret

__swift_reflection_version:
        .short  3

So apparently, explicit comparison produces less assembly in unoptimized version. I cannot drive any conclusions from this though, especially about compilation time.

alexhunsley · January 24, 2025, 9:01am

So my main source for this is the link I put in the post at top:

Basically the compiler has more stuff to resolve to decide what to do when it sees ==, as opposed to boolExpr or !boolExpr.

Yet another way in which ! and == bool are distinct, despite surface appearances.

(I've not re-validated the finding of slower compilation (yet) so not sure of current performance.)

vns · January 24, 2025, 9:12am

Thanks, that’s an interesting input, I was assuming that this might be due to type checking, which usually the first thing to slow the compiler. I wonder if this still holds after 4.5 years, would be interesting to do checks… That’s pretty unfortunate of course.

jtbandes · January 25, 2025, 10:33pm

Reading this thread is a fun blast from the past :)