NSNull: fix === returning false when both operands are nil#5486
Open
ayush-that wants to merge 1 commit into
Open
NSNull: fix === returning false when both operands are nil#5486ayush-that wants to merge 1 commit into
ayush-that wants to merge 1 commit into
Conversation
The current `===(NSNull?, NSNull?)` overload uses `guard let _ = lhs, let _ = rhs` and falls through to `return false` when either operand is nil. That makes `nil === nil` return false, contradicting the standard identity-operator contract. Two non-nil NSNull instances still compare ===, so the existing test_alwaysEqual case continues to pass. Replace the guard with a switch over the optional pair so the cases are explicit: (nil, nil) is true, mixed-nil is false, and two non-nil NSNull instances remain identical (preserving the singleton-like behavior). Add a matching !== overload so the symmetric operator agrees with === for the NSNull? signature. Without it, Swift's default !== for AnyObject? would return true for two distinct NSNull instances even though === says they are identical. Resolves swiftlang#5248.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes the === operator overload for NSNull? so that nil === nil returns true (previously returned false), and adds a matching !== overload. Includes new tests covering identity comparisons.
Changes:
- Rewrite
===forNSNull?to correctly handle the(nil, nil)case. - Add a
!==operator overload forNSNull?. - Add
test_identityOperatorand minor cleanups inTestNSNull.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Sources/Foundation/NSNull.swift | Corrects === semantics for optional NSNull and adds !==. |
| Tests/Foundation/TestNSNull.swift | Adds coverage for the new identity operator behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
In Objective-C, NSNull's allocator will always return the singleton. There is no way to get two distinct instances of |
parkera
approved these changes
Jun 30, 2026
Contributor
|
@swift-ci test |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5248.
The hack in
===(NSNull?, NSNull?)returns false when either operand is nil, sonil === nilreturns false. Replace the guard with an explicit switch on the optional pair:(nil, nil)is true, mixed-nil is false, and two non-nil NSNull instances remain identical (preserving the singleton behavior).Also add a matching
!==overload so the symmetric operator agrees with===for this signature. Without it, Swift's default!==forAnyObject?returns true for two distinct NSNull instances even though the custom===says they are identical.The new operator is a small public-API addition. Happy to drop it and land only the bug fix if a swift-evolution discussion is preferred.
Tests
Added
test_identityOperator()covering the full truth table (nil/nil, mixed, non-nil/non-nil for both===and!==). Existingtest_alwaysEqual()still passes; added one supplemental!==assertion to it.Verified on Linux via
swiftlang/swift:nightly-main-jammy: