Skip to content

[sil] Introduce FullApplySiteKind and ApplySiteKind to allow for exha… #19436

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

Conversation

gottesmm
Copy link
Contributor

…ustive switching over FullApplySites and ApplySiteKind.

Currently there is a bug in the closure specializer that was caused by
BeginApply not being handled correctly. Rather than just fixing that and leaving
the badness, I am instead in this commit introducing enums for apply sites so we
can avoid this problem in the future by using exhaustive switches to guide
developers adding new types of apply sites in the future.

rdar://44612356

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

@gottesmm gottesmm requested review from rjmccall and atrick September 20, 2018 23:03
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm gottesmm requested a review from eeckstein September 20, 2018 23:04
@gottesmm
Copy link
Contributor Author

Was just thinking... It may make sense for FullApplySiteKind to be a range instead of a unique type.

@gottesmm
Copy link
Contributor Author

No. I am wrong. Sorry, I confused myself.

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test macOS platform

Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

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

Cool.

operator innerty() const { return value; }

static Optional<ApplySiteKind>
mapNodeKindToApplySiteKind(SILInstructionKind kind) {
Copy link
Contributor

Choose a reason for hiding this comment

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

fromNodeKind says it all.

@gottesmm gottesmm force-pushed the pr-e2ea20678fd79a26d047317cef26331dddb0298e branch from 8d8facf to d783db4 Compare September 21, 2018 02:12
@gottesmm
Copy link
Contributor Author

Fixed and merging!

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

2 similar comments
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

…ustive switching over FullApplySites and ApplySiteKind.

Currently there is a bug in the closure specializer that was caused by
BeginApply not being handled correctly. Rather than just fixing that and leaving
the badness, I am instead in this commit introducing enums for apply sites so we
can avoid this problem in the future by using exhaustive switches to guide
developers adding new types of apply sites in the future.

rdar://44612356
@gottesmm gottesmm force-pushed the pr-e2ea20678fd79a26d047317cef26331dddb0298e branch from d783db4 to e3eb9ae Compare September 21, 2018 02:19
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

1 similar comment
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

nice!

Copy link
Contributor

@rjmccall rjmccall left a comment

Choose a reason for hiding this comment

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

Yeah, good idea.

@gottesmm gottesmm merged commit 78cd814 into swiftlang:master Sep 21, 2018
@gottesmm gottesmm deleted the pr-e2ea20678fd79a26d047317cef26331dddb0298e branch September 21, 2018 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants