Sema: Fix crash-on-invalid with incorrect use of variadic generic type

slavapestov · slavapestov · commit 67308e689409 · 2023-10-11T10:39:26.000-04:00
Applying generic arguments to a variadic generic type uses the PackMatcher to build a mapping between generic parameters and arguments. The PackMatcher is symmetric, so there was an unexpected failure mode that wasn't handled: if the variadic generic type had some non-pack parameters, but the argument list was a single pack expansion type, the match would succeed, grouping all of the generic parameters into a single match. This is non-sensical, so we need to explicitly check for this case and diagnose it. This requires a new diagnostic, since otherwise the general diagnostic we emit for variadic generic type mismatches doesn't make sense, since it's complaining about there being too few generic parameters. Fixes rdar://116713961 / swiftlang#69012.
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -4411,10 +4411,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,
diff --git a/lib/Sema/CSDiagnostics.cpp b/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"
@@ -9306,7 +9307,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;
 }
diff --git a/lib/Sema/TypeCheckType.cpp b/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"
@@ -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.
@@ -834,7 +880,6 @@ static Type applyGenericArguments(Type type, TypeResolution resolution,
           return paramDecl->isParameterPack();
       });
 
-  // Resolve the types of the generic arguments.
   auto argOptions = options.withoutContext().withContext(
       TypeResolverContext::GenericArgument);
   auto genericResolution = resolution.withOptions(argOptions);
@@ -848,6 +893,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.
@@ -864,15 +910,9 @@ static Type applyGenericArguments(Type type, TypeResolution resolution,
     // 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 +930,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);
     }
diff --git a/lib/Sema/TypeCheckType.h b/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.
@@ -655,6 +656,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 */
diff --git a/test/Generics/variadic_generic_types.swift b/test/Generics/variadic_generic_types.swift
@@ -96,3 +96,18 @@ func bindAliasPrefixAndSuffix() {
   typealias Three = Bind<Int, String, Float> // OK
   typealias Four = Bind<Int, String, Float, Bool> // OK
 }
+
+func invalidPackExpansion<each X, each Y, Z>(x: repeat each X, y: repeat each Y, z: Z) {
+  typealias A<T, each U> = (T, repeat each U) // expected-note 4{{generic type 'A' declared here}}
+  typealias B<each T, U> = (repeat each T, U) // expected-note 4{{generic type 'B' declared here}}
+
+  typealias One = A<repeat each X> // expected-error {{generic type 'A' specialized with mismatched type parameter pack}}
+  typealias Two = A<repeat each X, repeat each Y> // expected-error {{generic type 'A' specialized with mismatched type parameter pack}}
+  typealias Three = A<repeat each X, Z> // expected-error {{generic type 'A' specialized with mismatched type parameter pack}}
+  typealias Four = A<repeat each X, repeat each Y, Z> // expected-error {{generic type 'A' specialized with mismatched type parameter pack}}
+
+  typealias Five = B<repeat each X> // expected-error {{generic type 'B' specialized with mismatched type parameter pack}}
+  typealias Six = B<repeat each X, repeat each Y> // expected-error {{generic type 'B' specialized with mismatched type parameter pack}}
+  typealias Seven = B<Z, repeat each X> // expected-error {{generic type 'B' specialized with mismatched type parameter pack}}
+  typealias Eight = B<Z, repeat each X, repeat each Y> // expected-error {{generic type 'B' specialized with mismatched type parameter pack}}
+}
diff --git a/test/Macros/macros_diagnostics.swift b/test/Macros/macros_diagnostics.swift
@@ -3,7 +3,7 @@
 // RUN: %target-typecheck-verify-swift -swift-version 5 -enable-experimental-feature CodeItemMacros -module-name MacrosTest
 
 @expression macro stringify<T>(_ value: T) -> (T, String) = #externalMacro(module: "MacroDefinition", type: "StringifyMacro")
-// expected-note@-1 2{{'stringify' declared here}}
+// expected-note@-1 3{{'stringify' declared here}}
 // expected-warning@-2{{external macro implementation type}}
 // expected-warning@-3{{@expression has been removed in favor of @freestanding(expression)}}{{1-12=@freestanding(expression)}}
 @freestanding(expression) macro missingMacro1(_: Any) = MissingModule.MissingType // expected-note{{'missingMacro1' declared here}}
