Skip to content

Sema: Allow collection downcast in cast pattern and remove the diagnostic that it is not implemented #60682

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.

Sign up for GitHub

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
Merged
merged 1 commit into from
Nov 2, 2022
Merged

Sema: Allow collection downcast in cast pattern and remove the diagnostic that it is not implemented #60682

merged 1 commit into from
Nov 2, 2022

Conversation

simanerush
Copy link
Member

@simanerush simanerush commented Aug 20, 2022 •
edited
Loading

Resolves #56139.

xedin
xedin reviewed Aug 21, 2022
View reviewed changes
test/Parse/matching_patterns.swift Outdated
class Base { }
class Derived : Base { }


switch [Derived(), Derived(), Base()] {
case let ds as [Derived]: // expected-error{{collection downcast in cast pattern is not implemented; use an explicit downcast to '[Derived]' instead}}
case let ds as [Derived]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a test-case that uses switch + case let ... as ... inside of a closure and SILGen test-case for that as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I will address this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xedin I just pushed new changes. It has more tests!

@simanerush simanerush marked this pull request as ready for review August 26, 2022 22:27
AnthonyLatsis
AnthonyLatsis reviewed Aug 28, 2022
View reviewed changes
Copy link
Collaborator

@AnthonyLatsis AnthonyLatsis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

AnthonyLatsis
AnthonyLatsis reviewed Aug 30, 2022
View reviewed changes
@simanerush simanerush force-pushed the remove-diagnostics branch 4 times, most recently from efc15d7 to 7c560d8 Compare September 1, 2022 22:22
@AnthonyLatsis AnthonyLatsis requested review from slavapestov and removed request for jckarter September 13, 2022 11:31
@AnthonyLatsis
Copy link
Collaborator

AnthonyLatsis commented Sep 13, 2022 •
edited
Loading

Note: Requesting Slava to review SIL tests.

AnthonyLatsis
AnthonyLatsis reviewed Sep 13, 2022
View reviewed changes
test/SILGen/switch.swift Outdated
Comment on lines 733 to 734
// CHECK-LABEL: sil hidden [ossa] @$s6switch30test_isa_pattern_set_downcastt2psyShyxG_tSHRzlF : $@convention(thin) <T where T : Hashable> (@guaranteed Set<T>) -> () {
func test_isa_pattern_set_downcastt<T: Hashable>(ps: Set<T>) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not to commit directly:

Suggested change
// CHECK-LABEL: sil hidden [ossa] @$s6switch30test_isa_pattern_set_downcastt2psyShyxG_tSHRzlF : $@convention(thin) <T where T : Hashable> (@guaranteed Set<T>) -> () {
func test_isa_pattern_set_downcastt<T: Hashable>(ps: Set<T>) {
// CHECK-LABEL: sil hidden [ossa] @$s6switch29test_isa_pattern_set_downcast2psyShyxG_tSHRzlF : $@convention(thin) <T where T : Hashable> (@guaranteed Set<T>) -> () {
func test_isa_pattern_set_downcast<T: Hashable>(ps: Set<T>) {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed that, thanks!

@simanerush simanerush force-pushed the remove-diagnostics branch 2 times, most recently from 1ef521e to 968507c Compare September 13, 2022 18:04
@simanerush simanerush changed the title Sema: Allow "case let value as [Int]" and remove the diagnostic that it is not implemented Sema: Allow collection downcast in cast pattern and remove the diagnostic that it is not implemented Sep 19, 2022
@AnthonyLatsis
Copy link
Collaborator

@swift-ci please smoke test

@AnthonyLatsis
Copy link
Collaborator

AnthonyLatsis commented Sep 19, 2022 •
edited
Loading

@simanerush The test failure is a false positive, a single-element array literal cannot possibly be heterogeneous. Looks like we need another issue report. The warning will have to be verified in the meantime.

@simanerush
Copy link
Member Author

@slavapestov could you take a look?

@AnthonyLatsis
Copy link
Collaborator

@swift-ci please smoke test macOS

@AnthonyLatsis
Copy link
Collaborator

@swift-ci please smoke test

@AnthonyLatsis
Copy link
Collaborator

@swift-ci please clean smoke test macOS

@AnthonyLatsis
Copy link
Collaborator

@swift-ci please smoke test macOS

AnthonyLatsis
AnthonyLatsis approved these changes Nov 2, 2022
View reviewed changes
Copy link
Collaborator

@AnthonyLatsis AnthonyLatsis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! There are no functional changes to SIL generation anyway, so let’s land this. We can open a new PR shall Slava have any comments on test matter later on.

@AnthonyLatsis AnthonyLatsis merged commit e989357 into swiftlang:main Nov 2, 2022
@AnthonyLatsis
Copy link
Collaborator

@xedin Do you reckon this needs a changelog entry?

@simanerush simanerush deleted the remove-diagnostics branch November 2, 2022 19:08
@simanerush simanerush mentioned this pull request Nov 20, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xedin xedin xedin approved these changes

@AnthonyLatsis AnthonyLatsis AnthonyLatsis approved these changes

@slavapestov slavapestov Awaiting requested review from slavapestov slavapestov is a code owner

@ahoppen ahoppen Awaiting requested review from ahoppen

@bnbarham bnbarham Awaiting requested review from bnbarham bnbarham is a code owner

@CodaFi CodaFi Awaiting requested review from CodaFi CodaFi is a code owner

@DougGregor DougGregor Awaiting requested review from DougGregor DougGregor is a code owner

@rintaro rintaro Awaiting requested review from rintaro rintaro is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[SR-13742] Allow "case let value as [Int]" and remove the diagnostic that it is not implemented
3 participants
@simanerush @AnthonyLatsis @xedin