Merge pull request swiftlang#39627 from CodaFi/the-replacements

Suggest Replacement Types for Invalid Placeholders

Author: CodaFi

include/swift/AST/DiagnosticsSema.def

@@ -3624,8 +3624,6 @@ ERROR(cannot_force_unwrap_nil_literal,none,
 
 ERROR(could_not_infer_placeholder,none,
       "could not infer type for placeholder", ())
-ERROR(top_level_placeholder_type,none,
-      "placeholders are not allowed as top-level types", ())
 
 ERROR(type_of_expression_is_ambiguous,none,
       "type of expression is ambiguous without more context", ())
@@ -3705,6 +3703,12 @@ 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", ())
+NOTE(replace_placeholder_with_inferred_type,none,
+     "replace the placeholder with the inferred type %0", (Type))
 
 WARNING(use_of_void_pointer,none,
 "Unsafe%0Pointer<Void> has been replaced by Unsafe%0RawPointer", (StringRef))

lib/AST/ASTContext.cpp

@@ -3644,7 +3644,6 @@ GenericFunctionType *GenericFunctionType::get(GenericSignature sig,
                                               Optional<ExtInfo> info) {
   assert(sig && "no generic signature for generic function type?!");
   assert(!result->hasTypeVariable());
-  assert(!result->hasPlaceholder());
 
   llvm::FoldingSetNodeID id;
   GenericFunctionType::Profile(id, sig, params, result, info);

lib/AST/ASTMangler.cpp

@@ -1073,14 +1073,14 @@ void ASTMangler::appendType(Type type, GenericSignature sig,
   TypeBase *tybase = type.getPointer();
   switch (type->getKind()) {
     case TypeKind::TypeVariable:
-    case TypeKind::Placeholder:
       llvm_unreachable("mangling type variable");
 
     case TypeKind::Module:
       llvm_unreachable("Cannot mangle module type yet");
 
     case TypeKind::Error:
     case TypeKind::Unresolved:
+    case TypeKind::Placeholder:
       appendOperator("Xe");
       return;
 

lib/Sema/CSApply.cpp

@@ -8643,6 +8643,7 @@ ExprWalker::rewriteTarget(SolutionApplicationTarget target) {
     Type convertType = target.getExprConversionType();
     auto shouldCoerceToContextualType = [&]() {
       return convertType &&
+          !convertType->hasPlaceholder() &&
           !target.isOptionalSomePatternInit() &&
           !(solution.getType(resultExpr)->isUninhabited() &&
             cs.getContextualTypePurpose(target.getAsExpr())

lib/Sema/CSGen.cpp

@@ -1326,6 +1326,11 @@ namespace {
       if (result->hasError()) {
         return Type();
       }
+      // Diagnose top-level usages of placeholder types.
+      if (isa<TopLevelCodeDecl>(CS.DC) && isa<PlaceholderTypeRepr>(repr)) {
+        CS.getASTContext().Diags.diagnose(repr->getLoc(),
+                                          diag::placeholder_type_not_allowed);
+      }
       return result;
     }
 

lib/Sema/MiscDiagnostics.cpp

@@ -2712,6 +2712,65 @@ 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);
+    }
+  }
+
+  std::pair<bool, Expr *> walkToExprPre(Expr *E) override { return {true, E}; }
+
+  std::pair<bool, 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 {true, S};
+  }
+
+  // Don't descend into nested decls.
+  bool walkToDeclPre(Decl *D) override { return false; }
+};
+
 } // end anonymous namespace
 
 // After we have scanned the entire region, diagnose variables that could be
@@ -3277,6 +3336,13 @@ 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/PreCheckExpr.cpp

@@ -1506,9 +1506,7 @@ TypeExpr *PreCheckExpression::simplifyNestedTypeExpr(UnresolvedDotExpr *UDE) {
         },
         // FIXME: Don't let placeholder types escape type resolution.
         // For now, just return the placeholder type.
-        [](auto &ctx, auto *originator) {
-          return Type();
-        });
+        PlaceholderType::get);
     const auto BaseTy = resolution.resolveType(InnerTypeRepr);
 
     if (BaseTy->mayHaveMembers()) {
@@ -2061,9 +2059,7 @@ Expr *PreCheckExpression::simplifyTypeConstructionWithLiteralArg(Expr *E) {
         },
         // FIXME: Don't let placeholder types escape type resolution.
         // For now, just return the placeholder type.
-        [](auto &ctx, auto *originator) {
-          return Type();
-        });
+        PlaceholderType::get);
     const auto result = resolution.resolveType(typeExpr->getTypeRepr());
     if (result->hasError())
       return nullptr;

