swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.3k stars 10.34k forks source link

@resultBuilder if-else statement -> failed to produce diagnostic for expression #67791

Open stevenkellner opened 1 year ago

stevenkellner commented 1 year ago

Description The following is taken from a larger piece of code and condensed to show a compiler problem with a custom result builder and an if-else statement. This bug only occurred with this custom result builder and not with e.g. the ViewBuilder result builder.

A single if statement compiles successfully, but with an else block it produced this error: Failed to produce diagnostic for expression; please submit a bug report (https://swift.org/contributing/#reporting-bugs).

@ModifierBuilder private var rootModifiers: some ViewModifier {
    if self.myCondition {
        // some ViewModifier
    } else {
        // fallback ViewModifier
    }
}

The ModifierBuilder is just a result builder to build view modifiers. It implements both buildEither methods so it should be no problem to write an if-else statement.

@resultBuilder
struct ModifierBuilder {    
    static func buildPartialBlock(first: some ViewModifier) -> some ViewModifier {
        return first
    }

    static func buildPartialBlock(accumulated: some ViewModifier, next: some ViewModifier) -> some ViewModifier {
        return PairModifier(first: accumulated, second: next)
    }

    static func buildEither(first modifier: some ViewModifier) -> some ViewModifier {
        return ConditionalModifier(alwaysTrue: modifier)
    }

    static func buildEither(second modifier: some ViewModifier) -> some ViewModifier {
        return ConditionalModifier(alwaysFalse: modifier)
    }

    static func buildOptional(_ modifier: (some ViewModifier)?) -> some ViewModifier {
        return OptionalModifier(modifier: modifier)
    }

    static func buildLimitedAvailability(_ modifier: some ViewModifier) -> some ViewModifier {
        return LimitedAvailabilityModifier(modifier: modifier)
    }
}

Steps to reproduce

Compiling the following code in playground will generate the error Failed to produce diagnostic for expression; please submit a bug report (https://swift.org/contributing/#reporting-bugs).

Show code to reproduce ```swift import SwiftUI @resultBuilder struct ModifierBuilder { static func buildPartialBlock(first: some ViewModifier) -> some ViewModifier { return first } static func buildPartialBlock(accumulated: some ViewModifier, next: some ViewModifier) -> some ViewModifier { return PairModifier(first: accumulated, second: next) } static func buildEither(first modifier: some ViewModifier) -> some ViewModifier { return ConditionalModifier(alwaysTrue: modifier) } static func buildEither(second modifier: some ViewModifier) -> some ViewModifier { return ConditionalModifier(alwaysFalse: modifier) } static func buildOptional(_ modifier: (some ViewModifier)?) -> some ViewModifier { return OptionalModifier(modifier: modifier) } static func buildLimitedAvailability(_ modifier: some ViewModifier) -> some ViewModifier { return LimitedAvailabilityModifier(modifier: modifier) } } struct PairModifier: ViewModifier where FirstModifier: ViewModifier, SecondModifier: ViewModifier { private let firstModifier: FirstModifier private let secondModifier: SecondModifier init(first firstModifier: FirstModifier, second secondModifier: SecondModifier) { self.firstModifier = firstModifier self.secondModifier = secondModifier } func body(content: Content) -> some View { content .modifier(self.firstModifier) .modifier(self.secondModifier) } } struct ConditionalModifier: ViewModifier where TrueModifier: ViewModifier, FalseModifier: ViewModifier { private let condition: Bool private let trueModifier: TrueModifier private let falseModifier: FalseModifier init(condition: Bool, trueModifier: TrueModifier, falseModifier: FalseModifier) { self.condition = condition self.trueModifier = trueModifier self.falseModifier = falseModifier } func body(content: Content) -> some View { if self.condition { content .modifier(self.trueModifier) } else { content .modifier(self.falseModifier) } } } extension ConditionalModifier where FalseModifier == EmptyModifier { init(alwaysTrue modifier: TrueModifier) { self.condition = true self.trueModifier = modifier self.falseModifier = EmptyModifier() } } extension ConditionalModifier where TrueModifier == EmptyModifier { init(alwaysFalse modifier: FalseModifier) { self.condition = true self.trueModifier = EmptyModifier() self.falseModifier = modifier } } struct OptionalModifier: ViewModifier where Modifier: ViewModifier { private let modifier: Modifier? init(modifier: Modifier?) { self.modifier = modifier } func body(content: Content) -> some View { if let modifier = self.modifier { content .modifier(modifier) } else { content } } } struct LimitedAvailabilityModifier: ViewModifier where Modifier: ViewModifier { private let modifier: Modifier init(modifier: Modifier) { self.modifier = modifier } func body(content: Content) -> some View { content .modifier(self.modifier) } } struct MyViewModifier: ViewModifier { func body(content: Content) -> some View { content } } struct MyOtherViewModifier: ViewModifier { func body(content: Content) -> some View { content } } struct MyView: View { let myCondition: Bool var body: some View { Text("Hello, World!") .modifier(self.rootModifiers) } @ModifierBuilder private var rootModifiers: some ViewModifier { if self.myCondition { MyViewModifier() } else { MyOtherViewModifier() } } } ```

The error will show at the line @ModifierBuilder private var rootModifiers: some ViewModifier {

Expected behavior

The if-else statement compiles successfully without an error.

Environment

flockoffiles commented 8 months ago

I experience the same problem with my custom result builder. Is there any way to get a meaningful diagnostic from the compiler?

gregomni commented 2 months ago

The issue here is that buildEither(first:) and buildEither(second:) return different types, and rootModifiers must return a single consistent type (some ViewModifier is opaque but has to be a particular ViewModifier).

Replacing the opaque return types in the result builder methods themselves:

    static func buildPartialBlock<T: ViewModifier>(first: T) -> T {
        return first
    }

    static func buildEither<T: ViewModifier>(first modifier: T) -> ConditionalModifier<T,EmptyModifier> {
        return ConditionalModifier(alwaysTrue: modifier)
    }

    static func buildEither<T: ViewModifier>(second modifier: T) -> ConditionalModifier<EmptyModifier,T> {
        return ConditionalModifier(alwaysFalse: modifier)
    }

... it becomes more clear that the issue is that ConditionalModifier<MyViewModifier,EmptyModifier> != ConditionalModifier<EmptyModifier,MyOtherViewModifier>. If you make this change, the reported errors are still poor, but at least they exist:

rb.swift:123:24: error: cannot convert value of type 'MyViewModifier' to expected argument type 'EmptyModifier'
        if myCondition {
                       ^
rb.swift:125:16: error: cannot convert value of type 'MyOtherViewModifier' to expected argument type 'EmptyModifier'
        } else {
               ^

These errors are reported in the implicit calls to buildPartialBlock and are reported this way because using EmptyModifier() to make both the then and else results into ConditionalModifier<EmptyModifier,EmptyModifier> is the only way to make the constraints work out.

The easy (but possibly wrong?) way to fix this is to check in ResultBuilderTransform::isBuildableIfChain that the buildEither(first:) and buildEither(second:) declarations that we lookup match each others return types. That could be a very focused diagnosis. But are there possible (useful) result builder implementations where these return types structurally differ but match up in practice? (Like the trivial ConditionalModifier<EmptyModifier,EmptyModifier> in this example.) If so, does that mean we're closing off useful possibilities?

The more complicated but maybe more correct fix is in two parts: (1) With the modified (removed opaque types) ModifierBuilder, we get a solution with fixes and could examine the constraint system's overload choices after solving has failed in TypeChecker:applyResultBuilderBodyTransform and check the actual resolved types for the choices made for buildEither(first:) and buildEither(second:) and diagnose there. (2) But also, in the original code with opaque return types, there are no fixes and thus no solution and no overload choices left in the constraint system. The solution is probably a new kind of Fix where we allow two similar looking opaque return types to match each other and additional diagnoses for that.

@xedin What do you think?

gregomni commented 2 months ago

As I'm thinking about it more, it doesn't feel like restricting the result types to be structurally identical is a big problem. That seems to be the intention of the design, and there is flexibility with buildExpression and buildFinalResult to convert from and to other intermediate types.