I'd like to get some feedback on how best to extend sourcekit to better support IDE features to surface more concurrency information. I discussed this a bit here, and have a draft PR (here) that I would appreciate some directional guidance on. I haven't yet received much input on the design/implementation strategy, so I thought I'd try posting in this category of the forums to see if that gets the right eyes on things.
As I mentioned in the thread over in the Sourcekit-LSP category, two specific ideas I think would be useful to surface (or at least make possible to surface) are:
- Inferred isolation information, especially for closures.
- "Boundary" isolation-crossing callsites.
This information currently exists in the AST, so IIUC all we need to do is decide upon mechanisms to surface it through sourcekit.
In the current draft implementation, I've only attempted to expose inferred isolation information, exclusively for closures at the moment. The implementation seems fairly straightforward – I basically just copied the existing logic for inferred type info for declarations, but applied to inferred closure isolation instead. A sketch of what it does is roughly:
- There's some boilerplate copied from surrounding examples for a new
CollectInferredIsolationrequest (TBH I have not thought much about this, and mostly just copy/pasted from nearby logic). - The bulk of the actual implementation logic to fulfill the request is in
InferredIsolationCollector. This basically mirrors the wayVariableTypeCollectorworks, except it collects inferred isolation info only for closures. As we walk the AST, we inspect closures we encounter. If we find explicitly-written ones that we think have had isolation inferred (e.g. have no source-level attribute like@concurrentor@SomeGlobalActorin their signature) then we record that information. - The string values representing the inferred isolation are deduplicated and stored in a buffer, and bookkeeping info to correlate source locations to this isolation information is recorded in
InferredIsolationInfo(again, this basically copies what's done in theVariableTypeCollector/ExpressionTypeCollectorcases).
This has raised a number of questions, largely around trying to figure out how generic such an API should be, and what sorts of evolution concerns may be relevant.
In no particular order, some things I've been thinking:
- Is copying the existing variable/expression type collection logic appropriate? Structurally all of these things seem like they're doing quite similar work, so perhaps the logic should be consolidated. Would it make sense to explore a more general API for "inferred type" information, that could consolidate the existing var/expr requests and support this new use case?
- How general should this new inferred isolation collector be? As pointed out in another thread, it may be desirable to collect inferred isolation info on things other than closures, but IDEs may want flexibility in what they gather and display and display based on IDE settings (e.g. show all inferred isolation, closures-only, etc). This could be done via separate requests or a single request could be parameterized. The current implementation returns a "kind" value in the response payload to indicate the entity to which the isolation is attached (e.g. hard-coded to "closure" right now, but could be extended to other sorts of things).
- I noticed there is an outstanding feature request to supplement "cursor info" requests with this inferred closure isolation information. I've not looked into this much, but would that require separate implementation logic independent of this proposed new request (I'm currently assuming "yes", but may not understand properly)?
Lastly, I frankly still do not understand the flow of data in this service very well yet (in particular, how data is transferred from sourcekit to its clients), so if someone feels they do and would care to enlighten me, or can point to some docs or specific areas of the codebase that could speed up my mental-model construction that would be appreciated. Thanks!