lib/Sema/TypeCheckDecl.cpp

@@ -2213,8 +2213,9 @@ ResultTypeRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
   const auto options =
       TypeResolutionOptions(TypeResolverContext::FunctionResult);
   auto *const dc = decl->getInnermostDeclContext();
-  return TypeResolution::forInterface(dc, options, /*unboundTyOpener*/ nullptr,
-                                      /*placeholderHandler*/ nullptr)
+  return TypeResolution::forInterface(dc, options,
+                                      /*unboundTyOpener*/ nullptr,
+                                      PlaceholderType::get)
       .resolveType(resultTyRepr);
 }
 
@@ -2298,7 +2299,6 @@ static Type validateParameterType(ParamDecl *decl) {
 
   TypeResolutionOptions options(None);
   OpenUnboundGenericTypeFn unboundTyOpener = nullptr;
-  HandlePlaceholderTypeReprFn placeholderHandler = nullptr;
   if (isa<AbstractClosureExpr>(dc)) {
     options = TypeResolutionOptions(TypeResolverContext::ClosureExpr);
     options |= TypeResolutionFlags::AllowUnspecifiedTypes;
@@ -2309,9 +2309,6 @@ static Type validateParameterType(ParamDecl *decl) {
     };
     // FIXME: Don't let placeholder types escape type resolution.
     // For now, just return the placeholder type.
-    placeholderHandler = [](auto &ctx, auto *originator) {
-      return Type();
-    };
   } else if (isa<AbstractFunctionDecl>(dc)) {
     options = TypeResolutionOptions(TypeResolverContext::AbstractFunctionDecl);
   } else if (isa<SubscriptDecl>(dc)) {
@@ -2334,7 +2331,7 @@ static Type validateParameterType(ParamDecl *decl) {
 
   const auto resolution =
       TypeResolution::forInterface(dc, options, unboundTyOpener,
-                                   placeholderHandler);
+                                   PlaceholderType::get);
   auto Ty = resolution.resolveType(decl->getTypeRepr());
 
   if (Ty->hasError()) {
@@ -2950,9 +2947,7 @@ ExtendedTypeRequest::evaluate(Evaluator &eval, ExtensionDecl *ext) const {
       },
       // FIXME: Don't let placeholder types escape type resolution.
       // For now, just return the placeholder type.
-      [](auto &ctx, auto *originator) {
-        return Type();
-      });
+      PlaceholderType::get);
 
   const auto extendedType = resolution.resolveType(extendedRepr);
 

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -18,19 +18,19 @@
 
 #include "CodeSynthesis.h"
 #include "DerivedConformances.h"
-#include "TypeChecker.h"
+#include "MiscDiagnostics.h"
 #include "TypeCheckAccess.h"
+#include "TypeCheckAvailability.h"
 #include "TypeCheckConcurrency.h"
 #include "TypeCheckDecl.h"
-#include "TypeCheckAvailability.h"
 #include "TypeCheckObjC.h"
 #include "TypeCheckType.h"
-#include "MiscDiagnostics.h"
-#include "swift/AST/AccessNotes.h"
-#include "swift/AST/AccessScope.h"
+#include "TypeChecker.h"
 #include "swift/AST/ASTPrinter.h"
 #include "swift/AST/ASTVisitor.h"
 #include "swift/AST/ASTWalker.h"
+#include "swift/AST/AccessNotes.h"
+#include "swift/AST/AccessScope.h"
 #include "swift/AST/ExistentialLayout.h"
 #include "swift/AST/Expr.h"
 #include "swift/AST/ForeignErrorConvention.h"
@@ -42,15 +42,15 @@
 #include "swift/AST/PropertyWrappers.h"
 #include "swift/AST/ProtocolConformance.h"
 #include "swift/AST/SourceFile.h"
+#include "swift/AST/TypeCheckRequests.h"
+#include "swift/AST/TypeDifferenceVisitor.h"
 #include "swift/AST/TypeWalker.h"
+#include "swift/Basic/Defer.h"
 #include "swift/Basic/Statistic.h"
 #include "swift/Parse/Lexer.h"
 #include "swift/Parse/Parser.h"
 #include "swift/Serialization/SerializedModuleLoader.h"
 #include "swift/Strings.h"
-#include "swift/AST/NameLookupRequests.h"
-#include "swift/AST/TypeCheckRequests.h"
-#include "swift/Basic/Defer.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/APSInt.h"
@@ -967,11 +967,65 @@ static void checkInheritedDefaultValueRestrictions(ParamDecl *PD) {
   }
 }
 
