Add a newer Zstd module for Swift use, with a more idiomatic API, plus nullability annotations#4665
Open
wadetregaskis wants to merge 5 commits into
Open
Add a newer Zstd module for Swift use, with a more idiomatic API, plus nullability annotations#4665wadetregaskis wants to merge 5 commits into
Zstd module for Swift use, with a more idiomatic API, plus nullability annotations#4665wadetregaskis wants to merge 5 commits into
Conversation
This is generated (by default) when building with `swift` (the Swift CLI), e.g. `swift build`.
This folder contains generated Xcode project files that are used locally in some circumstances but should never be checked in (they're derived from Package.swift).
…existing `libzstd`. The new module exposes the same fundamental API but with names that are cleaner and more idiomatic in Swift, and with some key additional Swift-specific annotations (such as on closed enums, to make them import as Swift enums rather than just free-floating constants). `libzstd` remains unchanged for backwards compatibility (or those that prefer the "raw" C API, I suppose). Note that this stops short of actual code changes or API changes beyond just renames (e.g. it does not change the parameter orders to be more idiomatic in Swift, nor present the various structs as actual Swift structs or classes). It _could_ - it could include a whole shim layer, written in C and/or Swift, to make the Swift API look like whatever we want - but it's a conscious choice to not go that far at this time. There are existing Swift packages which wrap this library and perform more substantial API changes.
These apply to the C API as well (but are not supported by all compilers, e.g. GCC still doesn't AFAIK). They are important for Swift because they prevent the API using implicitly unwrapped optionals everywhere. In many cases the parameter (or return value) is _not_ in fact optional, and when it is that should be exposed explicitly in Swift (per Swift best practice).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a set of related patches which improve Swift support, by providing a new version of the Swift module with an improved API (named
Zstdto be distinct from the existinglibzstdwhich remains, unchanged, for backwards compatibility). This new version of the Swift API differs in that:Zstd.compress(…)).ZSTD_inBuffer_s→InputBuffer,ZSTD_getDictID_fromDDict→dictionaryID(fromDecompressionDictionary:)).ZSTD_compress→compress(into:capacity:from:size:level:)).I'd really like to map the C structs to Swift structs / classes, but unfortunately in order to do so those C structs have to be defined in the headers. Currently they are merely declared in the headers, making them pseudo-anonymous. If changing that is an option, I'd be happy to do that (the mapping into Swift would still not be perfect, since these C structs aren't reference counted nor intended to be pure value types, so they don't map cleanly to Swift classes or structs - but that can be mitigated).
Note that the C API is changed in that the nullability annotations apply there too, under compatible compilers (e.g. Clang). This has limited if any effect at runtime (the nullability information is compiler metadata only, though it may influence optimisation decisions) but may introduce new compilation warnings or errors for users of the C API. Some of those errors or warnings may be false positives - e.g. cases where the code doesn't clearly prevent inappropriate use of
nullbut in practicenullnever happens.I tried to ensure
libzstdis completely unchanged, including continued use of implicitly-unwrapped optionals - though I could go either way on that specific point. Since the C API is technically changing (in a limited way - nullability annotations), maybe it's reasonable to change the existing Swift API in that limited way too?Alternatives considered
Do nothing
The existing API, as expoed to Swift in
libzstd, is functional. But it's pretty unpleasant to use, because of how C works in Swift (it's a little like having to write assembly in C, in terms of appeal, complexity, and [lack of] safety). Granted this newZstdSwift module doesn't entirely solve that - it's still dealing with raw pointers, for example - but it does help a lot.Rely on intermediary Swift packages
There are existing 3rd party Swift packages which wrap the C library into an even more idiomatic Swift interface (with proper structs or classes, and extensions on standard Swift types like
Dataand[UInt8]), and it's certainly possible to implement changes like that here too - we can include C and/or Swift shims. I didn't pursue that [for now] because: