[Sema] Require function builders to have at least one buildBlock method

[Jumhyn]

If the supplied method is not static, offer a fix-it to make it static

This is technically source breaking, though AFAICT any function builders which don't have at least one valid buildBlock member aren't usable anyway, so maybe okay? If necessary, could downgrade this to a warning.

[Jumhyn]

@hborla (or anyone else) would you mind requesting the relevant reviewers?

[xedin]

I wonder whether it should be allowed to declare a static method in an extension instead of requiring it to be always attached to a type declaration, that might be helpful for file management. After all the requirement is that it buildBlock should exist is vague enough...

[Jumhyn]

Hm, @xedin could you clarify? I think this check already allows for that (though I just added a test to that effect, see ValidBuilder7)—if that's not what you were thinking of, please let me know. IMHO we shouldn't allow:

// Module A
@_functionBuilder
public struct Builder {}

// Module B
extension Builder {
  func buildBlock(_: Any...) -> Int { ... }
}

[Jumhyn]

Although, I guess with the support for callable types serving as build* functions, someone who really wanted to could do:

// Module A
@_functionBuilder
public struct Builder {
  public struct BuildBlock {}
  static var buildBlock = BuildBlock()
}

// Module B
extension Builder.BuildBlock {
  func callAsFunction(_ exprs: Any...) -> Int { ... }
}

[xedin]

@Jumhyn The way you made the check makes sense to me, I just wanted to point out that its should be allowed to declare buildBlock in extension in the same module (other module doesn't make much sense to me).

[Jumhyn]

Gotcha, thank you for clarifying @xedin!

I've brought back the check that validates the type of the buildBlock declaration, so that now we check:

    if (!(ty->is<AnyFunctionType>() || ty->isCallableNominalType(nominal)))
      continue;

This will reject the case I posted above where a non-callable buildBlock member's type is extended in another module to add a callAsFunction method. Are there other cases that should be allowed that this would reject?

I've also made a small tweak to allow the somewhat-pathological but still syntactically valid:

@_functionBuilder
enum ValidBuilder10 {
    case buildBlock(Any)
}

which can be used as

@ValidBuilder10 var v10: ValidBuilder10 { 1 }

[Jumhyn]

@DougGregor Thank you for the suggestion! I've updated this to share the logic between builderSupports and visitFunctionBuilderAttr at the cost of disallowing case buildBlock(Any), static var buildBlock: (Any...) -> Int and static var buildBlock: SomeCallableNominal type declarations. Let me know if you have any thoughts about whether those constructions should be allowed!

[DougGregor]

Much improved, thank you!

[DougGregor]

@swift-ci please smoke test

[DougGregor]

@swift-ci please smoke test

[DougGregor]

@swift-ci please smoke test

[DougGregor]

@swift-ci please smoke test macOS

[theblixguy]

Unexpectedly Passed Tests (1):
  lldb-api :: lang/swift/mix_any_object/TestSwiftMixAnyObjectType.py

Looks like we need to un-XFAIL it.

[DougGregor]

I'll just merge, thank you!