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.
Degrade concrete type specialization fix to warning for macros.

Author: Greg Titus

include/swift/AST/DiagnosticsSema.def

@@ -4605,8 +4605,6 @@ 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))
 ERROR(protocol_declares_unknown_primary_assoc_type,none,

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,12 @@ 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 out-of-place \c then statement.
   IgnoreOutOfPlaceThenStmt,
 
@@ -3720,8 +3723,10 @@ class AllowConcreteTypeSpecialization final : public ConstraintFix {
   Type ConcreteType;
 
   AllowConcreteTypeSpecialization(ConstraintSystem &cs, Type concreteTy,
-                                  ConstraintLocator *locator)
-      : ConstraintFix(cs, FixKind::AllowConcreteTypeSpecialization, locator),
+                                  ConstraintLocator *locator,
+                                  FixBehavior fixBehavior)
+      : ConstraintFix(cs, FixKind::AllowConcreteTypeSpecialization, locator,
+                      fixBehavior),
         ConcreteType(concreteTy) {}
 
 public:
@@ -3735,14 +3740,43 @@ class AllowConcreteTypeSpecialization final : public ConstraintFix {
     return diagnose(*commonFixes.front().first);
   }
 
-  static AllowConcreteTypeSpecialization *
-  create(ConstraintSystem &cs, Type concreteTy, ConstraintLocator *locator);
+  static AllowConcreteTypeSpecialization *create(ConstraintSystem &cs,
+                                                 Type concreteTy,
+                                                 ConstraintLocator *locator,
+                                                 FixBehavior fixBehavior);
 
   static bool classof(const ConstraintFix *fix) {
     return fix->getKind() == FixKind::AllowConcreteTypeSpecialization;
   }
 };
 
+class AllowGenericFunctionSpecialization final : public ConstraintFix {
+  ValueDecl *Decl;
+
+  AllowGenericFunctionSpecialization(ConstraintSystem &cs, ValueDecl *decl,
+                                     ConstraintLocator *locator)
+      : ConstraintFix(cs, FixKind::AllowGenericFunctionSpecialization, 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 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

@@ -3130,13 +3130,36 @@ class ConcreteTypeSpecialization final : public FailureDiagnostic {
 
 public:
   ConcreteTypeSpecialization(const Solution &solution, Type concreteTy,
-                             ConstraintLocator *locator)
-      : FailureDiagnostic(solution, locator),
+                             ConstraintLocator *locator,
+                             FixBehavior fixBehavior)
+      : FailureDiagnostic(solution, locator, fixBehavior),
         ConcreteType(resolveType(concreteTy)) {}
 
   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

@@ -2605,15 +2605,29 @@ AllowInvalidMemberReferenceInInitAccessor::create(ConstraintSystem &cs,
 
 bool AllowConcreteTypeSpecialization::diagnose(const Solution &solution,
                                                bool asNote) const {
-  ConcreteTypeSpecialization failure(solution, ConcreteType, getLocator());
+  ConcreteTypeSpecialization failure(solution, ConcreteType, getLocator(),
+                                     fixBehavior);
   return failure.diagnose(asNote);
 }
 
 AllowConcreteTypeSpecialization *
 AllowConcreteTypeSpecialization::create(ConstraintSystem &cs, Type concreteTy,
-                                        ConstraintLocator *locator) {
+                                        ConstraintLocator *locator,
+                                        FixBehavior fixBehavior) {
+  return new (cs.getAllocator())
+      AllowConcreteTypeSpecialization(cs, concreteTy, locator, fixBehavior);
+}
+
+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())
-      AllowConcreteTypeSpecialization(cs, concreteTy, locator);
+      AllowGenericFunctionSpecialization(cs, decl, locator);
 }
 
 bool IgnoreOutOfPlaceThenStmt::diagnose(const Solution &solution,

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;
       };