Merge pull request #84030 from hamishknight/hole-in-one

[CS] Avoid unnecessary type variables when opening ErrorTypes

Author: hamishknight

include/swift/AST/Types.h

@@ -7734,8 +7734,8 @@ class PlaceholderType : public TypeBase {
   // NOTE: If you add a new Type-based originator, you'll need to update the
   // recursive property logic in PlaceholderType::get.
   using Originator =
-      llvm::PointerUnion<TypeVariableType *, DependentMemberType *, VarDecl *,
-                         ErrorExpr *, TypeRepr *>;
+      llvm::PointerUnion<TypeVariableType *, DependentMemberType *, ErrorType *,
+                         VarDecl *, ErrorExpr *, TypeRepr *>;
 
   Originator O;
 

lib/AST/ASTContext.cpp

@@ -3675,6 +3675,9 @@ Type PlaceholderType::get(ASTContext &ctx, Originator originator) {
     if (auto *depTy = originator.dyn_cast<DependentMemberType *>())
       return depTy->getRecursiveProperties();
 
+    if (auto *errTy = originator.dyn_cast<ErrorType *>())
+      return errTy->getRecursiveProperties();
+
     return RecursiveTypeProperties();
   }();
   auto arena = getArena(originatorProps);

lib/AST/ASTDumper.cpp

@@ -6077,6 +6077,8 @@ namespace {
                             Label::optional("originating_var"), DeclColor);
       } else if (isa<ErrorExpr *>(originator)) {
         printFlag("error_expr");
+      } else if (auto *errTy = originator.dyn_cast<ErrorType *>()) {
+        printRec(errTy, Label::always("error_type"));
       } else if (auto *DMT = originator.dyn_cast<DependentMemberType *>()) {
         printRec(DMT, Label::always("dependent_member_type"));
       } else if (isa<TypeRepr *>(originator)) {

lib/AST/ASTPrinter.cpp

@@ -6146,6 +6146,8 @@ class TypePrinter : public TypeVisitor<TypePrinter, void, NonRecursivePrintOptio
         Printer << VD->getName();
       } else if (isa<ErrorExpr *>(originator)) {
         Printer << "error_expr";
+      } else if (auto *errTy = originator.dyn_cast<ErrorType *>()) {
+        visit(errTy);
       } else if (auto *DMT = originator.dyn_cast<DependentMemberType *>()) {
         visit(DMT);
       } else if (isa<TypeRepr *>(originator)) {

lib/Sema/CSGen.cpp

@@ -1712,17 +1712,12 @@ namespace {
       const auto result = TypeResolution::resolveContextualType(
           repr, CS.DC, options, genericOpener, placeholderHandler,
           packElementOpener, requirementOpener);
+      // If we have an error, record a fix and produce a hole for the type.
       if (result->hasError()) {
         CS.recordFix(
             IgnoreInvalidASTNode::create(CS, CS.getConstraintLocator(locator)));
 
-        // Immediately bind the result to a hole since we know it's invalid and
-        // want it to propagate rather than allowing other type variables to
-        // become holes.
-        auto *tv = CS.createTypeVariable(CS.getConstraintLocator(repr),
-                                         TVO_CanBindToHole);
-        CS.recordTypeVariablesAsHoles(tv);
-        return tv;
+        return PlaceholderType::get(CS.getASTContext(), repr);
       }
       // Diagnose top-level usages of placeholder types.
       if (auto *ty = dyn_cast<PlaceholderTypeRepr>(repr->getWithoutParens())) {
@@ -4901,18 +4896,6 @@ bool ConstraintSystem::generateConstraints(
             getConstraintLocator(expr, LocatorPathElt::ContextualType(ctp));
       }
 
-      auto getLocator = [&](Type ty) -> ConstraintLocator * {
-        // If we have a placeholder originating from a PlaceholderTypeRepr,
-        // tack that on to the locator.
-        if (auto *placeholderTy = ty->getAs<PlaceholderType>())
-          if (auto *typeRepr = placeholderTy->getOriginator()
-                                          .dyn_cast<TypeRepr *>())
-            return getConstraintLocator(
-                convertTypeLocator,
-                LocatorPathElt::PlaceholderType(typeRepr));
-        return convertTypeLocator;
-      };
-
       // If the contextual type has an error, we can't apply the solution.
       // Record a fix for an invalid AST node.
       if (convertType->hasError())
@@ -4921,18 +4904,30 @@ bool ConstraintSystem::generateConstraints(
       // Substitute type variables in for placeholder and error types.
       convertType =
           convertType.transformRec([&](Type type) -> std::optional<Type> {
-            if (!isa<PlaceholderType, ErrorType>(type.getPointer()))
-              return std::nullopt;
-
-            auto flags = TVO_CanBindToNoEscape | TVO_PrefersSubtypeBinding |
-                         TVO_CanBindToHole;
-            auto tv = Type(createTypeVariable(getLocator(type), flags));
-            // For ErrorTypes we want to eagerly bind to a hole since we
-            // know this is where the issue is.
-            if (isa<ErrorType>(type.getPointer())) {
-              recordTypeVariablesAsHoles(tv);
+            auto *tyPtr = type.getPointer();
+            // Open a type variable for a placeholder type repr. Note we don't
+            // do this for arbitrary placeholders since they may be existing
+            // holes when e.g generating the constraints for a ReturnStmt in a
+            // closure.
+            if (auto *placeholderTy = dyn_cast<PlaceholderType>(tyPtr)) {
+              auto originator = placeholderTy->getOriginator();
+              auto *typeRepr = originator.dyn_cast<TypeRepr *>();
+              if (!isa_and_nonnull<PlaceholderTypeRepr>(typeRepr))
+                return std::nullopt;
+
+              auto *loc = getConstraintLocator(
+                  convertTypeLocator,
+                  LocatorPathElt::PlaceholderType(typeRepr));
+              return createTypeVariable(loc, TVO_CanBindToNoEscape |
+                                                 TVO_PrefersSubtypeBinding |
+                                                 TVO_CanBindToHole);
             }
-            return tv;
+            // For ErrorTypes we want to eagerly produce a hole since we know
+            // this is where the issue is.
+            if (auto *errTy = dyn_cast<ErrorType>(tyPtr)) {
+              return PlaceholderType::get(getASTContext(), errTy);
+            }
+            return std::nullopt;
           });
 
       addContextualConversionConstraint(expr, convertType, ctp,

lib/Sema/TypeOfReference.cpp

@@ -274,11 +274,9 @@ class InferableTypeOpener final {
   }
 
   Type transformErrorType(ErrorType *errTy) {
-    // For ErrorTypes we want to eagerly bind to a hole since we know this is
+    // For ErrorTypes we want to eagerly produce a hole since we know this is
     // where the issue is.
-    auto *tv = createTypeVariable(cs.getConstraintLocator(locator));
-    cs.recordTypeVariablesAsHoles(tv);
-    return tv;
+    return PlaceholderType::get(cs.getASTContext(), errTy);
   }
 
   Type transform(Type type) {