Improve diagnoses of generic specializations

Always add constraints, find fixes during simplify.
New separate fix for allow generic function specialization.
Improve parse heuristic for isGenericTypeDisambiguatingToken.
New separate warning for allow macro specialization.

Author: Greg Titus

include/swift/AST/DiagnosticsSema.def

@@ -4605,10 +4605,11 @@ NOTE(duplicated_key_declared_here, none,
 // Generic specializations
 ERROR(cannot_explicitly_specialize_generic_function,none,
       "cannot explicitly specialize a generic function", ())
-ERROR(not_a_generic_definition,none,
-      "cannot specialize a non-generic definition", ())
 ERROR(not_a_generic_type,none,
       "cannot specialize non-generic type %0", (Type))
+WARNING(cannot_specialize_macro,none,
+      "specializing a macro with type parameters is deprecated; "
+      "replace with macro arguments instead", ())
 ERROR(protocol_declares_unknown_primary_assoc_type,none,
       "an associated type named %0 must be declared in the protocol %1 or a protocol it inherits",
       (Identifier, Type))

include/swift/AST/Types.h

@@ -7327,7 +7327,7 @@ class PlaceholderType : public TypeBase {
   // recursive property logic in PlaceholderType::get.
   using Originator =
       llvm::PointerUnion<TypeVariableType *, DependentMemberType *, VarDecl *,
-                         ErrorExpr *, PlaceholderTypeRepr *>;
+                         ErrorExpr *, TypeRepr *>;
 
   Originator O;
 

include/swift/Sema/CSFix.h

@@ -462,9 +462,15 @@ enum class FixKind : uint8_t {
   /// because its name doesn't appear in 'initializes' or 'accesses' attributes.
   AllowInvalidMemberReferenceInInitAccessor,
 
-  /// Ignore an attempt to specialize non-generic type.
+  /// Ignore an attempt to specialize a non-generic type.
   AllowConcreteTypeSpecialization,
 
+  /// Ignore an attempt to specialize a generic function.
+  AllowGenericFunctionSpecialization,
+
+  /// Ignore an attempt to specialize a macro.
+  AllowMacroSpecialization,
+
   /// Ignore an out-of-place \c then statement.
   IgnoreOutOfPlaceThenStmt,
 
@@ -3743,6 +3749,58 @@ class AllowConcreteTypeSpecialization final : public ConstraintFix {
   }
 };
 
+class AllowGenericFunctionSpecialization final : public ConstraintFix {
+  ValueDecl *Decl;
+
+  AllowGenericFunctionSpecialization(ConstraintSystem &cs, ValueDecl *decl,
+                                     ConstraintLocator *locator)
+      : ConstraintFix(cs, FixKind::AllowConcreteTypeSpecialization, locator),
+        Decl(decl) {}
+
+public:
+  std::string getName() const override {
+    return "allow generic function specialization";
+  }
+
+  bool diagnose(const Solution &solution, bool asNote = false) const override;
+
+  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
+    return diagnose(*commonFixes.front().first);
+  }
+
+  static AllowGenericFunctionSpecialization *
+  create(ConstraintSystem &cs, ValueDecl *decl, ConstraintLocator *locator);
+
+  static bool classof(const ConstraintFix *fix) {
+    return fix->getKind() == FixKind::AllowGenericFunctionSpecialization;
+  }
+};
+
+class AllowMacroSpecialization final : public ConstraintFix {
+  ValueDecl *Decl;
+
+  AllowMacroSpecialization(ConstraintSystem &cs, ValueDecl *decl,
+                           ConstraintLocator *locator)
+      : ConstraintFix(cs, FixKind::AllowMacroSpecialization, locator),
+        Decl(decl) {}
+
+public:
+  std::string getName() const override { return "allow macro specialization"; }
+
+  bool diagnose(const Solution &solution, bool asNote = false) const override;
+
+  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
+    return diagnose(*commonFixes.front().first);
+  }
+
+  static AllowMacroSpecialization *create(ConstraintSystem &cs, ValueDecl *decl,
+                                          ConstraintLocator *locator);
+
+  static bool classof(const ConstraintFix *fix) {
+    return fix->getKind() == FixKind::AllowMacroSpecialization;
+  }
+};
+
 class IgnoreOutOfPlaceThenStmt final : public ConstraintFix {
   IgnoreOutOfPlaceThenStmt(ConstraintSystem &cs, ConstraintLocator *locator)
       : ConstraintFix(cs, FixKind::IgnoreOutOfPlaceThenStmt, locator) {}

include/swift/Sema/ConstraintLocator.h

@@ -985,13 +985,13 @@ class LocatorPathElt::ConformanceRequirement final
 };
 
 class LocatorPathElt::PlaceholderType final
