[Sema] Check that at least one buildBlock is callable given arg labels

When validating an @_functionBuilder type, reject types which only provide `buildBlock` declarations with argument labels (which don't have default values). These declarations will never be callable by the `buildBlock` calls produced by the function builder transformation, which does not insert any argument labels for `buildBlock` calls.

Author: Jumhyn

include/swift/AST/DiagnosticsSema.def

@@ -5309,6 +5309,9 @@ NOTE(function_builder_non_static_buildblock, none,
 NOTE(function_builder_buildblock_enum_case, none,
      "enum case 'buildBlock' cannot be used to satisfy the function builder "
      "requirement", ())
+NOTE(function_builder_buildblock_arg_labels, none,
+     "potential match 'buildBlock' can never be called implicitly because it "
+     "has at least one labeled argument", ())
 NOTE(function_builder_buildblock_not_static_method, none,
      "potential match 'buildBlock' is not a static method", ())
 

lib/Sema/BuilderTransform.cpp

@@ -1881,51 +1881,83 @@ std::vector<ReturnStmt *> TypeChecker::findReturnStatements(AnyFunctionRef fn) {
 
 bool TypeChecker::typeSupportsBuilderOp(
     Type builderType, DeclContext *dc, Identifier fnName,
-    ArrayRef<Identifier> argLabels, SmallVectorImpl<ValueDecl *> *allResults) {
+    ArrayRef<Identifier> argLabels, SmallVectorImpl<FuncDecl *> *matches,
+    SmallVectorImpl<BuilderOpMismatch> *nearMisses) {
+
+  auto recordNearMiss = [&](ValueDecl *vd, BuilderOpMismatchKind kind) {
+    if (nearMisses)
+      nearMisses->emplace_back(vd, kind);
+  };
+
   bool foundMatch = false;
   SmallVector<ValueDecl *, 4> foundDecls;
   dc->lookupQualified(
       builderType, DeclNameRef(fnName),
       NL_QualifiedDefault | NL_ProtocolMembers, foundDecls);
   for (auto decl : foundDecls) {
-    if (auto func = dyn_cast<FuncDecl>(decl)) {
-      // Function must be static.
-      if (!func->isStatic())
-        continue;
+    if (isa<EnumElementDecl>(decl)) {
+      recordNearMiss(decl, BuilderOpMismatchKind::EnumCase);
+      continue;
+    } else if (!isa<FuncDecl>(decl)) {
+      recordNearMiss(decl, BuilderOpMismatchKind::Other);
+      continue;
+    }
 
-      // Function must have the right argument labels, if provided.
-      if (!argLabels.empty()) {
-        auto funcLabels = func->getName().getArgumentNames();
-        if (argLabels.size() > funcLabels.size() ||
-            funcLabels.slice(0, argLabels.size()) != argLabels)
-          continue;
+    auto *func = cast<FuncDecl>(decl);
+    // Function must be static.
+    if (!func->isStatic()) {
+      recordNearMiss(func, BuilderOpMismatchKind::InstanceMethod);
+      continue;
+    }
+
+    // Function must have the right argument labels, if provided.
+    if (!argLabels.empty()) {
+      auto funcLabels = func->getName().getArgumentNames();
+      if (argLabels.size() > funcLabels.size() ||
+          funcLabels.slice(0, argLabels.size()) != argLabels) {
+        recordNearMiss(func, BuilderOpMismatchKind::ArgumentLabelMismatch);
+        continue;
       }
+    }
 
-      foundMatch = true;
-      break;
+    // After the labels provided by the builder transform, any additional
+    // labeled parameters must have a default value (or else the function will
+    // be uncallable by the builder transformation).
+    auto params = func->getParameters()->getArray();
+    auto isLabeledUndefaulted = [&](ParamDecl *param) -> bool {
+      return !param->isDefaultArgument() && !param->getArgumentName().empty();
+    };
+    if (llvm::any_of(params.drop_front(argLabels.size()),
+                     isLabeledUndefaulted)) {
+      recordNearMiss(func,
+                     BuilderOpMismatchKind::ExtraneousUndefaultedArgument);
+      continue;
     }
-  }
 
-  if (allResults)
-    allResults->append(foundDecls.begin(), foundDecls.end());
+    foundMatch = true;
+
+    // If we're not recording any candidates or matches, we can bail out as soon
+    // as we find the first match.
+    if (!nearMisses && !matches)
+      break;
+
+    if (matches)
+      matches->push_back(func);
+  }
 
   return foundMatch;
 }
 
 Type swift::inferFunctionBuilderComponentType(NominalTypeDecl *builder) {
   Type componentType;
 
-  SmallVector<ValueDecl *, 4> potentialMatches;
+  SmallVector<FuncDecl *, 4> matches;
   ASTContext &ctx = builder->getASTContext();
   bool supportsBuildBlock = TypeChecker::typeSupportsBuilderOp(
       builder->getDeclaredInterfaceType(), builder, ctx.Id_buildBlock,
-      /*argLabels=*/{}, &potentialMatches);
+      /*argLabels=*/{}, &matches);
   if (supportsBuildBlock) {
-    for (auto decl : potentialMatches) {
-      auto func = dyn_cast<FuncDecl>(decl);
-      if (!func || !func->isStatic())
-        continue;
-
+    for (auto func : matches) {
       // If we haven't seen a component type before, gather it.
       if (!componentType) {
         componentType = func->getResultInterfaceType();

lib/Sema/TypeCheckAttr.cpp

@@ -3022,10 +3022,10 @@ void AttributeChecker::visitPropertyWrapperAttr(PropertyWrapperAttr *attr) {
 
 void AttributeChecker::visitFunctionBuilderAttr(FunctionBuilderAttr *attr) {
   auto *nominal = dyn_cast<NominalTypeDecl>(D);
-  SmallVector<ValueDecl *, 4> potentialMatches;
+  SmallVector<TypeChecker::BuilderOpMismatch, 4> nearMisses;
   bool supportsBuildBlock = TypeChecker::typeSupportsBuilderOp(
       nominal->getDeclaredType(), nominal, D->getASTContext().Id_buildBlock,
-      /*argLabels=*/{}, &potentialMatches);
+      /*argLabels=*/{}, nullptr, &nearMisses);
 
   if (!supportsBuildBlock) {
     {
@@ -3038,7 +3038,7 @@ void AttributeChecker::visitFunctionBuilderAttr(FunctionBuilderAttr *attr) {
       Type componentType;
       std::tie(buildInsertionLoc, stubIndent, componentType) =
           determineFunctionBuilderBuildFixItInfo(nominal);
-      if (buildInsertionLoc.isValid() && potentialMatches.empty()) {
+      if (buildInsertionLoc.isValid() && nearMisses.empty()) {
         std::string fixItString;
         {
           llvm::raw_string_ostream out(fixItString);
@@ -3054,19 +3054,35 @@ void AttributeChecker::visitFunctionBuilderAttr(FunctionBuilderAttr *attr) {
 
     // For any close matches, attempt to explain to the user why they aren't
     // valid.
-    for (auto *member : potentialMatches) {
-      if (member->isStatic() && isa<FuncDecl>(member))
-        continue;
+    for (auto nearMiss : nearMisses) {
+      auto *member = nearMiss.first;
+      auto note = diag::function_builder_buildblock_not_static_method;
+      std::function<void(InFlightDiagnostic &)> fixIt;
+      
+      switch (nearMiss.second) {
+      case TypeChecker::BuilderOpMismatchKind::InstanceMethod:
+        if (member->getDeclContext()->getSelfNominalTypeDecl() == nominal) {
+          note = diag::function_builder_non_static_buildblock;
+          fixIt = [&](InFlightDiagnostic &diag) {
+            diag.fixItInsert(member->getAttributeInsertionLoc(true), "static ");
+          };
+        }
+        break;
+      case TypeChecker::BuilderOpMismatchKind::EnumCase:
+        note = diag::function_builder_buildblock_enum_case;
+        break;
+      case TypeChecker::BuilderOpMismatchKind::ExtraneousUndefaultedArgument:
+        note = diag::function_builder_buildblock_arg_labels;
+        break;
+      case TypeChecker::BuilderOpMismatchKind::Other:
+        break;
+      case TypeChecker::BuilderOpMismatchKind::ArgumentLabelMismatch:
+          llvm_unreachable("buildBlock should not check argument labels");
+      }
 
-      if (isa<FuncDecl>(member) &&
-          member->getDeclContext()->getSelfNominalTypeDecl() == nominal)
-        diagnose(member->getLoc(), diag::function_builder_non_static_buildblock)
-          .fixItInsert(member->getAttributeInsertionLoc(true), "static ");
-      else if (isa<EnumElementDecl>(member))
-        diagnose(member->getLoc(), diag::function_builder_buildblock_enum_case);
-      else
-        diagnose(member->getLoc(),
-                 diag::function_builder_buildblock_not_static_method);
+      auto diag = diagnose(member->getLoc(), note);
+      if (fixIt)
+        fixIt(diag);
     }
   }
 }

lib/Sema/TypeChecker.h

@@ -1222,12 +1222,37 @@ bool requireArrayLiteralIntrinsics(ASTContext &ctx, SourceLoc loc);
 /// an \c UnresolvedMemberExpr, \c nullptr is returned.
 UnresolvedMemberExpr *getUnresolvedMemberChainBase(Expr *expr);
 
+enum class BuilderOpMismatchKind : unsigned {
+    /// The potential match was an instance method rather than a static method.
+    InstanceMethod,
+
+    /// The potential match was an enum case.
+    EnumCase,
+
+    /// The potential match did not have the correct argument labels.
+    ArgumentLabelMismatch,
+
+    /// The potential match had an argument that would not be provided by the
+    /// function builder transformation.
+    ExtraneousUndefaultedArgument,
+
+    /// Catchall for mismatches which could not be classified more specifically.
+    Other
+};
+
+using BuilderOpMismatch = std::pair<ValueDecl *, BuilderOpMismatchKind>;
+
 /// Checks whether a function builder type has a well-formed function builder
-/// method with the given name. If provided and non-empty, the argument labels
-/// are verified against any candidates.
-bool typeSupportsBuilderOp(Type builderType, DeclContext *dc, Identifier fnName,
-                           ArrayRef<Identifier> argLabels = {},
-                           SmallVectorImpl<ValueDecl *> *allResults = nullptr);
+/// method with the given name. If provided and non-empty, the \c argLabels
+/// are verified against any candidates. If provided, \c nearMisses will be
+/// populated with all members which have the correct base name, but were not
+/// considered matches (as well as the reason they were not considered matches,
+/// if known).
+bool typeSupportsBuilderOp(
+    Type builderType, DeclContext *dc, Identifier fnName,
+    ArrayRef<Identifier> argLabels = {},
+    SmallVectorImpl<FuncDecl *> *matches = nullptr,
+    SmallVectorImpl<BuilderOpMismatch> *nearMisses = nullptr);
 }; // namespace TypeChecker
 
 /// Temporary on-stack storage and unescaping for encoded diagnostic

test/decl/var/function_builders.swift

@@ -199,6 +199,11 @@ enum InvalidBuilder11 { // expected-error {{function builder must provide at lea
     case buildBlock(Any) // expected-note {{enum case 'buildBlock' cannot be used to satisfy the function builder requirement}}
 }
 
+@_functionBuilder
+struct InvalidBuilder12 { // expected-error {{function builder must provide at least one static 'buildBlock' method}}
+  static func buildBlock(exprs: Any...) -> Int { return exprs.count } // expected-note {{potential match 'buildBlock' can never be called implicitly because it has at least one labeled argument}}
+}
+
 struct S {
   @ValidBuilder1 var v1: Int { 1 }
   @ValidBuilder2 var v2: Int { 1 }
@@ -216,4 +221,5 @@ struct S {
   @InvalidBuilder9 var i9: Int { 1 }
   @InvalidBuilder10 var i10: Int { 1 }
   @InvalidBuilder11 var i11: InvalidBuilder11 { 1 }
+  @InvalidBuilder12 var i12: Int { 1 } // expected-error {{missing argument label 'exprs:' in call}}
 }