Skip to content

[sil] Add an implementation of isIndirectResultOperand() onto ApplySite that returns false for partial_apply. #33112

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

Sometimes when working with the operands of an ApplySite, one needs to bail
early if one has a full apply site and the operand is an indirect result
argument. Sadly, there isn't any such API on ApplySite (it is only on
FullApplySite), even though technically a partial_apply can be viewed as just
not having any indirect result operands! That is what this patch implements.

To implement this I added a simple useful method called:

Optional<FullApplySite> ApplySite::asFullApplySite() const

Seems like a useful thing to have for these sorts of cases.

…te that returns false for partial_apply.

Sometimes when working with the operands of an ApplySite, one needs to bail
early if one has a full apply site and the operand is an indirect result
argument. Sadly, there isn't any such API on ApplySite (it is only on
FullApplySite), even though technically a partial_apply can be viewed as just
not having any indirect result operands! That is what this patch implements.

To implement this I added a simple useful method called:

```
Optional<FullApplySite> ApplySite::asFullApplySite() const
```

Seems like a useful thing to have for these sorts of cases.
@gottesmm gottesmm requested a review from atrick July 25, 2020 00:36
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm gottesmm requested a review from meg-gupta July 26, 2020 19:29
@gottesmm gottesmm merged commit 396709c into swiftlang:master Jul 27, 2020
@gottesmm
Copy link
Contributor Author

I think this is pretty uncontroversial.

@gottesmm gottesmm deleted the pr-c334b2062d3c019bcc64f91201c92417548b8992 branch July 27, 2020 18:04

namespace swift {

inline Optional<FullApplySite> ApplySite::asFullApplySite() const {
Copy link
Contributor

Choose a reason for hiding this comment

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

@gottesmm this new API is inconsistent with existing apply site type coercion which always returns an invalid apply site on failure. Adding another layer of Optional is extremely confusing. I don't think you need this API anyway, but it definitely should not change the return type.

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.

2 participants