Merge pull request #69116 from slavapestov/fix-apply-variadic-generic-args

Sema: Fix two bugs with type resolution of generic arguments

Author: slavapestov

include/swift/AST/DiagnosticsSema.def

@@ -4420,10 +4420,21 @@ ERROR(cannot_specialize_self,none,
       "cannot specialize 'Self'", ())
 NOTE(specialize_explicit_type_instead,none,
      "did you mean to explicitly reference %0 instead?", (Type))
-ERROR(type_parameter_count_mismatch,none,
-      "generic type %0 specialized with %select{too many|too few}3 type "
-      "parameters (got %2, but expected %select{%1|at least %1}4)",
-      (Identifier, unsigned, unsigned, bool, bool))
+ERROR(too_few_generic_arguments,none,
+      "generic type %0 specialized with too few type parameters "
+      "(got %1, but expected %2)",
+      (Identifier, unsigned, unsigned))
+ERROR(too_few_generic_arguments_pack,none,
+      "generic type %0 specialized with too few type parameters "
+      "(got %1, but expected at least %2)",
+      (Identifier, unsigned, unsigned))
+ERROR(too_many_generic_arguments,none,
+      "generic type %0 specialized with too many type parameters "
+      "(got %1, but expected %2)",
+      (Identifier, unsigned, unsigned))
+ERROR(generic_argument_pack_mismatch,none,
+      "generic type %0 specialized with mismatched type parameter pack",
+      (Identifier))
 ERROR(generic_type_requires_arguments,none,
       "reference to generic type %0 requires arguments in <...>", (Type))
 NOTE(descriptive_generic_type_declared_here,none,
@@ -5721,7 +5732,7 @@ ERROR(superclass_with_pack,none,
       "cannot inherit from a generic class that declares a type pack", ())
 ERROR(expansion_not_allowed,none,
       "pack expansion %0 can only appear in a function parameter list, "
-      "tuple element, or generic argument list", (Type))
+      "tuple element, or generic argument of a variadic type", (Type))
 ERROR(expansion_expr_not_allowed,none,
       "value pack expansion can only appear inside a function argument list "
       "or tuple element", ())

lib/Sema/CSDiagnostics.cpp

@@ -17,6 +17,7 @@
 #include "CSDiagnostics.h"
 #include "MiscDiagnostics.h"
 #include "TypeCheckProtocol.h"
+#include "TypeCheckType.h"
 #include "TypoCorrection.h"
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/ASTPrinter.h"
@@ -9313,7 +9314,9 @@ bool OutOfPlaceThenStmtFailure::diagnoseAsError() {
 }
 
 bool InvalidTypeSpecializationArity::diagnoseAsError() {
-  emitDiagnostic(diag::type_parameter_count_mismatch, D->getBaseIdentifier(),
-                 NumParams, NumArgs, NumArgs < NumParams, HasParameterPack);
+  diagnoseInvalidGenericArguments(getLoc(), D,
+                                  NumArgs, NumParams,
+                                  HasParameterPack,
+                                  /*generic=*/nullptr);
   return true;
 }

lib/Sema/TypeCheckType.cpp

@@ -38,6 +38,7 @@
 #include "swift/AST/SourceFile.h"
 #include "swift/AST/TypeCheckRequests.h"
 #include "swift/AST/TypeLoc.h"
+#include "swift/AST/TypeRepr.h"
 #include "swift/AST/TypeResolutionStage.h"
 #include "swift/Basic/SourceManager.h"
 #include "swift/Basic/Statistic.h"
