Skip to content
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

[Swift 4.1] Fix the handling of defer statements in the playground transform and PC macro. #13885

Merged
merged 5 commits into from Jan 13, 2018
Merged

[Swift 4.1] Fix the handling of defer statements in the playground transform and PC macro. #13885

merged 5 commits into from Jan 13, 2018

Conversation

cwakamo
Copy link
Member

@cwakamo cwakamo commented Jan 12, 2018

Cherry-picks changes in #13835 which fix the playground transform and the PC macro so that they correctly instrument defer statements. (Previously, both ignored defer entirely.)

Resolves SR-5641 / <rdar://problem/33764082>.

@cwakamo
[PlaygroundTransform] Implemented support for defer statements.
ce6edea 
Added handling of `defer` statements in the playground transform.
Tests will be added in a follow-on commit.

Patch by Roman Levenstein!

This addresses SR-5641/<rdar://problem/33764082>.

(cherry picked from commit 0a9a4e7)
@cwakamo
[test] Introduced tests of the playground transform and defer and nes…
6f9e74d 
…ted functions.

(cherry picked from commit 9ac03b7)
@cwakamo
[PCMacro] Implemented support for defer statements in the PC macro.
edc8f16 
As with the playground transform, defer statements were not supported in the PC macro.
This commit addresses that oversight.

This partially addresses <rdar://problem/29007242>.

(cherry picked from commit 2c04058)
@cwakamo
[test] Introduced a test of the PC macro and nested functions.
23e1eb9 
(cherry picked from commit 2f9ab44)
@cwakamo
Added comments based on review feedback.
8c83bf7 
(cherry picked from commit 1d602f2)
@cwakamo
Copy link
Member Author

cwakamo commented Jan 12, 2018

@swift-ci Please test

@jrose-apple
Copy link
Contributor

Cherry-picks to the 4.1 branch do need approval at this point.

@cwakamo
Copy link
Member Author

cwakamo commented Jan 12, 2018

Thanks for the reminder; I'll nominate this once the tests finish running.

@cwakamo
Copy link
Member Author

cwakamo commented Jan 12, 2018

  • Explanation: Defer statements were previously unhandled by both the playground transform and the PC macro, so any code in a defer statement was not instrumented. This updates both instrumenters to handle defer statements correctly.
  • Scope: Affects defer statements in playgrounds where the playground transform and/or the PC macro is enabled. If the instrumenters are disabled (as is the case in all non-playground contexts), then no change in behavior should be observed.
  • SR Issue: SR-5641
  • Risk: Low; the only risk is that code in defer statements is incorrectly instrumented, but this change uses the existing instrumenters for functions so the risk of that should be low.
  • Testing: Added new tests of the playground transform and the PC macro with defer statements.
  • Reviewers: Jordan Rose reviewed the original PR ([SR 5641] Implement support for defer in the playground transform and PC macro. #13835).

@bob-wilson bob-wilson merged commit 2e48f09 into apple:swift-4.1-branch Jan 13, 2018
@cwakamo cwakamo deleted the fix-defer-playground-pcmacro-swift-4.1 branch February 15, 2018 02:42
@belkadan belkadan mentioned this pull request Feb 21, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrose-apple jrose-apple Awaiting requested review from jrose-apple

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@cwakamo @jrose-apple @bob-wilson