Skip to content

[5.1][CSApply] Restructure the implicit AST when applying @dynamicCallable #26004

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

Closed

Conversation

Azoy
Copy link
Contributor

@Azoy Azoy commented Jul 8, 2019

  • Explanation: This patch cleans up how we handle @dynamicCallable applications and how the implicit AST is created.
  • Scope: This is a non source breaking change that just fixes some broken use cases of @dynamicCallable.
  • SRs: SR-10313, SR-9239, and SR-10753
  • Risk: This should be relatively low risk because we maintain source stability, but fix some compiler crashes with some use cases.
  • Reviewers: @jrose-apple and @slavapestov

[CSApply] Restructure the implicit AST when applying @dynamicCallable
@CodaFi
Copy link
Contributor

CodaFi commented Jul 8, 2019

Please cherry pick the two commits you landed instead of the merge commit. It makes the history easier to reason about.

@jrose-apple
Copy link
Contributor

jrose-apple commented Jul 8, 2019

I'm going to push back against that; cherry-picking the merge makes it easier to find the original PR and verify what got cherry-picked. (I don't think we have a formal policy either way.)

Can you add the information from the 5.1 Release Process?

@swift-ci Please test

@swift-ci
Copy link
Contributor

swift-ci commented Jul 8, 2019

Build failed
Swift Test Linux Platform
Git Sha - 234c9a1

@swift-ci
Copy link
Contributor

swift-ci commented Jul 8, 2019

Build failed
Swift Test OS X Platform
Git Sha - 234c9a1

@Azoy
Copy link
Contributor Author

Azoy commented Jul 9, 2019

Ahh, this relies on #24797, which has not been merged to 5.1.

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.

5 participants