Fix compiler errors for library evolution. #1443

mbrandonw · 2022-10-04T15:43:33Z

While we do not support library evolution in TCA, it can be helpful to build for evolution as long as you know what you're doing (see #1442). Apparently the main branch builds just fine with library evolution on, but there are a few errors to fix in the protocol branch. I had to do a few weird things to make things build, but maybe others have better ideas.

mbrandonw · 2022-10-04T15:43:55Z

.github/workflows/ci.yml

@@ -20,7 +20,7 @@ jobs:
    strategy:
      matrix:
        xcode: ['13.4.1', '14.0']
-        config: ['debug', 'release']
+        config: ['debug', 'release', 'evolution']


let's start building for evolution in CI.

mbrandonw · 2022-10-04T15:44:37Z

Sources/ComposableArchitecture/Reducer/ReducerBuilder.swift

-    @inlinable
+    @usableFromInline


a lot of fixes were done by just changing internal APIs that were marked with @inlinable to be only @usableFromInline. I think that make more sense anyway.

I think we want to annotate with both if possible. Since @usableFromInline calls are not inlined unless they also contain the other annotation. Wanna try adding both and seeing if things still build?

Unfortunately not. Adding both produces a warning in regular builds and the same error as before in library evolution builds.

mbrandonw · 2022-10-04T15:46:34Z

Sources/ComposableArchitecture/Reducer/Reducers/BindingReducer.swift

  @inlinable
-  public init() {}
+  public init() {
+    self.init(internal: ())
+  }
+
+  @usableFromInline
+  init(internal: Void) {}


So I guess with library evolution you can only call out to other inits when marking the init as @inlinable. So, to work around that I need to make an internal, @usableFromInline init, and then call out to that one from the public init. And on top of that, for inits that don't take any arguments I need to do something to disambiguate the initializers, so I passed a void value.

Please someone let me know if there's a better way.

mbrandonw · 2022-10-04T15:47:02Z

Sources/ComposableArchitecture/Reducer/Reducers/CombineReducers.swift

-    self.reducers = build()
+    self.init(internal: build())
+  }
+
+  @usableFromInline
+  init(internal reducers: Reducers) {
+    self.reducers = reducers
  }


same situation here, but since the init takes an argument I can disambiguate by giving it a different external argument name.

flockoffiles · 2022-10-04T16:22:13Z

It builds fine now! You are SUPER awesome! :-)

Sources/ComposableArchitecture/Reducer/Reducers/Reduce.swift

This reverts commit 002a199.

mbrandonw · 2022-10-04T16:35:14Z

There are still some things to figure out because now with the additional internal inits we're getting a bunch of ambiguity errors in tests. We can update all of those call sites to not use trailing closures so that we can name the argument, but I'm hoping there's a better way.

tgrapperon · 2022-10-04T17:54:50Z

Please note that each @usableFromInline annotation makes the property/function part of the public ABI. It is very likely that many of them are not required to be exposed by @inlinable functions, so it would probably be safer to audit them before committing to support library evolution. As they directly depend on the @inlinable surface, this one should probably be checked too.
I think it would probably be safer to defer the official support to v1.0. It doesn't mean that one can't prepare for it though.
The workaround for binary framework providers would be to statically link the TCA version they ship with if possible.

mbrandonw · 2022-10-04T18:11:18Z

@tgrapperon We are not making any promises of binary compatibility with this PR. All we have done is make it possible for one to build with library evolution if they choose to, and they are taking responsibility for making sure to wield it correctly. We still reserve the right to make binary incompatible changes in any future release of TCA.

tgrapperon · 2022-10-04T18:17:37Z

@mbrandonw Ha, OK, I've read the original use-case, which is for private use and not to redistribute the binary. I have no problem with this change then.

mbrandonw · 2022-10-04T18:37:18Z

Tests/ComposableArchitectureTests/StoreTests.swift

