Skip to content
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.

Sign up for GitHub

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

Use opaque reducers in 5.7 to fix builder inference #1591

Merged
merged 6 commits into from
Oct 28, 2022
Merged

Use opaque reducers in 5.7 to fix builder inference #1591

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4b8eab5
Use opaque reducers in 5.7 to fix builder inference
stephencelis Oct 28, 2022
3ec3dbf
wip
stephencelis Oct 28, 2022
f51e569
wip
stephencelis Oct 28, 2022
d1a269e
Revert "wip"
stephencelis Oct 28, 2022
0b7bf46
wip
stephencelis Oct 28, 2022
9d2f359
Fix availability; simplify optionality
stephencelis Oct 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
347 changes: 203 additions & 144 deletions Sources/ComposableArchitecture/Reducer/ReducerBuilder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,175 +7,101 @@
/// See ``CombineReducers`` for an entry point into a reducer builder context.
@resultBuilder
public enum ReducerBuilder<State, Action> {
@inlinable
public static func buildArray<R: ReducerProtocol>(_ reducers: [R]) -> _SequenceMany<R>
where R.State == State, R.Action == Action {
_SequenceMany(reducers: reducers)
}

@inlinable
public static func buildBlock() -> EmptyReducer<State, Action> {
EmptyReducer()
}

@inlinable
public static func buildBlock<R: ReducerProtocol>(_ reducer: R) -> R
where R.State == State, R.Action == Action {
reducer
}

@inlinable
public static func buildEither<R0: ReducerProtocol, R1: ReducerProtocol>(
first reducer: R0
) -> _Conditional<R0, R1>
where R0.State == State, R0.Action == Action {
.first(reducer)
}

@inlinable
public static func buildEither<R0: ReducerProtocol, R1: ReducerProtocol>(
second reducer: R1
) -> _Conditional<R0, R1>
where R1.State == State, R1.Action == Action {
.second(reducer)
}

@inlinable
public static func buildExpression<R: ReducerProtocol>(_ expression: R) -> R
where R.State == State, R.Action == Action {
expression
}

@inlinable
public static func buildFinalResult<R: ReducerProtocol>(_ reducer: R) -> R
where R.State == State, R.Action == Action {
reducer
}

#if swift(<5.7)
@_disfavoredOverload
#if swift(>=5.7)
@inlinable
public static func buildFinalResult<R: ReducerProtocol>(_ reducer: R) -> Reduce<State, Action>
where R.State == State, R.Action == Action {
Reduce(reducer)
public static func buildArray(
_ reducers: [some ReducerProtocol<State, Action>]
) -> some ReducerProtocol<State, Action> {
_SequenceMany(reducers: reducers)
}
#endif

@inlinable
public static func buildLimitedAvailability<R: ReducerProtocol>(
_ wrapped: R
) -> _Optional<R>
where R.State == State, R.Action == Action {
_Optional(wrapped: wrapped)
}

@inlinable
public static func buildOptional<R: ReducerProtocol>(_ wrapped: R?) -> _Optional<R>
where R.State == State, R.Action == Action {
_Optional(wrapped: wrapped)
}

@inlinable
public static func buildPartialBlock<R: ReducerProtocol>(first: R) -> R
where R.State == State, R.Action == Action {
first
}

@inlinable
public static func buildPartialBlock<R0: ReducerProtocol, R1: ReducerProtocol>(
accumulated: R0, next: R1
) -> _Sequence<R0, R1>
where R0.State == State, R0.Action == Action {
_Sequence(accumulated, next)
}

public enum _Conditional<First: ReducerProtocol, Second: ReducerProtocol>: ReducerProtocol
where
First.State == Second.State,
First.Action == Second.Action
{
case first(First)
case second(Second)

@inlinable
public func reduce(into state: inout First.State, action: First.Action) -> EffectTask<
First.Action
> {
switch self {
case let .first(first):
return first.reduce(into: &state, action: action)

case let .second(second):
return second.reduce(into: &state, action: action)
}
public static func buildBlock() -> some ReducerProtocol<State, Action> {
EmptyReducer()
}
}

public struct _Optional<Wrapped: ReducerProtocol>: ReducerProtocol {
@usableFromInline
let wrapped: Wrapped?
@inlinable
public static func buildBlock(
_ reducer: some ReducerProtocol<State, Action>
) -> some ReducerProtocol<State, Action> {
reducer
}

@usableFromInline
init(wrapped: Wrapped?) {
self.wrapped = wrapped
@inlinable
public static func buildEither<R0: ReducerProtocol, R1: ReducerProtocol>(
first reducer: R0
) -> _Conditional<R0, R1>
where R0.State == State, R0.Action == Action, R1.State == State, R1.Action == Action {
.first(reducer)
}

@inlinable
public func reduce(
into state: inout Wrapped.State, action: Wrapped.Action
) -> EffectTask<Wrapped.Action> {
switch wrapped {
case let .some(wrapped):
return wrapped.reduce(into: &state, action: action)
case .none:
return .none
}
public static func buildEither<R0: ReducerProtocol, R1: ReducerProtocol>(
second reducer: R1
) -> _Conditional<R0, R1>
where R0.State == State, R0.Action == Action, R1.State == State, R1.Action == Action {
Comment on lines +39 to +42
Copy link
Member Author

Choose a reason for hiding this comment

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

buildEither was the one spot I couldn't get some ReducerProtocol to work...so there may be some inference issues when it comes to if-else...but that's rare and I guess we can revisit if/when it comes up.

Copy link
Contributor

Choose a reason for hiding this comment

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

@stephencelis were you using _Conditional<R0, R1>.first(reducer)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was trying to, but couldn't figure out how to get R0 to be a some and R1 to be a generic that propagates.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I get you.
The only way I can think of at the moment would be to use custom inits. eg: _Conditional(first: reducer, secondType: R1.self) but not sure if that's any better than just passing the two generics.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fun idea! Seems to work and I imagine is better on the type checker than the alternative, so pushed, thanks! 😄

.second(reducer)
}
}

public struct _Sequence<R0: ReducerProtocol, R1: ReducerProtocol>: ReducerProtocol
where R0.State == R1.State, R0.Action == R1.Action {
@usableFromInline
let r0: R0
@inlinable
public static func buildExpression(
_ expression: some ReducerProtocol<State, Action>
) -> some ReducerProtocol<State, Action> {
expression
}

@usableFromInline
let r1: R1
@inlinable
public static func buildFinalResult(
_ reducer: some ReducerProtocol<State, Action>
) -> some ReducerProtocol<State, Action> {
reducer
}

@usableFromInline
init(_ r0: R0, _ r1: R1) {
self.r0 = r0
self.r1 = r1
@inlinable
public static func buildLimitedAvailability(
_ wrapped: some ReducerProtocol<State, Action>
) -> some ReducerProtocol<State, Action> {
_Optional(wrapped: wrapped)
}

@inlinable
public func reduce(into state: inout R0.State, action: R0.Action) -> EffectTask<R0.Action> {
self.r0.reduce(into: &state, action: action)
.merge(with: self.r1.reduce(into: &state, action: action))
public static func buildOptional(
_ wrapped: (some ReducerProtocol<State, Action>)?
) -> some ReducerProtocol<State, Action> {
_Optional(wrapped: wrapped)
}
}

public struct _SequenceMany<Element: ReducerProtocol>: ReducerProtocol {
@usableFromInline
let reducers: [Element]
@inlinable
public static func buildPartialBlock(
first: some ReducerProtocol<State, Action>
) -> some ReducerProtocol<State, Action> {
first
}

@usableFromInline
init(reducers: [Element]) {
self.reducers = reducers
@inlinable
public static func buildPartialBlock(
accumulated: some ReducerProtocol<State, Action>, next: some ReducerProtocol<State, Action>
) -> some ReducerProtocol<State, Action> {
_Sequence(accumulated, next)
}
#else
@inlinable
public static func buildArray<R: ReducerProtocol>(_ reducers: [R]) -> _SequenceMany<R>
where R.State == State, R.Action == Action {
_SequenceMany(reducers: reducers)
}

@inlinable
public func reduce(
into state: inout Element.State, action: Element.Action
) -> EffectTask<Element.Action> {
self.reducers.reduce(.none) { $0.merge(with: $1.reduce(into: &state, action: action)) }
public static func buildBlock() -> EmptyReducer<State, Action> {
EmptyReducer()
}
}
}

public typealias ReducerBuilderOf<R: ReducerProtocol> = ReducerBuilder<R.State, R.Action>
@inlinable
public static func buildBlock<R: ReducerProtocol>(_ reducer: R) -> R
where R.State == State, R.Action == Action {
reducer
}

#if swift(<5.7)
extension ReducerBuilder {
@inlinable
public static func buildBlock<
R0: ReducerProtocol,
Expand Down Expand Up @@ -403,5 +329,138 @@ public typealias ReducerBuilderOf<R: ReducerProtocol> = ReducerBuilder<R.State,
r9
)
}

@inlinable
public static func buildEither<R0: ReducerProtocol, R1: ReducerProtocol>(
first reducer: R0
) -> _Conditional<R0, R1>
where R0.State == State, R0.Action == Action {
.first(reducer)
}

@inlinable
public static func buildEither<R0: ReducerProtocol, R1: ReducerProtocol>(
second reducer: R1
) -> _Conditional<R0, R1>
where R1.State == State, R1.Action == Action {
.second(reducer)
}

@inlinable
public static func buildExpression<R: ReducerProtocol>(_ expression: R) -> R
where R.State == State, R.Action == Action {
expression
}

@inlinable
public static func buildFinalResult<R: ReducerProtocol>(_ reducer: R) -> R
where R.State == State, R.Action == Action {
reducer
}

@_disfavoredOverload
@inlinable
public static func buildFinalResult<R: ReducerProtocol>(_ reducer: R) -> Reduce<State, Action>
where R.State == State, R.Action == Action {
Reduce(reducer)
}

@inlinable
public static func buildLimitedAvailability<R: ReducerProtocol>(
_ wrapped: R
) -> _Optional<R>
where R.State == State, R.Action == Action {
_Optional(wrapped: wrapped)
}

@inlinable
public static func buildOptional<R: ReducerProtocol>(_ wrapped: R?) -> _Optional<R>
Copy link
Contributor

@tgrapperon tgrapperon Oct 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

SwiftUI makes Optional<some View> a View itself. Was making Optional<some ReducerProtocol> a ReducerProtocol instead of using this _Optional<some Reducer> considered at some point?

EDIT: I made the comment in the 5.6 branch, but it's only because diffing is all messed up. My remark applies to both branches.

Copy link
Contributor

@tgrapperon tgrapperon Oct 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

For example, with

extension Optional: ReducerProtocol where Wrapped: ReducerProtocol {
  public func reduce(into state: inout Wrapped.State, action: Wrapped.Action)
    -> EffectTask<Wrapped.Action>
  {
    switch self {
    case .none:
      return .none
    case .some(let wrapped):
      return wrapped.reduce(into: &state, action: action)
    }
  }
}

We can return directly wrapped in buildOptional, and Optional.some(wrapped) in buildLimitedAvailability, while using opaque reducers.

BTW, I've just spotted this thing while checking in SwiftUI interface:

@available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *)
@inlinable @inline(__always) internal func asOptional<T>(_ value: T) -> T? {
    func unwrap<T>() -> T { value as! T }
    let optionalValue: T? = unwrap()
    return optionalValue
}

This is some weird gymnastic and I'm not getting why they don't use Optional.some instead. But this is mostly trivia and unrelated to the PR.

EDIT: I think this is to make sure they don't create T??, etc. So we can probably use this too for reducers.

EDIT2: Well, it still creates T?? as I'm getting it, so I don't know. I'm not 100% on nested optionals, as I think there is some automatic coalescing in some places.

Copy link
Member Author

Choose a reason for hiding this comment

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

@tgrapperon Good find! I'm not sure why we didn't conform optional directly, maybe because of a bug with type aliases in pre-5.7. Added support and removed _Optional, thanks!

We also realized our buildLimitedAvailability wasn't right. This function needs to erase the reducer somehow. I think we snuck a fix into this PR, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we still need the buildOptional block? As I'm seeing it, it should be handled by the buildBlock(reducer: R?) now that R? is a reducer, isn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind, it is required to work with buildLimitedAvailability at least.

Copy link
Member Author

Choose a reason for hiding this comment

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

@tgrapperon Just merged and missed this. What hit main should at least be an improvement, but if you find other tweaks that can be made def PR!

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I think we need this. As I'm getting it now:

if #available(iOS 16, *) {
  R()
}

will buildLimitedAvailability R into AnyReducer and then buildOptional it to produce AnyReducer?. If you remove buildOptional, the test for buildLimitedAvailability doesn't pass anymore, so I guess this is what happens (and it is maybe involved with other blocks/situations).

Copy link
Contributor

Choose a reason for hiding this comment

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

@stephencelis buildOptional is involved as soon as we have unbalanced if checks of some sort, so it is required for sure.

where R.State == State, R.Action == Action {
_Optional(wrapped: wrapped)
}
#endif

public enum _Conditional<First: ReducerProtocol, Second: ReducerProtocol>: ReducerProtocol
where
First.State == Second.State,
First.Action == Second.Action
{
case first(First)
case second(Second)

@inlinable
public func reduce(into state: inout First.State, action: First.Action) -> EffectTask<
First.Action
> {
switch self {
case let .first(first):
return first.reduce(into: &state, action: action)

case let .second(second):
return second.reduce(into: &state, action: action)
}
}
}

public struct _Optional<Wrapped: ReducerProtocol>: ReducerProtocol {
@usableFromInline
let wrapped: Wrapped?

@usableFromInline
init(wrapped: Wrapped?) {
self.wrapped = wrapped
}

@inlinable
public func reduce(
into state: inout Wrapped.State, action: Wrapped.Action
) -> EffectTask<Wrapped.Action> {
switch wrapped {
case let .some(wrapped):
return wrapped.reduce(into: &state, action: action)
case .none:
return .none
}
}
}
#endif

public struct _Sequence<R0: ReducerProtocol, R1: ReducerProtocol>: ReducerProtocol
where R0.State == R1.State, R0.Action == R1.Action {
@usableFromInline
let r0: R0

@usableFromInline
let r1: R1

@usableFromInline
init(_ r0: R0, _ r1: R1) {
self.r0 = r0
self.r1 = r1
}

@inlinable
public func reduce(into state: inout R0.State, action: R0.Action) -> EffectTask<R0.Action> {
self.r0.reduce(into: &state, action: action)
.merge(with: self.r1.reduce(into: &state, action: action))
}
}

public struct _SequenceMany<Element: ReducerProtocol>: ReducerProtocol {
@usableFromInline
let reducers: [Element]

@usableFromInline
init(reducers: [Element]) {
self.reducers = reducers
}

@inlinable
public func reduce(
into state: inout Element.State, action: Element.Action
) -> EffectTask<Element.Action> {
self.reducers.reduce(.none) { $0.merge(with: $1.reduce(into: &state, action: action)) }
}
}
}

public typealias ReducerBuilderOf<R: ReducerProtocol> = ReducerBuilder<R.State, R.Action>
Loading