swiftlang / swift

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

[SR-15286] Expression type checks as standalone but is ambiguous inside result builder #57608

Open ahoppen opened 2 years ago

ahoppen commented 2 years ago
Previous ID SR-15286
Radar rdar://83929254
Original Reporter @ahoppen
Type Bug
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: f7a7fb45fba6ec59908fe7403ab7acf2

Issue Description:

When typechecking the following, the first call of TokenWrapper(token: .test) type checks fine, but the call inside the result builder later on fails with an error message Type of expression is ambiguous without more context.

protocol ExpressibleAsTokenSyntax {}

struct TokenSyntax: ExpressibleAsTokenSyntax {}

extension ExpressibleAsTokenSyntax where Self == TokenSyntax {
  static var test: TokenSyntax { TokenSyntax() }
}

struct TokenWrapper {
  init(token: ExpressibleAsTokenSyntax) {}
}

@resultBuilder
struct MyBuilder {
  public static func buildBlock(_ components: TokenWrapper...) -> TokenWrapper {
    return components.first!
  }
}

func build(@MyBuilder builder: () -> TokenWrapper) {}

// This compiles
TokenWrapper(token: .test)

// But this fails: Type of expression is ambiguous without more context
build {
  TokenWrapper(token: .test)
}

Happens in both Swift 5.5 and swift-DEVELOPMENT-SNAPSHOT-2021-10-05-a.

xedin commented 2 years ago

@ahoppen Just FYI - static member lookup is not supported to work with existential types like that, there is a bug in `repairFailures` that allows first case to slip by which I need to fix. Initializer for TokenWrapper should be declared as `init\<T: ExpressibleAsTokenSyntax>(token: T) {}` and both examples would work just fine.

ahoppen commented 2 years ago

Oh, that’s good to know. Unfortunately, your suggestion with generic parameters doesn’t work in the presence of defaulted arguments.

struct TokenWrapper {
  init<T: ExpressibleAsTokenSyntax>(token: T = TokenSyntax.test) {} // Default argument value of type 'TokenSyntax' cannot be converted to type 'T'
}

which makes sense because we could specialize TokenWrapper with T != TokenSyntax and thus the default argument doesn’t produce a value of type T.

ahoppen commented 2 years ago

@kimdv it looks like Swift doesn’t allow us to support both leading dot syntax and default arguments for ExpressibleByTokenSyntax. Maybe we should consider not having an ExpressibleByTokenSyntax protocol at all and just use TokenSyntax where we currently use ExpressibleByTokenSyntax instead. I don’t see any issues with it at the moment because we don’t conform any other syntax types to ExpressibleByTokenSyntax.

kimdv commented 2 years ago

It sounds like a fair trade of.
We can always implement it again if needed.

kimdv commented 2 years ago

I have been digging into it, and we merged (after your latest comment here) protocols that conform to `ExpressibleByTokenSyntax`.

So I don't this we can delete it anymore.
But in my opinion it's not the that bad, even though it's a bit more clean with just dot syntax.

It will add some convenient methods and possibilities that is more valuable (in my eyes) than the dot syntax.
https://github.com/apple/swift-syntax/blob/e73e11b0cac2a4f38668d87f0a96a9f32f38558e/Sources/SwiftSyntaxBuilder/gyb_generated/Buildables.swift#L11331-L11347

ahoppen commented 2 years ago

@kimdv My point was that no types other than `TokenSyntax` conform to `ExpressibleAsTokenSyntax`, so IMO we should be able to replace all occurrences of `ExpressibleAsTokenSytnax` by `TokenSyntax`. This might be a little challenging code-generation wise, but I don’t see any other issues with it.