[Sema] Reject placeholders in type resolution for param and result types

Not all clients can properly handle the presence of placeholders in
interface types and it doesn't seem worth the complexity for the 
type replacement diagnostic.

Author: hamishknight

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,

lib/AST/TypeCheckRequests.cpp

@@ -1227,6 +1227,11 @@ void InterfaceTypeRequest::cacheResult(Type type) const {
     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);
 }

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

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,

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

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.

test/Sema/placeholder_type.swift

@@ -10,8 +10,7 @@ let dict2: [Character: _] = ["h": 0]
 
 let arr = [_](repeating: "hi", count: 3)
 
-func foo(_ arr: [_] = [0]) {} // expected-error {{type placeholder may not appear in top-level parameter}}
-// expected-note@-1 {{replace the placeholder with the inferred type 'Int'}}
+func foo(_ arr: [_] = [0]) {} // expected-error {{type placeholder not allowed here}}
 
 let foo = _.foo // expected-error {{type placeholder not allowed here}}
 let zero: _ = .zero // expected-error {{cannot infer contextual base in reference to member 'zero'}}
@@ -78,33 +77,25 @@ where T: ExpressibleByIntegerLiteral, U: ExpressibleByIntegerLiteral {
 }
 
 extension Bar {
-  func frobnicate2() -> Bar<_, _> { // expected-error {{type placeholder may not appear in function return type}}
-    // expected-note@-1 {{replace the placeholder with the inferred type 'T'}}
-    // expected-note@-2 {{replace the placeholder with the inferred type 'U'}}
+  func frobnicate2() -> Bar<_, _> { // expected-error {{type placeholder not allowed here}}
     return Bar(t: 42, u: 42)
   }
   func frobnicate3() -> Bar {
     return Bar<_, _>(t: 42, u: 42)
   }
-  func frobnicate4() -> Bar<_, _> { // expected-error {{type placeholder may not appear in function return type}}
-    // expected-note@-1 {{replace the placeholder with the inferred type 'Int'}}
-    // expected-note@-2 {{replace the placeholder with the inferred type 'Int'}}
+  func frobnicate4() -> Bar<_, _> { // expected-error {{type placeholder not allowed here}}
     return Bar<_, _>(t: 42, u: 42)
   }
-  func frobnicate5() -> Bar<_, U> { // expected-error {{type placeholder may not appear in function return type}}
-    // expected-note@-1 {{replace the placeholder with the inferred type 'T'}}
+  func frobnicate5() -> Bar<_, U> { // expected-error {{type placeholder not allowed here}}
     return Bar(t: 42, u: 42)
   }
   func frobnicate6() -> Bar {
     return Bar<_, U>(t: 42, u: 42)
   }
-  func frobnicate7() -> Bar<_, _> { // expected-error {{type placeholder may not appear in function return type}}
-    // expected-note@-1 {{replace the placeholder with the inferred type 'Int'}}
-    // expected-note@-2 {{replace the placeholder with the inferred type 'U'}}
+  func frobnicate7() -> Bar<_, _> { // expected-error {{type placeholder not allowed here}}
     return Bar<_, U>(t: 42, u: 42)
   }
-  func frobnicate8() -> Bar<_, U> { // expected-error {{type placeholder may not appear in function return type}}
-    // expected-note@-1 {{replace the placeholder with the inferred type 'Int'}}
+  func frobnicate8() -> Bar<_, U> { // expected-error {{type placeholder not allowed here}}
     return Bar<_, _>(t: 42, u: 42)
   }
 }
@@ -244,11 +235,11 @@ let _: SetFailureType<Int, String> = Just<Int>().setFailureType(to: _.self).setF
 // diagnostic.
 func mismatchedDefault<T>(_ x: [_] = [String: T]()) {} // expected-error {{type placeholder not allowed here}}
 
-func mismatchedReturnTypes() -> _ { // expected-error {{type placeholder may not appear in function return type}}
+func mismatchedReturnTypes() -> _ { // expected-error {{type placeholder not allowed here}}
   if true {
-    return "" // expected-note@-2 {{replace the placeholder with the inferred type 'String'}}
+    return ""
   } else {
-    return 0.5 // expected-note@-4 {{replace the placeholder with the inferred type 'Double'}}
+    return 0.5
   }
 }
 
@@ -261,9 +252,8 @@ func opaque() -> some _ { // expected-error {{type placeholder not allowed here}
 }
 
 enum EnumWithPlaceholders {
-  case topLevelPlaceholder(x: _) // expected-error {{type placeholder may not appear in top-level parameter}}
-  case placeholderWithDefault(x: _ = 5) // expected-error {{type placeholder may not appear in top-level parameter}}
-  // expected-note@-1 {{replace the placeholder with the inferred type 'Int'}}
+  case topLevelPlaceholder(x: _) // expected-error {{type placeholder not allowed here}}
+  case placeholderWithDefault(x: _ = 5) // expected-error {{type placeholder not allowed here}}
 }
 
 func deferredInit(_ c: Bool) {
@@ -303,30 +293,29 @@ protocol TestPlaceholderRequirement {
   subscript() -> _ { get } // expected-error {{type placeholder not allowed here}}
 }
 
-func testPlaceholderFn1(_:_) {} // expected-error {{type placeholder may not appear in top-level parameter}}
-func testPlaceholderFn2() -> _ {} // expected-error {{type placeholder may not appear in function return type}}
+func testPlaceholderFn1(_:_) {} // expected-error {{type placeholder not allowed here}}
+func testPlaceholderFn2() -> _ {} // expected-error {{type placeholder not allowed here}}
 
 var testPlaceholderComputed1: _ { 0 } // expected-error {{type placeholder not allowed here}}
 var testPlaceholderComputed2: [_] { [0] } // expected-error {{type placeholder not allowed here}}
 
 struct TestPlaceholderSubscript {
-  // FIXME: Shouldn't diagnose twice.
-  subscript(_: _) -> Void { () } // expected-error 2{{type placeholder may not appear in top-level parameter}}
-  subscript(_: [_]) -> Void { () } // expected-error 2{{type placeholder may not appear in top-level parameter}}
+  subscript(_: _) -> Void { () } // expected-error {{type placeholder not allowed here}}
+  subscript(_: [_]) -> Void { () } // expected-error {{type placeholder not allowed here}}
   subscript() -> _ { () } // expected-error {{type placeholder not allowed here}}
   subscript() -> [_] { [0] } // expected-error {{type placeholder not allowed here}}
 }
 
 enum TestPlaceholderInEnumElement {
-  case a(_) // expected-error {{type placeholder may not appear in top-level parameter}}
-  case b([_]) // expected-error {{type placeholder may not appear in top-level parameter}}
+  case a(_) // expected-error {{type placeholder not allowed here}}
+  case b([_]) // expected-error {{type placeholder not allowed here}}
 }
 
 @freestanding(expression) macro testPlaceholderMacro(_ x: _) -> String = #file
-// expected-error@-1 {{type placeholder may not appear in top-level parameter}}
+// expected-error@-1 {{type placeholder not allowed here}}
 
 @freestanding(expression) macro testPlaceholderMacro(_ x: [_]) -> String = #file
-// expected-error@-1 {{type placeholder may not appear in top-level parameter}}
+// expected-error@-1 {{type placeholder not allowed here}}
 
 @freestanding(expression) macro testPlaceholderMacro() -> _ = #file
 // expected-error@-1 {{type placeholder not allowed here}}
@@ -351,8 +340,10 @@ func usePlaceholderDecls(
   _ = TestPlaceholderInEnumElement.a(0)
   _ = TestPlaceholderInEnumElement.b([])
 
-  _ = #testPlaceholderMacro(0)
-  _ = #testPlaceholderMacro([])
+  // There are marked invalid so get removed from the overload set.
+  // FIXME: We ought to turn them into holes instead
+  _ = #testPlaceholderMacro(0) // expected-error {{no macro named 'testPlaceholderMacro'}}
+  _ = #testPlaceholderMacro([]) // expected-error {{no macro named 'testPlaceholderMacro'}}
 
   _ = testPlaceholderFn1(0)
   _ = testPlaceholderFn2()

validation-test/compiler_crashers_2_fixed/b9c30d56ac83b29.swift

@@ -0,0 +1,6 @@
+// {"kind":"typecheck","original":"1bb3e6fd","signature":"swift::constraints::ConstraintSystem::applySolution(swift::constraints::Solution&, swift::constraints::SyntacticElementTarget)","signatureAssert":"Assertion failed: (isValidType(type) && \"type binding has invalid type\"), function applySolution"}
+// RUN: not %target-swift-frontend -typecheck %s
+@propertyWrapper struct a<b {
+  wrappedValue: b
+}
+func c(@a _ )