@@ -143,8 +144,8 @@ static unsigned getGenericRequirementKind(TypeResolutionOptions options) {
   case TypeResolverContext::FunctionInput:
   case TypeResolverContext::PackElement:
   case TypeResolverContext::TupleElement:
-  case TypeResolverContext::GenericArgument:
-  case TypeResolverContext::ProtocolGenericArgument:
+  case TypeResolverContext::ScalarGenericArgument:
+  case TypeResolverContext::VariadicGenericArgument:
   case TypeResolverContext::ExtensionBinding:
   case TypeResolverContext::TypeAliasDecl:
   case TypeResolverContext::GenericTypeAliasDecl:
@@ -667,6 +668,51 @@ bool TypeChecker::checkContextualRequirements(GenericTypeDecl *decl,
   llvm_unreachable("invalid requirement check type");
 }
 
+void swift::diagnoseInvalidGenericArguments(SourceLoc loc,
+                                            ValueDecl *decl,
+                                            unsigned argCount,
+                                            unsigned paramCount,
+                                            bool hasParameterPack,
+                                            GenericIdentTypeRepr *generic) {
+  auto &ctx = decl->getASTContext();
+  auto &diags = ctx.Diags;
+
+  if (!hasParameterPack) {
+    // For generic types without type parameter packs, we require
+    // the number of declared generic parameters match the number of
+    // arguments.
+    if (argCount < paramCount) {
+      auto diag = diags
+          .diagnose(loc, diag::too_few_generic_arguments, decl->getBaseIdentifier(),
+                    argCount, paramCount);
+      if (generic)
+        diag.highlight(generic->getAngleBrackets());
+    } else {
+      auto diag = diags
+          .diagnose(loc, diag::too_many_generic_arguments, decl->getBaseIdentifier(),
+                    argCount, paramCount);
+      if (generic)
+        diag.highlight(generic->getAngleBrackets());
+    }
+  } else {
+    if (argCount < paramCount - 1) {
+      auto diag = diags
+          .diagnose(loc, diag::too_few_generic_arguments_pack, decl->getBaseIdentifier(),
+                    argCount, paramCount - 1);
+      if (generic)
+        diag.highlight(generic->getAngleBrackets());
+    } else {
+      auto diag = diags
+          .diagnose(loc, diag::generic_argument_pack_mismatch, decl->getBaseIdentifier());
+      if (generic)
+        diag.highlight(generic->getAngleBrackets());
+    }
+  }
+
+  decl->diagnose(diag::kind_declname_declared_here,
+                 DescriptiveDeclKind::GenericType, decl->getName());
+}
+
 /// Apply generic arguments to the given type.
 ///
 /// If the type is itself not generic, this does nothing.
@@ -747,6 +793,7 @@ static Type applyGenericArguments(Type type, TypeResolution resolution,
 
   auto genericArgs = generic->getGenericArgs();
 
+  // Parameterized protocol types have their own code path.
   if (auto *protoType = type->getAs<ProtocolType>()) {
     auto *protoDecl = protoType->getDecl();
 
@@ -761,6 +808,7 @@ static Type applyGenericArguments(Type type, TypeResolution resolution,
       return ErrorType::get(ctx);
     }
 
+    // Make sure we have the right number of generic arguments.
     if (genericArgs.size() != assocTypes.size()) {
       diags.diagnose(loc,
                      diag::parameterized_protocol_type_argument_count_mismatch,
@@ -775,7 +823,7 @@ static Type applyGenericArguments(Type type, TypeResolution resolution,
     if (options.is(TypeResolverContext::ExistentialConstraint))
       options |= TypeResolutionFlags::DisallowOpaqueTypes;
     auto argOptions = options.withoutContext().withContext(
-        TypeResolverContext::ProtocolGenericArgument);
+        TypeResolverContext::ScalarGenericArgument);
     auto genericResolution = resolution.withOptions(argOptions);
 
     // Resolve the generic arguments.
@@ -791,8 +839,6 @@ static Type applyGenericArguments(Type type, TypeResolution resolution,
     auto parameterized =
         ParameterizedProtocolType::get(ctx, protoType, argTys);
 
-    // Build ParameterizedProtocolType if the protocol has primary associated
-    // types and we're in a supported context.
     if (resolution.getOptions().isConstraintImplicitExistential() &&
         !ctx.LangOpts.hasFeature(Feature::ImplicitSome)) {
       diags.diagnose(loc, diag::existential_requires_any, parameterized,
@@ -827,16 +873,20 @@ static Type applyGenericArguments(Type type, TypeResolution resolution,
   auto *unboundType = type->castTo<UnboundGenericType>();
   auto *decl = unboundType->getDecl();
 
-  // Make sure we have the right number of generic arguments.
   auto genericParams = decl->getGenericParams();
   auto hasParameterPack = llvm::any_of(
       *genericParams, [](auto *paramDecl) {
           return paramDecl->isParameterPack();
       });
 
-  // Resolve the types of the generic arguments.
+  // If the type declares at least one parameter pack, allow pack expansions
+  // anywhere in the argument list. We'll use the PackMatcher to ensure that
+  // everything lines up. Otherwise, don't allow pack expansions to appear
+  // at all.
   auto argOptions = options.withoutContext().withContext(
-      TypeResolverContext::GenericArgument);
+      hasParameterPack
+      ? TypeResolverContext::VariadicGenericArgument
+      : TypeResolverContext::ScalarGenericArgument);
   auto genericResolution = resolution.withOptions(argOptions);
 
   // In SIL mode, Optional<T> interprets T as a SIL type.
@@ -848,6 +898,7 @@ static Type applyGenericArguments(Type type, TypeResolution resolution,
     }
   }
 
+  // Resolve the types of the generic arguments.
   SmallVector<Type, 2> args;
   for (auto tyR : genericArgs) {
     // Propagate failure.
@@ -858,21 +909,16 @@ static Type applyGenericArguments(Type type, TypeResolution resolution,
     args.push_back(substTy);
   }
 
+  // Make sure we have the right number of generic arguments.
   if (!hasParameterPack) {
     // For generic types without type parameter packs, we require
     // the number of declared generic parameters match the number of
     // arguments.
     if (genericArgs.size() != genericParams->size()) {
       if (!options.contains(TypeResolutionFlags::SilenceErrors)) {
-        diags
-            .diagnose(loc, diag::type_parameter_count_mismatch, decl->getName(),
-                      genericParams->size(),
-                      genericArgs.size(),
-                      genericArgs.size() < genericParams->size(),
-                      /*hasParameterPack=*/0)
-            .highlight(generic->getAngleBrackets());
-        decl->diagnose(diag::kind_declname_declared_here,
-                       DescriptiveDeclKind::GenericType, decl->getName());
+        diagnoseInvalidGenericArguments(
+            loc, decl, genericArgs.size(), genericParams->size(),
+            /*hasParameterPack=*/false, generic);
       }
       return ErrorType::get(ctx);
     }
@@ -890,17 +936,11 @@ static Type applyGenericArguments(Type type, TypeResolution resolution,
     }
 
     PackMatcher matcher(params, args, ctx);
-    if (matcher.match()) {
+    if (matcher.match() || matcher.pairs.size() != params.size()) {
       if (!options.contains(TypeResolutionFlags::SilenceErrors)) {
-        diags
-            .diagnose(loc, diag::type_parameter_count_mismatch, decl->getName(),
-                      genericParams->size() - 1,
-                      genericArgs.size(),
-                      genericArgs.size() < genericParams->size(),
-                      /*hasParameterPack=*/1)
-            .highlight(generic->getAngleBrackets());
-        decl->diagnose(diag::kind_declname_declared_here,
-                       DescriptiveDeclKind::GenericType, decl->getName());
+        diagnoseInvalidGenericArguments(
+            loc, decl, genericArgs.size(), genericParams->size(),
+            /*hasParameterPack=*/true, generic);
       }
       return ErrorType::get(ctx);
     }
@@ -936,6 +976,7 @@ static Type applyGenericArguments(Type type, TypeResolution resolution,
     }
   }
 
+  // Construct the substituted type.
   const auto result = resolution.applyUnboundGenericArguments(
       decl, unboundType->getParent(), loc, args);
 
@@ -4433,7 +4474,7 @@ NeverNullType
 TypeResolver::resolveDictionaryType(DictionaryTypeRepr *repr,
                                     TypeResolutionOptions options) {
   auto argOptions = options.withoutContext().withContext(
-      TypeResolverContext::GenericArgument);
+      TypeResolverContext::ScalarGenericArgument);
 
   auto keyTy = resolveType(repr->getKey(), argOptions);
   if (keyTy->hasError()) {
@@ -4507,8 +4548,8 @@ NeverNullType TypeResolver::resolveImplicitlyUnwrappedOptionalType(
     break;
   case TypeResolverContext::PackElement:
   case TypeResolverContext::TupleElement:
-  case TypeResolverContext::GenericArgument:
-  case TypeResolverContext::ProtocolGenericArgument:
+  case TypeResolverContext::ScalarGenericArgument:
+  case TypeResolverContext::VariadicGenericArgument:
   case TypeResolverContext::VariadicFunctionInput:
   case TypeResolverContext::ForEachStmt:
   case TypeResolverContext::ExtensionBinding:

lib/Sema/TypeCheckType.h

@@ -31,6 +31,7 @@ class PackElementTypeRepr;
 class GenericEnvironment;
 class GenericSignature;
 class SILTypeResolutionContext;
+class GenericIdentTypeRepr;
 
 /// Flags that describe the context of type checking a pattern or
 /// type.
@@ -91,11 +92,17 @@ enum class TypeResolverContext : uint8_t {
   /// No special type handling is required.
   None,
 
-  /// Whether we are checking generic arguments of a bound generic type.
-  GenericArgument,
+  /// Whether we are checking generic arguments of a non-variadic bound generic
+  /// type. This includes parameterized protocol types. We don't allow pack
+  /// expansions here.
+  ScalarGenericArgument,
 
-  /// Whether we are checking generic arguments of a parameterized protocol type.
-  ProtocolGenericArgument,
+  /// Whether we are checking generic arguments of a variadic generic type.
+  /// We allow pack expansions in all argument positions, and then use the
+  /// PackMatcher to ensure that scalar parameters line up with scalar
+  /// arguments, and pack expansion parameters line up with pack expansion
+  /// arguments.
+  VariadicGenericArgument,
 
   /// Whether we are checking a tuple element type.
   TupleElement,
@@ -261,8 +268,8 @@ class TypeResolutionOptions {
     case Context::ClosureExpr:
       return true;
     case Context::None:
-    case Context::GenericArgument:
-    case Context::ProtocolGenericArgument:
+    case Context::ScalarGenericArgument:
+    case Context::VariadicGenericArgument:
     case Context::TupleElement:
     case Context::PackElement:
     case Context::FunctionInput:
@@ -307,8 +314,8 @@ class TypeResolutionOptions {
     case Context::MetatypeBase:
       return false;
     case Context::None:
-    case Context::GenericArgument:
-    case Context::ProtocolGenericArgument:
+    case Context::ScalarGenericArgument:
+    case Context::VariadicGenericArgument:
     case Context::PackElement:
     case Context::TupleElement:
     case Context::InExpression:
@@ -341,11 +348,11 @@ class TypeResolutionOptions {
     case Context::VariadicFunctionInput:
     case Context::PackElement:
     case Context::TupleElement:
-    case Context::GenericArgument:
+    case Context::VariadicGenericArgument:
       return true;
-    case Context::PatternBindingDecl:
     case Context::None:
-    case Context::ProtocolGenericArgument:
+    case Context::PatternBindingDecl:
+    case Context::ScalarGenericArgument:
     case Context::Inherited:
     case Context::GenericParameterInherited:
     case Context::AssociatedTypeInherited:
@@ -390,8 +397,8 @@ class TypeResolutionOptions {
     case Context::FunctionInput:
     case Context::PackElement:
     case Context::TupleElement:
-    case Context::GenericArgument:
-    case Context::ProtocolGenericArgument:
+    case Context::ScalarGenericArgument:
+    case Context::VariadicGenericArgument:
     case Context::ExtensionBinding:
     case Context::TypeAliasDecl:
     case Context::GenericTypeAliasDecl:
@@ -655,6 +662,13 @@ class TypeResolution {
                                     ArrayRef<Type> genericArgs) const;
 };
 
+void diagnoseInvalidGenericArguments(SourceLoc loc,
+                                     ValueDecl *decl,
+                                     unsigned argCount,
+                                     unsigned paramCount,
+                                     bool hasParameterPack,
+                                     GenericIdentTypeRepr *generic);
+
 } // end namespace swift
 
 #endif /* SWIFT_SEMA_TYPE_CHECK_TYPE_H */

test/Constraints/pack-expansion-expressions.swift

@@ -36,9 +36,9 @@ func coerceExpansion<each T>(_ value: repeat each T) {
 
 func localValuePack<each T>(_ t: repeat each T) -> (repeat each T, repeat each T) {
   let local = repeat each t
-  // expected-error@-1{{pack expansion 'repeat each T' can only appear in a function parameter list, tuple element, or generic argument list}}
+  // expected-error@-1{{pack expansion 'repeat each T' can only appear in a function parameter list, tuple element, or generic argument of a variadic type}}
   let localAnnotated: repeat each T = repeat each t
-  // expected-error@-1{{pack expansion 'repeat each T' can only appear in a function parameter list, tuple element, or generic argument list}}
+  // expected-error@-1{{pack expansion 'repeat each T' can only appear in a function parameter list, tuple element, or generic argument of a variadic type}}
 
   return (repeat each local, repeat each localAnnotated)
 }
@@ -92,7 +92,7 @@ func sameShapeDiagnostics<each T, each U>(t: repeat each T, u: repeat each U) {
   _ = (repeat (Array<each T>(), each u)) // expected-error {{pack expansion requires that 'each T' and 'each U' have the same shape}}
 }
 
-func returnPackExpansionType<each T>(_ t: repeat each T) -> repeat each T { // expected-error {{pack expansion 'repeat each T' can only appear in a function parameter list, tuple element, or generic argument list}}
+func returnPackExpansionType<each T>(_ t: repeat each T) -> repeat each T { // expected-error {{pack expansion 'repeat each T' can only appear in a function parameter list, tuple element, or generic argument of a variadic type}}
   fatalError()
 }
 

test/Generics/rdar115538574.swift

@@ -5,5 +5,5 @@ protocol P<A> {
 }
 
 func f<each T>(_: some P<repeat each T>) {}
-// expected-error@-1 {{pack expansion 'repeat each T' can only appear in a function parameter list, tuple element, or generic argument list}}
+// expected-error@-1 {{pack expansion 'repeat each T' can only appear in a function parameter list, tuple element, or generic argument of a variadic type}}
 // expected-error@-2 {{generic parameter 'T' is not used in function signature}}