-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[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
[sil] Introduce FullApplySiteKind and ApplySiteKind to allow for exha… #19436
Conversation
@swift-ci smoke test |
Was just thinking... It may make sense for FullApplySiteKind to be a range instead of a unique type. |
No. I am wrong. Sorry, I confused myself. |
@swift-ci smoke test macOS platform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool.
include/swift/SIL/SILInstruction.h
Outdated
operator innerty() const { return value; } | ||
|
||
static Optional<ApplySiteKind> | ||
mapNodeKindToApplySiteKind(SILInstructionKind kind) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fromNodeKind
says it all.
8d8facf
to
d783db4
Compare
Fixed and merging! |
@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
d783db4
to
e3eb9ae
Compare
@swift-ci smoke test and merge |
1 similar comment
@swift-ci smoke test and merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, good idea.
…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.