Skip to content

Adopt Sendability in Remaining Models #251

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

Merged
merged 2 commits into from
Jun 13, 2025

Conversation

moshe-foreflight
Copy link
Contributor

@moshe-foreflight moshe-foreflight commented Jun 6, 2025

Several structs already declared Sendable conformance. This PR adds support to (most of) the remaining ones.

  • SymbolOccurrence and SymbolRelation are easy and safe changes.
  • IndexStoreDBError, PathMapping, and StoreUnitInfo are similar, but I don't know if this change is what you had in mind, @ahoppen.

Less safe changes:

  • IndexDelegate can be useful to wrap in a Swift actor, so I'd like to mark this as Sendable, but am unsure if this is wise, given there's mention of asynchronous behavior within that protocol.
  • IndexStoreDB contains an UnsafeMutableRawPointer so the best that we can do (with what I'm aware of) is @unchecked Sendable.

Reviewers: Please let me know if any of these changes should be adjusted and/or how we might test this beyond visual inspection.

Addresses conversation on #248 and issues #250.

Several structs already declared Sendable conformance. This PR adds support to (most of) the remaining ones.

- `SymbolOccurrence` and `SymbolRelation` are easy and safe changes.
- `IndexStoreDBError`, `PathMapping`, and `StoreUnitInfo` are similar, but I don't know if this change is beneficial.
- `IndexStoreDB` contains an `UnsafeMutableRawPointer` so the best that we can do (with what I'm aware of) is `@unchecked Sendable`.

Please let me know if any of these changes should be adjusted and/or how we might test this beyond visual inspection.
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. By related types I meant SymbolLocation, SymbolRole etc. I just forgot that I added those conformances already in the past. Your additions look good to me with two exceptions. See inline.

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@ahoppen
Copy link
Member

ahoppen commented Jun 11, 2025

@swift-ci Please test

@bnbarham
Copy link
Contributor

@swift-ci please test

@ahoppen ahoppen merged commit 47ba8ad into swiftlang:main Jun 13, 2025
3 checks passed
@moshe-foreflight moshe-foreflight deleted the moshe-foreflight/sendability branch June 13, 2025 15:37
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