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

Jump to bottom

[SR-10118] Warn when OptionSet static constants are initialized with a zero value #52520

Closed
beccadax opened this issue Mar 16, 2019 · 12 comments
Closed

[SR-10118] Warn when OptionSet static constants are initialized with a zero value #52520

beccadax opened this issue Mar 16, 2019 · 12 comments
Labels
compiler The Swift compiler in itself diagnostics QoI Bug: Diagnostics Quality of Implementation good first issue Good for newcomers improvement missing warning Bug: Missing warning OptionSet Area → standard library: The `OptionSet` protocol type checker Area → compiler: Semantic analysis

Comments

@beccadax
Copy link
Contributor

Previous ID SR-10118
Radar rdar://problem/48689254
Original Reporter @beccadax
Type Improvement
Status Resolved
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Improvement, DiagnosticsQoI, StarterBug
Assignee Vercantez (JIRA)
Priority Medium

md5: dd3960163b3287c2517c33c41718adda

Issue Description:

An OptionSet-conforming instance with a value of 0 behaves very strangely; for instance, contains(_:) will always return true for it. Because of this, and because [] is the preferred syntax for an empty option set, ClangImporter removes none members when it imports an NS_OPTIONS enum.

However, if you write an OptionSet-conforming type in Swift, there's nothing stopping you from adding one yourself:

struct MyOptions: OptionSet {
  var rawValue: Int
  static let none = MyOptions(rawValue: 0)
}

It would be nice if the compiler emitted a warning when you did this. The criteria would probably be something like:

  • A call to init(rawValue:)

  • Passing the literal 0

  • In an initial value expression (Initializer, in the compiler's parlance)

  • For a static let variable

  • In a type conforming to OptionSet

  • When the variable belongs to the type it's declared within
@belkadan
Copy link
Contributor

One problem is that there has to be a way to silence the warning. For the importer, that's providing an explicit NS_SWIFT_NAME, but that's less obvious here. Any ideas?

@beccadax
Copy link
Contributor Author

There are a number of ways you could write zero that we could intentionally not detect, such as (0) or .zero (since the raw type must conform to FixedWidthInteger, which refines AdditiveArithmetic). We could also suggest that you write MyOptions([]), which explicitly uses the syntax we want to encourage.

@belkadan
Copy link
Contributor

Ah, good idea with []. static let none: MyOptions = [] would also work. Okay!

@swift-ci
Copy link
Collaborator

Comment by Miguel Salinas (JIRA)

Just checking in. I got started with this after try! Swift SJ a couple weeks ago. Working slowly but steadily. One question I’ve had, however, is why the strange behavior isn’t changed instead of emitting a warning.

@belkadan
Copy link
Contributor

The behavior is "correct" by the definition of OptionSet: any particular set contains all zero of the elements in the none set.

@swift-ci
Copy link
Collaborator

Comment by Miguel Salinas (JIRA)

Ah ok![]( That makes sense. I misunderstood the behavior. I thought that the none set would return true for any set. e.g. MyOptions.none.contains( x ) would return true for all x. A simple test proved my assumption wrong. Thanks)

@belkadan
Copy link
Contributor

Did you end up putting up a PR for this, Vercantez (JIRA User)?

(I'm checking in on all the StarterBugs that haven't been touched in over a month; it's totally fine if you just haven't had time but still want to keep it.)

@swift-ci
Copy link
Collaborator

Comment by Miguel Salinas (JIRA)

Haven't put out a PR yet. Sorry for the late response.

@swift-ci
Copy link
Collaborator

swift-ci commented Sep 5, 2019

Comment by Miguel Salinas (JIRA)

Almost done. I'll put out a PR pretty soon.

@swift-ci
Copy link
Collaborator

swift-ci commented Sep 9, 2019

Comment by Miguel Salinas (JIRA)

PR: #27089

@swift-ci
Copy link
Collaborator

swift-ci commented Nov 4, 2019

Comment by Miguel Salinas (JIRA)

#27089

@swift-ci
Copy link
Collaborator

swift-ci commented Feb 4, 2022

Comment by Robert (JIRA)

Hello, why it doesn't work in case:

(rawValue: 1 << 999999)

?

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@AnthonyLatsis AnthonyLatsis added type checker Area → compiler: Semantic analysis OptionSet Area → standard library: The `OptionSet` protocol missing warning Bug: Missing warning labels Sep 15, 2023
@AnthonyLatsis AnthonyLatsis mentioned this issue Sep 15, 2023
Closed
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler The Swift compiler in itself diagnostics QoI Bug: Diagnostics Quality of Implementation good first issue Good for newcomers improvement missing warning Bug: Missing warning OptionSet Area → standard library: The `OptionSet` protocol type checker Area → compiler: Semantic analysis
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@belkadan @beccadax @swift-ci @AnthonyLatsis