Merge pull request #84390 from hamishknight/out-of-place

hamishknight · web-flow · commit 801cd646adf9 · 2025-09-19T23:19:48.000+01:00
[Sema] Reject placeholders in type resolution for param and result types
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -4863,10 +4863,6 @@ NOTE(descriptive_generic_type_declared_here,none,
 
 ERROR(placeholder_type_not_allowed,none,
       "type placeholder not allowed here", ())
-ERROR(placeholder_type_not_allowed_in_return_type,none,
-      "type placeholder may not appear in function return type", ())
-ERROR(placeholder_type_not_allowed_in_parameter,none,
-      "type placeholder may not appear in top-level parameter", ())
 ERROR(placeholder_type_not_allowed_in_pattern,none,
       "placeholder type may not appear as type of a variable", ())
 NOTE(replace_placeholder_with_inferred_type,none,
diff --git a/include/swift/AST/TypeMatcher.h b/include/swift/AST/TypeMatcher.h
@@ -117,12 +117,6 @@ class TypeMatcher {
 #define SINGLETON_TYPE(SHORT_ID, ID) TRIVIAL_CASE(ID##Type)
 #include "swift/AST/TypeNodes.def"
 
-    bool visitPlaceholderType(CanPlaceholderType firstType, Type secondType,
-                              Type sugaredFirstType) {
-      // Placeholder types never match.
-      return mismatch(firstType.getPointer(), secondType, sugaredFirstType);
-    }
-
     bool visitUnresolvedType(CanUnresolvedType firstType, Type secondType,
                              Type sugaredFirstType) {
       // Unresolved types never match.
diff --git a/include/swift/Sema/CSFix.h b/include/swift/Sema/CSFix.h
@@ -362,9 +362,6 @@ enum class FixKind : uint8_t {
   /// resolved.
   SpecifyTypeForPlaceholder,
 
-  /// Ignore an invalid placeholder in a decl's interface type.
-  IgnoreInvalidPlaceholderInDeclRef,
-
   /// Allow Swift -> C pointer conversion in an argument position
   /// of a Swift function.
   AllowSwiftToCPointerConversion,
@@ -3194,30 +3191,6 @@ class SpecifyTypeForPlaceholder final : public ConstraintFix {
   }
 };
 
-class IgnoreInvalidPlaceholderInDeclRef final : public ConstraintFix {
-  IgnoreInvalidPlaceholderInDeclRef(ConstraintSystem &cs,
-                                    ConstraintLocator *locator)
-      : ConstraintFix(cs, FixKind::IgnoreInvalidPlaceholderInDeclRef, locator) {}
-
-public:
-  std::string getName() const override {
-    return "ignore invalid placeholder in decl ref";
-  }
-
-  bool diagnose(const Solution &solution, bool asNote = false) const override;
-
-  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
-    return diagnose(*commonFixes.front().first);
-  }
-
-  static IgnoreInvalidPlaceholderInDeclRef *create(ConstraintSystem &cs,
-                                                   ConstraintLocator *locator);
-
-  static bool classof(const ConstraintFix *fix) {
-    return fix->getKind() == FixKind::IgnoreInvalidPlaceholderInDeclRef;
-  }
-};
-
 class AllowRefToInvalidDecl final : public ConstraintFix {
   AllowRefToInvalidDecl(ConstraintSystem &cs, ConstraintLocator *locator)
       : ConstraintFix(cs, FixKind::AllowRefToInvalidDecl, locator) {}
diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp
@@ -1222,11 +1222,16 @@ std::optional<Type> InterfaceTypeRequest::getCachedResult() const {
 void InterfaceTypeRequest::cacheResult(Type type) const {
   auto *decl = std::get<0>(getStorage());
   if (type) {
-    assert(!type->hasTypeVariable() && "Type variable in interface type");
-    assert(!type->is<InOutType>() && "Interface type must be materializable");
-    assert(!type->hasPrimaryArchetype() && "Archetype in interface type");
-    assert(decl->getDeclContext()->isLocalContext() || !type->hasLocalArchetype() &&
+    ASSERT(!type->hasTypeVariable() && "Type variable in interface type");
+    ASSERT(!type->is<InOutType>() && "Interface type must be materializable");
+    ASSERT(!type->hasPrimaryArchetype() && "Archetype in interface type");
+    ASSERT(decl->getDeclContext()->isLocalContext() || !type->hasLocalArchetype() &&
            "Local archetype in interface type of non-local declaration");
+    // Placeholders are only permitted in closure parameters.
+    if (!(isa<ParamDecl>(decl) &&
+          isa<AbstractClosureExpr>(decl->getDeclContext()))) {
+      ASSERT(!type->hasPlaceholder() && "Placeholder in interface type");
+    }
   }
   decl->TypeAndAccess.setPointer(type);
 }
diff --git a/lib/Sema/AssociatedTypeInference.cpp b/lib/Sema/AssociatedTypeInference.cpp
@@ -2317,9 +2317,9 @@ AssociatedTypeInference::getPotentialTypeWitnessesByMatchingTypes(ValueDecl *req
     /// Deduce associated types from dependent member types in the witness.
     bool mismatch(DependentMemberType *firstDepMember,
                   TypeBase *secondType, Type sugaredFirstType) {
-      // If the second type is an error or placeholder, don't look at it
-      // further, but proceed to find other matches.
-      if (secondType->hasError() || secondType->hasPlaceholder())
+      // If the second type is an error, don't look at it further, but proceed
+      // to find other matches.
+      if (secondType->hasError())
         return true;
 
       // If the second type is a generic parameter of the witness, the match
diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp
@@ -2264,20 +2264,6 @@ SpecifyTypeForPlaceholder::create(ConstraintSystem &cs,
   return new (cs.getAllocator()) SpecifyTypeForPlaceholder(cs, locator);
 }
 
-IgnoreInvalidPlaceholderInDeclRef *
-IgnoreInvalidPlaceholderInDeclRef::create(ConstraintSystem &cs, ConstraintLocator *locator) {
-  return new (cs.getAllocator()) IgnoreInvalidPlaceholderInDeclRef(cs, locator);
-}
-
-bool
-IgnoreInvalidPlaceholderInDeclRef::diagnose(const Solution &solution,
-                                            bool asNote) const {
-  // These are diagnosed separately. Unfortunately we can't enforce that a
-  // diagnostic has already been emitted since their diagnosis depends on e.g
-  // type-checking a function body for a placeholder result of a function.
-  return true;
-}
-
 bool AllowRefToInvalidDecl::diagnose(const Solution &solution,
                                      bool asNote) const {
   ReferenceToInvalidDeclaration failure(solution, getLocator());
diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp
@@ -16099,7 +16099,6 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::AllowValueExpansionWithoutPackReferences:
   case FixKind::IgnoreInvalidPatternInExpr:
   case FixKind::IgnoreInvalidPlaceholder:
-  case FixKind::IgnoreInvalidPlaceholderInDeclRef:
   case FixKind::IgnoreOutOfPlaceThenStmt:
   case FixKind::IgnoreMissingEachKeyword:
   case FixKind::AllowInlineArrayLiteralCountMismatch:
diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp
@@ -3868,71 +3868,6 @@ class OpaqueUnderlyingTypeChecker : public ASTWalker {
   }
 };
 
-class ReturnTypePlaceholderReplacer : public ASTWalker {
-  FuncDecl *Implementation;
-  BraceStmt *Body;
-  SmallVector<Type, 4> Candidates;
-
-  bool HasInvalidReturn = false;
-
-public:
-  ReturnTypePlaceholderReplacer(FuncDecl *Implementation, BraceStmt *Body)
-      : Implementation(Implementation), Body(Body) {}
-
-  void check() {
-    auto *resultRepr = Implementation->getResultTypeRepr();
-    if (!resultRepr) {
-      return;
-    }
-
-    Implementation->getASTContext()
-        .Diags
-        .diagnose(resultRepr->getLoc(),
-                  diag::placeholder_type_not_allowed_in_return_type)
-        .highlight(resultRepr->getSourceRange());
-
-    Body->walk(*this);
-
-    // If given function has any invalid returns in the body
-    // let's not try to validate the types, since it wouldn't
-    // be accurate.
-    if (HasInvalidReturn)
-      return;
-
-    auto writtenType = Implementation->getResultInterfaceType();
-    llvm::SmallPtrSet<TypeBase *, 8> seenTypes;
-    for (auto candidate : Candidates) {
-      if (!seenTypes.insert(candidate.getPointer()).second) {
-        continue;
-      }
-      TypeChecker::notePlaceholderReplacementTypes(writtenType, candidate);
-    }
-  }
-
-  MacroWalking getMacroWalkingBehavior() const override {
-    return MacroWalking::ArgumentsAndExpansion;
-  }
-
-  PreWalkResult<Expr *> walkToExprPre(Expr *E) override { return Action::Continue(E); }
-
-  PreWalkResult<Stmt *> walkToStmtPre(Stmt *S) override {
-    if (auto *RS = dyn_cast<ReturnStmt>(S)) {
-      if (RS->hasResult()) {
-        auto resultTy = RS->getResult()->getType();
-        HasInvalidReturn |= resultTy.isNull() || resultTy->hasError();
-        Candidates.push_back(resultTy);
-      }
-    }
-
-    return Action::Continue(S);
-  }
-
-  // Don't descend into nested decls.
-  PreWalkAction walkToDeclPre(Decl *D) override {
-    return Action::SkipNode();
-  }
-};
-
 } // end anonymous namespace
 
 SourceLoc swift::getFixItLocForVarToLet(VarDecl *var) {
@@ -4545,13 +4480,6 @@ void swift::performAbstractFuncDeclDiagnostics(AbstractFunctionDecl *AFD) {
         OpaqueUnderlyingTypeChecker(AFD, opaqueResultTy, body).check();
       }
     }
-  } else if (auto *FD = dyn_cast<FuncDecl>(AFD)) {
-    auto resultIFaceTy = FD->getResultInterfaceType();
-    // If the result has a placeholder, we need to try to use the contextual
-    // type inferred in the body to replace it.
-    if (resultIFaceTy && resultIFaceTy->hasPlaceholder()) {
-      ReturnTypePlaceholderReplacer(FD, body).check();
-    }
   }
 }
 
diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp
@@ -2121,17 +2121,10 @@ ResultTypeRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
   if (decl->preconcurrency())
     options |= TypeResolutionFlags::Preconcurrency;
 
-  // Placeholders are only currently allowed for FuncDecls with bodies, which
-  // we diagnose in ReturnTypePlaceholderReplacer.
-  HandlePlaceholderTypeReprFn placeholderOpener;
-  if (auto *FD = dyn_cast<FuncDecl>(decl)) {
-    if (FD->hasBody() && !FD->isBodySkipped())
-      placeholderOpener = PlaceholderType::get;
-  }
   auto *const dc = decl->getInnermostDeclContext();
   return TypeResolution::forInterface(dc, options,
                                       /*unboundTyOpener*/ nullptr,
-                                      placeholderOpener,
+                                      /*placeholderOpener*/ nullptr,
                                       /*packElementOpener*/ nullptr)
       .resolveType(resultTyRepr);
 }
@@ -2259,6 +2252,7 @@ static Type validateParameterType(ParamDecl *decl) {
 
   TypeResolutionOptions options(std::nullopt);
   OpenUnboundGenericTypeFn unboundTyOpener = nullptr;
+  HandlePlaceholderTypeReprFn placeholderOpener = nullptr;
   if (isa<AbstractClosureExpr>(dc)) {
     options = TypeResolutionOptions(TypeResolverContext::ClosureExpr);
     options |= TypeResolutionFlags::AllowUnspecifiedTypes;
@@ -2267,8 +2261,9 @@ static Type validateParameterType(ParamDecl *decl) {
       // For now, just return the unbound generic type.
       return unboundTy;
     };
-    // FIXME: Don't let placeholder types escape type resolution.
-    // For now, just return the placeholder type.
+    // FIXME: Don't let placeholder types escape type resolution. For now, just
+    // return the placeholder type, which we open in `inferClosureType`.
+    placeholderOpener = PlaceholderType::get;
   } else if (isa<AbstractFunctionDecl>(dc)) {
     options = TypeResolutionOptions(TypeResolverContext::AbstractFunctionDecl);
   } else if (isa<SubscriptDecl>(dc)) {
@@ -2317,14 +2312,6 @@ static Type validateParameterType(ParamDecl *decl) {
                      : TypeResolverContext::FunctionInput);
   options |= TypeResolutionFlags::Direct;
 
-  // We allow placeholders in parameter types to improve recovery since if a
-  // default argument is present we can suggest the inferred type. Avoid doing
-  // this for protocol requirements though since those can't ever have default
-  // arguments anyway.
-  HandlePlaceholderTypeReprFn placeholderOpener;
-  if (!isa<ProtocolDecl>(dc->getParent()))
-    placeholderOpener = PlaceholderType::get;
-
   const auto resolution =
       TypeResolution::forInterface(dc, options, unboundTyOpener,
                                    placeholderOpener,
diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp
@@ -1193,73 +1193,18 @@ static bool checkExpressionMacroDefaultValueRestrictions(ParamDecl *param) {
 #endif
 }
 
-void TypeChecker::notePlaceholderReplacementTypes(Type writtenType,
-                                                  Type inferredType) {
-  assert(writtenType && inferredType &&
-         "Must provide both written and inferred types");
-  assert(writtenType->hasPlaceholder() && "Written type has no placeholder?");
-
-  class PlaceholderNotator
-      : public CanTypeDifferenceVisitor<PlaceholderNotator> {
-  public:
-    bool visitDifferentComponentTypes(CanType t1, CanType t2) {
-      // Never replace anything the user wrote with an error type.
-      if (t2->hasError() || t2->hasUnresolvedType()) {
-        return false;
-      }
-
-      auto *placeholder = t1->getAs<PlaceholderType>();
-      if (!placeholder) {
-        return false;
-      }
-
-      if (auto *origRepr =
-              placeholder->getOriginator().dyn_cast<TypeRepr *>()) {
-        assert(isa<PlaceholderTypeRepr>(origRepr));
-        t1->getASTContext()
-            .Diags
-            .diagnose(origRepr->getLoc(),
-                      diag::replace_placeholder_with_inferred_type, t2)
-            .fixItReplace(origRepr->getSourceRange(), t2.getString());
-      }
-      return false;
-    }
-
-    bool check(Type t1, Type t2) {
-      return !visit(t1->getCanonicalType(), t2->getCanonicalType());
-    };
-  };
-
-  PlaceholderNotator().check(writtenType, inferredType);
-}
-
 /// Check the default arguments that occur within this parameter list.
 static void checkDefaultArguments(ParameterList *params) {
   // Force the default values in case they produce diagnostics.
   for (auto *param : *params) {
-    auto ifacety = param->getInterfaceType();
-    auto *expr = param->getTypeCheckedDefaultExpr();
+    (void)param->getTypeCheckedDefaultExpr();
 
     // Force captures since this can emit diagnostics.
-    (void) param->getDefaultArgumentCaptureInfo();
+    (void)param->getDefaultArgumentCaptureInfo();
 
     // If the default argument has isolation, it must match the
     // isolation of the decl context.
     (void)param->getInitializerIsolation();
-
-    if (!ifacety->hasPlaceholder()) {
-      continue;
-    }
-
-    // Placeholder types are banned for all parameter decls. We try to use the
-    // freshly-checked default expression's contextual type to suggest a
-    // reasonable type to insert.
-    param->diagnose(diag::placeholder_type_not_allowed_in_parameter)
-        .highlight(param->getSourceRange());
-    if (expr && !expr->getType()->hasError()) {
-      TypeChecker::notePlaceholderReplacementTypes(
-          ifacety, expr->getType()->mapTypeOutOfContext());
-    }
   }
 }
 
diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h
@@ -1238,24 +1238,6 @@ bool diagnoseInvalidFunctionType(FunctionType *fnTy, SourceLoc loc,
                                  std::optional<FunctionTypeRepr *> repr,
                                  DeclContext *dc,
                                  std::optional<TypeResolutionStage> stage);
-
-/// Walk the parallel structure of a type with user-provided placeholders and
-/// an inferred type produced by the type checker. Where placeholders can be
-/// found, suggest the corresponding inferred type.
-///
-/// For example,
-///
-/// \code
-///  func foo(_ x: [_] = [0])
-/// \endcode
-///
-/// Has a written type of `(ArraySlice (Placeholder))` and an inferred type of
-/// `(ArraySlice Int)`, so we walk to `Placeholder` and `Int` in each type and
-/// suggest replacing `_` with `Int`.
-///
-/// \param writtenType The interface type usually derived from a user-written
-/// type repr. \param inferredType The type inferred by the type checker.
-void notePlaceholderReplacementTypes(Type writtenType, Type inferredType);
 } // namespace TypeChecker
 
 /// Returns the protocol requirement kind of the given declaration.
diff --git a/lib/Sema/TypeOfReference.cpp b/lib/Sema/TypeOfReference.cpp
diff --git a/test/Sema/placeholder_type.swift b/test/Sema/placeholder_type.swift
diff --git a/validation-test/compiler_crashers_2_fixed/b9c30d56ac83b29.swift b/validation-test/compiler_crashers_2_fixed/b9c30d56ac83b29.swift
diff --git a/validation-test/compiler_crashers_2_fixed/da2c588d8b81b55b.swift b/validation-test/compiler_crashers_2_fixed/da2c588d8b81b55b.swift
