close
The Wayback Machine - https://web.archive.org/web/20201125155112/https://github.com/apple/swift/pull/27382
Skip to content

/ swift

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

[CSApply] Don't diagnose a 'static let none' as ambiguous none unless its an enum type #27382

Merged
merged 7 commits into from Sep 28, 2019
Merged

[CSApply] Don't diagnose a 'static let none' as ambiguous none unless its an enum type #27382

merged 7 commits into from Sep 28, 2019

Conversation

@theblixguy
Copy link
Collaborator

@theblixguy theblixguy commented Sep 26, 2019

We were incorrectly diagnosing the following:

enum Foo {
  case bar
  static let none = 1
}

let optionalFoo: Foo? = .none // warning: assuming you mean 'Optional<Foo>.none'; 
                              // did you mean 'Foo.none' instead?

There's no ambiguity here because .none could only refer to Optional.none, since the type of Foo.none is Int.

We should still offer a warning in case the user does something like this:

enum Foo {
  case bar
  static let none = Foo.bar
}

let optionalFoo: Foo? = .none // warning

Resolves SR-11522.
@theblixguy
[CSApply] Don't diagnose a 'static let none' as ambiguous none unless…
8d869e6 
… its type matches the enum
@theblixguy theblixguy requested review from xedin and jrose-apple Sep 26, 2019
@jrose-apple
jrose-apple reviewed Sep 26, 2019
View changes
lib/Sema/CSApply.cpp Outdated Show resolved Hide resolved
@brentdax
brentdax requested changes Sep 26, 2019
View changes
Copy link
Contributor

@brentdax brentdax left a comment

Good idea!
test/decl/enum/enumtest.swift Show resolved Hide resolved
theblixguy added 2 commits Sep 26, 2019
@theblixguy
[CSApply] Filter out functions and instance members before the ambigu…
25e1bb5 
…ous none check
@theblixguy
[CSApply] Compare variable's type with the base type of the enum we'r…
aa565f5 
…e assigning to
@brentdax
brentdax reviewed Sep 26, 2019
View changes
test/decl/enum/enumtest.swift Outdated Show resolved Hide resolved
@jrose-apple
jrose-apple reviewed Sep 27, 2019
View changes
lib/Sema/CSApply.cpp Outdated Show resolved Hide resolved
theblixguy added 2 commits Sep 27, 2019
@theblixguy
[CSApply] Move the variable type check into the lookup filter call
0a39e7d
@theblixguy
[Test] Add a FIXME next to a test case that should be diagnosed
Loading status checks…
389232a
@theblixguy theblixguy force-pushed the theblixguy:fix/SR-11522 branch to 389232a Sep 27, 2019
@theblixguy
Copy link
Collaborator Author

@theblixguy theblixguy commented Sep 27, 2019

Alright, this should be good to go now.

@swift-ci please smoke test
@jrose-apple
jrose-apple reviewed Sep 27, 2019
View changes
lib/Sema/CSApply.cpp Outdated Show resolved Hide resolved
@theblixguy
[CSApply] Filtering 'none' results should filter out any other decl (…
2de4755 
…such as typealiases, structs, etc) by default
@theblixguy theblixguy force-pushed the theblixguy:fix/SR-11522 branch 3 times, most recently Sep 27, 2019
@theblixguy
[Test] Add some more tests around edge cases
Loading status checks…
b3d7962
@theblixguy theblixguy force-pushed the theblixguy:fix/SR-11522 branch to b3d7962 Sep 27, 2019
@jrose-apple
jrose-apple approved these changes Sep 27, 2019
View changes
lib/Sema/CSApply.cpp Show resolved Hide resolved
test/decl/enum/enumtest.swift Show resolved Hide resolved
@theblixguy
Copy link
Collaborator Author

@theblixguy theblixguy commented Sep 27, 2019

@swift-ci please smoke test
@brentdax
brentdax approved these changes Sep 28, 2019
View changes
@theblixguy
Copy link
Collaborator Author

@theblixguy theblixguy commented Sep 28, 2019

@swift-ci please smoke test Linux platform
@theblixguy theblixguy merged commit ad4d623 into apple:master Sep 28, 2019
2 checks passed
2 checks passed
Swift Test Linux Platform (smoke test)
Details
Swift Test OS X Platform (smoke test)
Details
@theblixguy theblixguy deleted the theblixguy:fix/SR-11522 branch Sep 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brentdax brentdax

@jrose-apple jrose-apple

@xedin xedin

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@theblixguy @brentdax @jrose-apple
You can’t perform that action at this time.