@@ -32,14 +32,14 @@ final class StoreTests: XCTestCase {

      enum Action { case start, end }

-      let reducer = Reduce<Void, Action> { _, action in
+      let reducer = Reduce<Void, Action>({ _, action in


All of these changes in StoreTests.swift are just to help swift out with the ambiguity errors. This only happens because of the @testable import. Without that it is not ambiguous.

stephencelis

Nice! Thanks for looking into fixes.

Just a few small thoughts. Also might want to try a pass at some of the more bizarre internal-but-usable-from-inline initializers, since those interfaces will basically be frozen for products built against library evolution binaries.

Sources/ComposableArchitecture/Reducer/Reducers/Observe.swift

…ution

* Add previewValue to DependencyKey, and fixed build errors on Xcode 13. * wip * wip * wip * wip * wip * wip * wip * wip * wip * Add Xcode 14 to CI (#1266) * Add Xcode 14 to CI * Update ci.yml * wip * wip * Update Makefile * Update Makefile * wip * runtime warning when no live dependency available for live app * bring back live * wip * soft deprecate Reducer * fusion test * ternary clean up * wip * add some todos * typealias Reducer inside AnyReducer * Add `now` convenience property to date dependency (#1286) * Remove type alias * LiveDependencyKey: DependencyKey -> DependencyKey: TestDependencyKey (#1288) * LiveDependencyKey: DependencyKey -> DependencyKey: TestDependencyKey * wip * wip * wip * wip * wip * fix * wip * fix navigate case studies * wip * fixes and docs * more migration * wip * wip * thread deps through ifLet and forEach examples. * update * clean up * wip * wip * Update MigratingToReducerProtocols.md * wip * wip * docs * docs * fix some docs * Fix for CombineReducers compiler bug * wip * wip * wip * wip * todos for docs * Don't warn for overridden test dependencies (#1324) * Don't warn while setting test dependencies * wip * fix conflict * wip * Docs and deprecations * add binding info * wip * Add URL session * wip * wip * wip * wip * wip * wip * wip * wip * Don't run debug reducer in tests * wip * updated reducer debugging * Update UUID.swift (#1374) * wip * optional * added Self as default associatedtype value for TestDependencyKey.value (#1395) * added Self as default associatedtype value for TestDependencyKey.value * Update Tests/ComposableArchitectureTests/DependencyKeyTests.swift Co-authored-by: Brandon Williams <[email protected]> * Update example to set badge to the unread count (#1391) * Add store.finish(). * Fix warnings introduced in Xcode 14.1 (#1388) * Fix warnings introduced in Xcode 14.1 * wip * Conform dependency values to DependencyKey where convenient * fix * Fail if testValue is invoked without providing implementation (#1399) * Update example to set badge to the unread count (#1391) * Add store.finish(). * Fix the CaseStudies (UIKit) (#1392) * Fix warnings introduced in Xcode 14.1 (#1388) * Fix warnings introduced in Xcode 14.1 * wip * Fail when accessing testValue when one hasn't been provided. * wip * wip * wip * test Co-authored-by: Mark Adams <[email protected]> Co-authored-by: Maciek Czarnik <[email protected]> Co-authored-by: Stephen Celis <[email protected]> * Add DateGenerator.init(_ generate:) * Start running some tests in release config. (#1405) * Start running some tests in release config. * wip * wip * wip * wip * wip * Update Sources/ComposableArchitecture/Store.swift Co-authored-by: Thomas Grapperon <[email protected]> * wip * wip Co-authored-by: Thomas Grapperon <[email protected]> * added test that dependencies are transferred to effects * wip * self * clean up * simplify * fix * doc fixes * wip * fix * wip * wip * wip * fix * Run dependencies tests on CI. (#1408) * Run dependencies tests on CI. * wip * update makefile * wtf * more docs * more docs * wip * docs * wip * docs * wip * message tweaks * wip * docs and clean up * wip * wip * wip * wip * wip * wip * wip * wip * typo fix * docs clean up * doc fixes * cancel in progress * cancel in progress * no need to make recope release-only, and make more use of XCTAssert to get better release tests * fix test * Remove ActorIsolated from tests where it's not needed * fix test * wip * wip * Add a benchmark for dependency key writing. * fix test * make benchmark 5.7 compatible * iflet docs * more docs * dependency docs * rearrange * @dependency docs * docs * fix benchmark in 5.6 * doc fix * wip * wip * wip * wip * more docs * clean up * fix * wip * Fixes compilation on watchOS (#1423) Co-authored-by: Gunnar Herzog <[email protected]> * move deprecation * format * wip * Fix openURL * wip * wip * wip * wip * fix * Keep simple * Reducer.debug -> Reducer._printChanges We want to revisit some of these debugging APIs in the future, so let's keep them around, but underscore them to allow for more flexible evolution. * `ReducerProtocol.debug` -> `ReducerProtocol._printChanges` (#1426) * Reducer.debug -> Reducer._printChanges We want to revisit some of these debugging APIs in the future, so let's keep them around, but underscore them to allow for more flexible evolution. * Update DebugTests.swift * OpenURL fix for macCatalyst (#1429) * OpenURL fix for macCatalyst Fixes #1428. * Update Makefile * Correct typo in "Designing dependencies" chapter (#1430) protocol AudioPlayerClient should be a struct * fix * fix * protocol docc * docs * Typos (#1439) * remove unneeded internals * wip * Fix missing parameter in code sample (#1450) * typo fix * more docs for DependencyKeyWritingReducer * wip * wip * update readme * wip * update image * main actor * Added withValue, added docs and tests * public properties * cleaned up overload * changed reducer->feature in a bunch of spots of dependencies docs * add articles to readme * 6 * fix tests for swift 5.6 * tweak * tweak * Update ReducerProtocol.swift * wip * wip * docs update * docs for conforming dependency directly to DependencyKey * typo fix * performance article update * fixes * deprecate another effect timer API * link to migration doc in deprecation messages * make some tests that deal with line numbers less fragile * package.swift clean up * tweak to echos * remove docs at root, not in docs-out * remove todo * update WithViewStore.init deprecation message to explain and link to performance doc * Add back ReducerProtocolOf for 5.7.1 (#1444) * Add back ReducerProtocolOf for 5.7.1 * wip * Move scheme tests to matrix * streamline * try this * store.finish * wip * dependencies -> transformDependency * wip * wip * Compat * fix * preserve deps for combine publishers * Remove dump calls (#1460) * Fix compiler errors for library evolution. (#1443) * Fix compiler errors for library evolution. * wip * wip * work around ambiguous init * Revert "work around ambiguous init" This reverts commit 002a199. * fix ambiguity errors * remove _Observe for now * wip * wip * wip * longer timeout * more waiting * wip Co-authored-by: Stephen Celis <[email protected]> Co-authored-by: Ken Grigsby <[email protected]> Co-authored-by: Petr Sima <[email protected]> Co-authored-by: Mark Adams <[email protected]> Co-authored-by: Maciek Czarnik <[email protected]> Co-authored-by: Thomas Grapperon <[email protected]> Co-authored-by: Gunnar Herzog <[email protected]> Co-authored-by: Gunnar Herzog <[email protected]> Co-authored-by: Guttorm Aase <[email protected]> Co-authored-by: Julien Sagot <[email protected]> Co-authored-by: Jaanus Siim <[email protected]>

mbrandonw added 2 commits October 4, 2022 11:40

Fix compiler errors for library evolution.

3ee8238

wip

0281e54

mbrandonw requested a review from stephencelis October 4, 2022 15:43

mbrandonw commented Oct 4, 2022

View reviewed changes

wip

f93b780

work around ambiguous init

002a199

mbrandonw commented Oct 4, 2022

View reviewed changes

Sources/ComposableArchitecture/Reducer/Reducers/Reduce.swift Outdated Show resolved Hide resolved

Revert "work around ambiguous init"

476898c

This reverts commit 002a199.

fix ambiguity errors

8fcfdf3

mbrandonw commented Oct 4, 2022

View reviewed changes

stephencelis reviewed Oct 4, 2022

View reviewed changes

Sources/ComposableArchitecture/Reducer/Reducers/Observe.swift Outdated Show resolved Hide resolved

Sources/ComposableArchitecture/Reducer/Reducers/Observe.swift Outdated Show resolved Hide resolved

remove _Observe for now

9de03cb

stephencelis added the reducer protocol Related to our reducer protocol release. label Oct 6, 2022

mbrandonw mentioned this pull request Oct 6, 2022

Fix missing parameter in code sample #1449

Closed

mbrandonw added 6 commits October 10, 2022 09:09

Merge remote-tracking branch 'origin/protocol-beta' into library-evol…

172dc9c

…ution

wip

7991b9b

wip

fd57560

wip

14e9d7f

longer timeout

2f2024a

more waiting

47a6793

mbrandonw merged commit 85c74ff into protocol-beta Oct 10, 2022

mbrandonw deleted the library-evolution branch October 10, 2022 13:57

abel3cl mentioned this pull request Feb 11, 2023

Library evolution pointfreeco/swift-parsing#285

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix compiler errors for library evolution. #1443

Fix compiler errors for library evolution. #1443

Uh oh!

mbrandonw commented Oct 4, 2022

Uh oh!

mbrandonw Oct 4, 2022

Uh oh!

mbrandonw Oct 4, 2022 •

edited

Loading

Uh oh!

stephencelis Oct 4, 2022 •

edited

Loading

Uh oh!

mbrandonw Oct 4, 2022

Uh oh!

mbrandonw Oct 4, 2022

Uh oh!

mbrandonw Oct 4, 2022

Uh oh!

flockoffiles commented Oct 4, 2022

Uh oh!

Uh oh!

mbrandonw commented Oct 4, 2022

Uh oh!

tgrapperon commented Oct 4, 2022 •

edited

Loading

Uh oh!

mbrandonw commented Oct 4, 2022

Uh oh!

tgrapperon commented Oct 4, 2022

Uh oh!

mbrandonw Oct 4, 2022

Uh oh!

stephencelis left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Fix compiler errors for library evolution. #1443

Fix compiler errors for library evolution. #1443

Uh oh!

Conversation

mbrandonw commented Oct 4, 2022

Uh oh!

mbrandonw Oct 4, 2022

Choose a reason for hiding this comment

Uh oh!

mbrandonw Oct 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stephencelis Oct 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbrandonw Oct 4, 2022

Choose a reason for hiding this comment

Uh oh!

mbrandonw Oct 4, 2022

Choose a reason for hiding this comment

Uh oh!

mbrandonw Oct 4, 2022

Choose a reason for hiding this comment

Uh oh!

flockoffiles commented Oct 4, 2022

Uh oh!

Uh oh!

mbrandonw commented Oct 4, 2022

Uh oh!

tgrapperon commented Oct 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbrandonw commented Oct 4, 2022

Uh oh!

tgrapperon commented Oct 4, 2022

Uh oh!

mbrandonw Oct 4, 2022

Choose a reason for hiding this comment

Uh oh!

stephencelis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mbrandonw Oct 4, 2022 •

edited

Loading

stephencelis Oct 4, 2022 •

edited

Loading

tgrapperon commented Oct 4, 2022 •

edited

Loading