Should Algorithm's public structs be @frozen?

We distribute an SDK which depends in the Charts library as an external dependency, recent versions of which now depend on Algorithms. Our build process for the SDK archive leverages Carthage, with us setting BUILD_LIBRARY_FOR_DISTRIBUTION to YES as one of the build settings to Carthage.

However, with the recent versions of Charts that now depend on Algorithms, setting BUILD_LIBRARY_FOR_DISTRIBUTION results in the following build errors:

/Users/djs/Library/Caches/org.carthage.CarthageKit/DerivedData/14.0_14A309/Charts/v4.1.0/SourcePackages/checkouts/swift-algorithms/Sources/Algorithms/AdjacentPairs.swift:66:10: error: 'let' property 'base' may not be initialized directly; use "self.init(...)" or "self = ..." instead
    self.base = base
         ^
/Users/djs/Library/Caches/org.carthage.CarthageKit/DerivedData/14.0_14A309/Charts/v4.1.0/SourcePackages/checkouts/swift-algorithms/Sources/Algorithms/AdjacentPairs.swift:61:16: note: 'base' declared here
  internal let base: Base
               ^
/Users/djs/Library/Caches/org.carthage.CarthageKit/DerivedData/14.0_14A309/Charts/v4.1.0/SourcePackages/checkouts/swift-algorithms/Sources/Algorithms/AdjacentPairs.swift:134:10: error: 'let' property 'base' may not be initialized directly; use "self.init(...)" or "self = ..." instead
    self.base = base
         ^
/Users/djs/Library/Caches/org.carthage.CarthageKit/DerivedData/14.0_14A309/Charts/v4.1.0/SourcePackages/checkouts/swift-algorithms/Sources/Algorithms/AdjacentPairs.swift:127:16: note: 'base' declared here
  internal let base: Base
               ^
/Users/djs/Library/Caches/org.carthage.CarthageKit/DerivedData/14.0_14A309/Charts/v4.1.0/SourcePackages/checkouts/swift-algorithms/Sources/Algorithms/AdjacentPairs.swift:135:10: error: 'let' property 'secondBaseIndex' may not be initialized directly; use "self.init(...)" or "self = ..." instead
    self.secondBaseIndex = base.isEmpty

...

Some futzing around has yielded that decorating the culprit Sequences and Collections (e.g. AdjacentPairsSequence , AdjacentPairsCollection , etc) with the @frozen modifier fixes these build errors.

Alas, I concede that reasoning about ABI Stability isn't quite my forte, so I would like to confirm here before doing anything. Does application of the @frozen decorator sound like the Correct™ approach? If so, I'd be happy to follow up with a PR. Otherwise, I'll shift my investigation elsewhere.

yes and no.

with very few exceptions (e.g. structures that need to evolve over time while maintaining binary compatibility) library types should always be @frozen. but swift-algorithms is not my library, so there may be a reason why these types are not @frozen.

cc @nnnnnnnn

at the same time, direct-initialization of struct properties by out-of-module users is usually a bad idea, because the initializer might have special logic to initialize the type’s state. in fact i usually go further and say you shouldn’t do this from outside of the file the type lives in.

so i don’t know why Charts is doing this, it should be calling the appropriate memberwise init, which if the type has frozen layout, should compile to direct initialization anyway.

1 Like

Agreed. Charts should use existing inits on these structs, and if for some reason an appropriate init doesn't exist, they should make a feature request to add it.

It should be noted that Charts is not initializing nor configuring these structs directly. Rather, it is invoking @inlinable convenience methods from Algorithms itself (namely, indexed() and partitioningIndex(where:)), which appears to be the reason for the aforemented build errors when enabling BUILD_LIBRARY_FOR_DISTRIBUTION.

Ah. Well, then I suppose the question is why BUILD_LIBRARY_FOR_DISTRIBUTION is being set for a library that does not claim to be ABI stable.

Carthage-ism, unfortunately. Our SDK has 4 external dependencies, and for our SDK to support library evolution, we tell Carthage to enable BUILD_LIBRARY_FOR_DISTRIBUTION when building said dependencies. However, it seems to apply transitively as well to the libraries our 4 dependencies depend on (so Charts -> Algorithms).

We acknowledge this may require a deeper (and perhaps much-needed) re-think of our SDK build process, but just trying to exhaust all other resources and options first.

BUILD_LIBRARY_FOR_DISTRIBUTION enables two things: it makes it so that the library can be consumed by different versions of the Swift compiler than was used to build it, and it potentially makes it so that the library can be replaced with a newer version without rebuilding downstream consumers. The first is often desirable even when the second is irrelevant (as it basically always is for everyone not shipping an OS), and doesn't require that the library make any ABI stability promises.

An upstream dependency doesn't need to guarantee ABI stability for a downstream library to provide that (it merely means that the two have to be updated together), but both do have to be built in evolution mode.

cc @nnnnnnnn (Sorry to be a pest; this has been a big blocker and we're trying to figure out next steps. :confused:)