-    : public StoredPointerElement<PlaceholderTypeRepr> {
+    : public StoredPointerElement<TypeRepr> {
 public:
-  PlaceholderType(PlaceholderTypeRepr *placeholderRepr)
+  PlaceholderType(TypeRepr *placeholderRepr)
       : StoredPointerElement(PathElementKind::PlaceholderType,
                              placeholderRepr) {}
 
-  PlaceholderTypeRepr *getPlaceholderRepr() const { return getStoredPointer(); }
+  TypeRepr *getPlaceholderRepr() const { return getStoredPointer(); }
 
   static bool classof(const LocatorPathElt *elt) {
     return elt->getKind() == ConstraintLocator::PlaceholderType;

lib/AST/ASTDumper.cpp

@@ -3987,8 +3987,8 @@ namespace {
         printFlag("error_expr");
       } else if (auto *DMT = originator.dyn_cast<DependentMemberType *>()) {
         printRec(DMT, "dependent_member_type");
-      } else if (originator.is<PlaceholderTypeRepr *>()) {
-        printFlag("placeholder_type_repr");
+      } else if (originator.is<TypeRepr *>()) {
+        printFlag("type_repr");
       } else {
         assert(false && "unknown originator");
       }

lib/AST/ASTPrinter.cpp

@@ -5870,8 +5870,8 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
         Printer << "error_expr";
       } else if (auto *DMT = originator.dyn_cast<DependentMemberType *>()) {
         visit(DMT);
-      } else if (originator.is<PlaceholderTypeRepr *>()) {
-        Printer << "placeholder_type_repr";
+      } else if (originator.is<TypeRepr *>()) {
+        Printer << "type_repr";
       } else {
         assert(false && "unknown originator");
       }

lib/Parse/ParseType.cpp

@@ -1476,7 +1476,9 @@ static bool isGenericTypeDisambiguatingToken(Parser &P) {
   auto &tok = P.Tok;
   switch (tok.getKind()) {
   default:
-    return false;
+    // If this is the end of the expr (wouldn't match parseExprSequenceElement),
+    // prefer generic type list over an illegal unary postfix '>' operator.
+    return P.isStartOfSwiftDecl() || P.isStartOfStmt(/*prefer expr=*/true);
   case tok::r_paren:
   case tok::r_square:
   case tok::l_brace:

lib/Sema/CSDiagnostics.cpp

@@ -9364,6 +9364,16 @@ bool ConcreteTypeSpecialization::diagnoseAsError() {
   return true;
 }
 
+bool GenericFunctionSpecialization::diagnoseAsError() {
+  emitDiagnostic(diag::cannot_explicitly_specialize_generic_function);
+  return true;
+}
+
+bool MacroSpecialization::diagnoseAsError() {
+  emitDiagnostic(diag::cannot_specialize_macro);
+  return true;
+}
+
 bool OutOfPlaceThenStmtFailure::diagnoseAsError() {
   emitDiagnostic(diag::out_of_place_then_stmt);
   return true;

lib/Sema/CSDiagnostics.h

@@ -3137,6 +3137,28 @@ class ConcreteTypeSpecialization final : public FailureDiagnostic {
   bool diagnoseAsError() override;
 };
 
+class GenericFunctionSpecialization final : public FailureDiagnostic {
+  ValueDecl *Decl;
+
+public:
+  GenericFunctionSpecialization(const Solution &solution, ValueDecl *decl,
+                                ConstraintLocator *locator)
+      : FailureDiagnostic(solution, locator), Decl(decl) {}
+
+  bool diagnoseAsError() override;
+};
+
+class MacroSpecialization final : public FailureDiagnostic {
+  ValueDecl *Decl;
+
+public:
+  MacroSpecialization(const Solution &solution, ValueDecl *decl,
+                      ConstraintLocator *locator)
+      : FailureDiagnostic(solution, locator), Decl(decl) {}
+
+  bool diagnoseAsError() override;
+};
+
 /// Diagnose an out-of-place ThenStmt.
 class OutOfPlaceThenStmtFailure final : public FailureDiagnostic {
 public:

lib/Sema/CSFix.cpp

@@ -2616,6 +2616,30 @@ AllowConcreteTypeSpecialization::create(ConstraintSystem &cs, Type concreteTy,
       AllowConcreteTypeSpecialization(cs, concreteTy, locator);
 }
 
+bool AllowGenericFunctionSpecialization::diagnose(const Solution &solution,
+                                                  bool asNote) const {
+  GenericFunctionSpecialization failure(solution, Decl, getLocator());
+  return failure.diagnose(asNote);
+}
+
+AllowGenericFunctionSpecialization *AllowGenericFunctionSpecialization::create(
+    ConstraintSystem &cs, ValueDecl *decl, ConstraintLocator *locator) {
+  return new (cs.getAllocator())
+      AllowGenericFunctionSpecialization(cs, decl, locator);
+}
+
+bool AllowMacroSpecialization::diagnose(const Solution &solution,
+                                        bool asNote) const {
+  MacroSpecialization failure(solution, Decl, getLocator());
+  return failure.diagnose(asNote);
+}
+
+AllowMacroSpecialization *
+AllowMacroSpecialization::create(ConstraintSystem &cs, ValueDecl *decl,
+                                 ConstraintLocator *locator) {
+  return new (cs.getAllocator()) AllowMacroSpecialization(cs, decl, locator);
+}
+
 bool IgnoreOutOfPlaceThenStmt::diagnose(const Solution &solution,
                                         bool asNote) const {
   OutOfPlaceThenStmtFailure failure(solution, getLocator());

lib/Sema/CSGen.cpp

@@ -1903,9 +1903,9 @@ namespace {
     /// introduce them as an explicit generic arguments constraint.
     ///
     /// \returns true if resolving any of the specialization types failed.
-    bool addSpecializationConstraint(
-        ConstraintLocator *locator, Type boundType,
-        ArrayRef<TypeRepr *> specializationArgs) {
+    void addSpecializationConstraint(ConstraintLocator *locator, Type boundType,
+                                     SourceLoc lAngleLoc,
+                                     ArrayRef<TypeRepr *> specializationArgs) {
       // Resolve each type.
       SmallVector<Type, 2> specializationArgTypes;
       auto options =
@@ -1916,61 +1916,35 @@ namespace {
           options |= TypeResolutionFlags::AllowPackReferences;
           elementEnv = OuterExpansions.back();
         }
-        const auto result = TypeResolution::resolveContextualType(
+        auto result = TypeResolution::resolveContextualType(
             specializationArg, CurDC, options,
             // Introduce type variables for unbound generics.
             OpenUnboundGenericType(CS, locator),
             HandlePlaceholderType(CS, locator),
             OpenPackElementType(CS, locator, elementEnv));
-        if (result->hasError())
-          return true;
-
+        if (result->hasError()) {
+          auto &ctxt = CS.getASTContext();
+          result = PlaceholderType::get(ctxt, specializationArg);
+          ctxt.Diags.diagnose(lAngleLoc,
+                              diag::while_parsing_as_left_angle_bracket);
+        }
         specializationArgTypes.push_back(result);
       }
 
-      CS.addConstraint(
-          ConstraintKind::ExplicitGenericArguments, boundType,
-          PackType::get(CS.getASTContext(), specializationArgTypes),
-          locator);
-      return false;
+      auto constraint = Constraint::create(
+          CS, ConstraintKind::ExplicitGenericArguments, boundType,
+          PackType::get(CS.getASTContext(), specializationArgTypes), locator);
+      CS.addUnsolvedConstraint(constraint);
+      CS.activateConstraint(constraint);
     }
 
     Type visitUnresolvedSpecializeExpr(UnresolvedSpecializeExpr *expr) {
       auto baseTy = CS.getType(expr->getSubExpr());
-
-      if (baseTy->isTypeVariableOrMember()) {
-        return baseTy;
-      }
-
-      // We currently only support explicit specialization of generic types.
-      // FIXME: We could support explicit function specialization.
-      auto &de = CS.getASTContext().Diags;
-      if (baseTy->is<AnyFunctionType>()) {
-        de.diagnose(expr->getSubExpr()->getLoc(),
-                    diag::cannot_explicitly_specialize_generic_function);
-        de.diagnose(expr->getLAngleLoc(),
-                    diag::while_parsing_as_left_angle_bracket);
-        return Type();
-      }
-      
-      if (AnyMetatypeType *meta = baseTy->getAs<AnyMetatypeType>()) {
-        auto *overloadLocator = CS.getConstraintLocator(expr->getSubExpr());
-        if (addSpecializationConstraint(overloadLocator,
-                                        meta->getInstanceType(),
-                                        expr->getUnresolvedParams())) {
-          return Type();
-        }
-
-        return baseTy;
-      }
-
-      // FIXME: If the base type is a type variable, constrain it to a metatype
-      // of a bound generic type.
-      de.diagnose(expr->getSubExpr()->getLoc(),
-                  diag::not_a_generic_definition);
-      de.diagnose(expr->getLAngleLoc(),
-                  diag::while_parsing_as_left_angle_bracket);
-      return Type();
+      auto *overloadLocator = CS.getConstraintLocator(expr->getSubExpr());
+      addSpecializationConstraint(
+          overloadLocator, baseTy->getMetatypeInstanceType(),
+          expr->getLAngleLoc(), expr->getUnresolvedParams());
+      return baseTy;
     }
 
     Type visitSequenceExpr(SequenceExpr *expr) {
@@ -4080,10 +4054,9 @@ namespace {
 
       // Add explicit generic arguments, if there were any.
       if (expr->getGenericArgsRange().isValid()) {
-        if (addSpecializationConstraint(
-              CS.getConstraintLocator(expr), macroRefType,
-              expr->getGenericArgs()))
-          return Type();
+        addSpecializationConstraint(CS.getConstraintLocator(expr), macroRefType,
+                                    expr->getGenericArgsRange().Start,
+                                    expr->getGenericArgs());
       }
 
       // Form the applicable-function constraint. The result type
@@ -4828,11 +4801,11 @@ bool ConstraintSystem::generateConstraints(
         // If we have a placeholder originating from a PlaceholderTypeRepr,
         // tack that on to the locator.
         if (auto *placeholderTy = ty->getAs<PlaceholderType>())
-          if (auto *placeholderRepr = placeholderTy->getOriginator()
-                                          .dyn_cast<PlaceholderTypeRepr *>())
+          if (auto *typeRepr = placeholderTy->getOriginator()
+                                          .dyn_cast<TypeRepr *>())
             return getConstraintLocator(
                 convertTypeLocator,
-                LocatorPathElt::PlaceholderType(placeholderRepr));
+                LocatorPathElt::PlaceholderType(typeRepr));
         return convertTypeLocator;
       };