Skip to content

[5.0][SourceKit/SwiftLang] Keep sourcekitd response alive for variant lifetime #20375

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

nathawes
Copy link
Contributor

@nathawes nathawes commented Nov 6, 2018

This PR contains two changes to SwiftLang, the Swift layer exposing SourceKit in Swift:

  1. Make each Variant keep a strong reference to its SourceKitdResponse context. This is needed because sourcekitd_variant_t is only safe to use while the sourcekitd_response_t it was retrieved from (wrapped by SourceKitdResponse) is still alive.
  2. Expose the new Data variant SourceKit APIs that were added to support returning the Syntax tree in a binary format in SwiftLang.

PR for master: #20370

Nathan Hawes added 2 commits November 6, 2018 14:45
…tdResponse context

sourcekitd_variant_t is only safe to use while the sourcekitd_response_t it was
retrieved from is still alive, so keep a strong reference to SourceKitdResponse
(the Swift wrapper of sourcekitd_response_t) in each Variant (the Swift wrapper
of sourcekitd_variant_t).
…wrapper

These were recently added to support returning the SyntaxTree in the bytetree
from SourceKit but were never added in SwiftLang (the Swift layer wrapping
SourceKit).
@nathawes nathawes requested a review from nkcsgexi November 6, 2018 22:55
@nathawes
Copy link
Contributor Author

nathawes commented Nov 6, 2018

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Nov 7, 2018

Build failed
Swift Test Linux Platform
Git Sha - f155ee7

@nkcsgexi nkcsgexi merged commit f9887c6 into swiftlang:swift-5.0-branch Nov 7, 2018
@nathawes nathawes deleted the keep-sourcekitd-response-alive-for-variant-lifetime-5.0 branch November 7, 2018 03:23
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.

3 participants