+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<PlaceholderTypeRepr *>()) {
+        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 pattern.
 static void checkDefaultArguments(ParameterList *params) {
   // Force the default values in case they produce diagnostics.
-  for (auto *param : *params)
-    (void)param->getTypeCheckedDefaultExpr();
+  for (auto *param : *params) {
+    auto ifacety = param->getInterfaceType();
+    auto *expr = param->getTypeCheckedDefaultExpr();
+    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());
+    }
+  }
 }
 
 Expr *DefaultArgumentExprRequest::evaluate(Evaluator &evaluator,

lib/Sema/TypeCheckPattern.cpp

@@ -494,9 +494,7 @@ class ResolvePattern : public ASTVisitor<ResolvePattern,
         },
         // FIXME: Don't let placeholder types escape type resolution.
         // For now, just return the placeholder type.
-        [](auto &ctx, auto *originator) {
-          return Type();
-        });
+        PlaceholderType::get);
     const auto ty = resolution.resolveType(repr);
     auto *enumDecl = dyn_cast_or_null<EnumDecl>(ty->getAnyNominal());
     if (!enumDecl)
@@ -618,9 +616,7 @@ class ResolvePattern : public ASTVisitor<ResolvePattern,
               },
               // FIXME: Don't let placeholder types escape type resolution.
               // For now, just return the placeholder type.
-              [](auto &ctx, auto *originator) {
-                return Type();
-              })
+              PlaceholderType::get)
               .resolveType(prefixRepr);
       auto *enumDecl = dyn_cast_or_null<EnumDecl>(enumTy->getAnyNominal());
       if (!enumDecl)
@@ -830,9 +826,7 @@ Type PatternTypeRequest::evaluate(Evaluator &evaluator,
       };
       // FIXME: Don't let placeholder types escape type resolution.
       // For now, just return the placeholder type.
-      placeholderHandler = [](auto &ctx, auto *originator) {
-        return Type();
-      };
+      placeholderHandler = PlaceholderType::get;
     }
     return validateTypedPattern(
         cast<TypedPattern>(P),
@@ -900,9 +894,7 @@ Type PatternTypeRequest::evaluate(Evaluator &evaluator,
         };
         // FIXME: Don't let placeholder types escape type resolution.
         // For now, just return the placeholder type.
-        placeholderHandler = [](auto &ctx, auto *originator) {
-          return Type();
-        };
+        placeholderHandler = PlaceholderType::get;
       }
       TypedPattern *TP = cast<TypedPattern>(somePat->getSubPattern());
       const auto type = validateTypedPattern(