Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OptionSet that is also an NSObject seems to be broken #72765

Open
igor-makarov opened this issue Apr 2, 2024 · 4 comments
Open

OptionSet that is also an NSObject seems to be broken #72765

igor-makarov opened this issue Apr 2, 2024 · 4 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels

Comments

@igor-makarov
Copy link

Description

When defining an OptionSet that inherits from NSObject, the contains(_:) method seems to be broken.

It's possible that other functionality is also broken. There are no compiler warnings or errors about this.

Reproduction

public class OptionA: NSObject, OptionSet {
    public let rawValue: Int32
    public static let alpha: OptionA = .init(rawValue: 1 << 0)
    public static let beta: OptionA = .init(rawValue: 1 << 1)
    public required init(rawValue: Int32) {
        self.rawValue = rawValue
    }
    // this seems to resolve it, but what else is wrong?
    // public func contains(_ member: OptionA) -> Bool {
    //     rawValue & member.rawValue == member.rawValue
    // }
}

print(OptionA.alpha.contains(.alpha)) // prints false

Expected behavior

The contains(_:) method should be implemented correctly, similar to cases where the OptionSet is not an NSObject.

I don't know whether other OptionSet methods are affected.

Environment

swift-driver version: 1.87.3 Apple Swift version 5.9.2 (swiftlang-5.9.2.2.56 clang-1500.1.0.2.5)
Target: arm64-apple-macosx14.0

Additional information

No response

@igor-makarov igor-makarov added bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels labels Apr 2, 2024
@benrimmington
Copy link
Collaborator

I suspect that your subclass needs to override isEqual(_:) and hash. For example:

public override func isEqual(_ object: Any?) -> Bool {
  (object as? Self)?.rawValue == rawValue
}

public override var hash: Int {
  rawValue.hashValue
}

The default implementations from NSObject only use the memory address of the instance.

@igor-makarov
Copy link
Author

It looks like the docs only say "For your type to automatically receive default implementations for set-related operations, the rawValue property must be of a type that conforms to the FixedWidthInteger protocol, such as Int or UInt8".

Doesn't seem to be anything about Equatable...

@benrimmington
Copy link
Collaborator

OptionSet inherits from SetAlgebra, which inherits from Equatable.

The documentation probably assumes that your option set will be a struct, for which the compiler will synthesize an Equatable conformance (SE-0185).

@igor-makarov
Copy link
Author

@benrimmington You're right, there are 2 occurrences where SetAlgebra is relying on Equatable:

  // 1
  public func isSubset(of other: Self) -> Bool {
    return self.intersection(other) == self
  }
  // 2
  public var isEmpty: Bool {
    return self == Self()
  }

Both of these are broken if the OptionSet is a subclass of NSObject, unless -[isEqual:] is overridden.

This NSObject auto-conformance is a footgun that I've sadly triggered far too many times to count.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels
Projects
None yet
Development

No branches or pull requests

2 participants