Merge pull request #84345 from hamishknight/time-and-place

A few fixes for placeholder types in interface types

Author: hamishknight

include/swift/AST/Decl.h

@@ -7101,6 +7101,14 @@ class ParamDecl : public VarDecl {
       Identifier parameterName, Expr *defaultValue,
       DefaultArgumentInitializer *defaultValueInitContext, DeclContext *dc);
 
+  SourceLoc getLocFromSource() const {
+    // If we have a name loc, use it, otherwise fallback to the start loc for
+    // e.g enum elements without parameter names.
+    if (auto nameLoc = getNameLoc())
+      return nameLoc;
+    return getStartLoc();
+  }
+
   /// Retrieve the argument (API) name for this function parameter.
   Identifier getArgumentName() const {
     return ArgumentNameAndFlags.getPointer();

include/swift/AST/TypeMatcher.h

@@ -117,6 +117,12 @@ 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.

include/swift/Sema/CSFix.h

@@ -362,6 +362,9 @@ 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,
@@ -3191,6 +3194,30 @@ 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) {}

lib/AST/ASTContext.cpp

@@ -4686,6 +4686,8 @@ isAnyFunctionTypeCanonical(ArrayRef<AnyFunctionType::Param> params,
 // For now, generic function types cannot be dependent (in fact,
 // they erase dependence) or contain type variables, and they're
 // always materializable.
+// FIXME: This doesn't seem great, we should consider changing it to be opt-out
+// rather than opt-in.
 static RecursiveTypeProperties
 getGenericFunctionRecursiveProperties(ArrayRef<AnyFunctionType::Param> params,
                                       Type result, Type globalActor,
@@ -4694,34 +4696,25 @@ getGenericFunctionRecursiveProperties(ArrayRef<AnyFunctionType::Param> params,
                 "revisit this if you add new recursive type properties");
   RecursiveTypeProperties properties;
 
-  for (auto param : params) {
-    if (param.getPlainType()->getRecursiveProperties().hasError())
-      properties |= RecursiveTypeProperties::HasError;
-    if (param.getPlainType()->getRecursiveProperties().isUnsafe())
-      properties |= RecursiveTypeProperties::IsUnsafe;
-  }
+  using Prop = RecursiveTypeProperties::Property;
+  auto mask = (unsigned)Prop::HasError | Prop::IsUnsafe | Prop::HasPlaceholder;
+
+  auto unionBits = [&](Type ty) {
+    if (!ty)
+      return;
+    auto bits = ty->getRecursiveProperties().getBits();
+    properties |= Prop(bits & mask);
+  };
+
+  for (auto param : params)
+    unionBits(param.getPlainType());
 
   if (result->getRecursiveProperties().hasDynamicSelf())
     properties |= RecursiveTypeProperties::HasDynamicSelf;
-  if (result->getRecursiveProperties().hasError())
-    properties |= RecursiveTypeProperties::HasError;
-  if (result->getRecursiveProperties().isUnsafe())
-    properties |= RecursiveTypeProperties::IsUnsafe;
-  
-  if (globalActor) {
-    if (globalActor->getRecursiveProperties().hasError())
-      properties |= RecursiveTypeProperties::HasError;
-    if (globalActor->getRecursiveProperties().isUnsafe())
-      properties |= RecursiveTypeProperties::IsUnsafe;
-  }
-
-  if (thrownError) {
-    if (thrownError->getRecursiveProperties().hasError())
-      properties |= RecursiveTypeProperties::HasError;
-    if (thrownError->getRecursiveProperties().isUnsafe())
-      properties |= RecursiveTypeProperties::IsUnsafe;
-  }
 
+  unionBits(result);
+  unionBits(globalActor);
+  unionBits(thrownError);
   return properties;
 }
 

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, don't look at it further, but proceed
-      // to find other matches.
-      if (secondType->hasError())
+      // 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())
         return true;
 
       // If the second type is a generic parameter of the witness, the match

lib/Sema/CSFix.cpp

@@ -2264,6 +2264,20 @@ 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());

lib/Sema/CSGen.cpp

@@ -2542,8 +2542,11 @@ namespace {
       Type thrownErrorTy = [&] {
         // Explicitly-specified thrown type.
         if (closure->getExplicitThrownTypeRepr()) {
-          if (Type explicitType = closure->getExplicitThrownType())
-            return explicitType;
+          if (Type explicitType = closure->getExplicitThrownType()) {
+            // The thrown type may have errors, open as placeholders if needed.
+            return CS.replaceInferableTypesWithTypeVars(explicitType,
+                                                        thrownErrorLocator);
+          }
         }
 
         // Explicitly-specified 'throws' without a type is untyped throws.

lib/Sema/CSSimplify.cpp

@@ -16109,6 +16109,7 @@ 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:

lib/Sema/TypeCheckDecl.cpp

@@ -2121,10 +2121,17 @@ 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,
-                                      PlaceholderType::get,
+                                      placeholderOpener,
                                       /*packElementOpener*/ nullptr)
       .resolveType(resultTyRepr);
 }
@@ -2310,9 +2317,17 @@ 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,
-                                   PlaceholderType::get,
+                                   placeholderOpener,
                                    /*packElementOpener*/ nullptr);
 
   if (isa<VarargTypeRepr>(nestedRepr)) {
@@ -3026,9 +3041,11 @@ bool TypeChecker::isPassThroughTypealias(TypeAliasDecl *typealias,
 
 Type
 ExtendedTypeRequest::evaluate(Evaluator &eval, ExtensionDecl *ext) const {
-  auto error = [&ext]() {
+  auto &ctx = ext->getASTContext();
+
+  auto error = [&]() {
     ext->setInvalid();
-    return ErrorType::get(ext->getASTContext());
+    return ErrorType::get(ctx);
   };
 
   // If we didn't parse a type, fill in an error type and bail out.
@@ -3052,6 +3069,15 @@ ExtendedTypeRequest::evaluate(Evaluator &eval, ExtensionDecl *ext) const {
   if (extendedType->hasError())
     return error();
 
+  auto &diags = ctx.Diags;
+
+  // Cannot extend types who contain placeholders.
+  if (extendedType->hasPlaceholder()) {
+    diags.diagnose(ext->getLoc(), diag::extension_placeholder)
+      .highlight(extendedRepr->getSourceRange());
+    return error();
+  }
+
   // Hack to allow extending a generic typealias.
   if (auto *unboundGeneric = extendedType->getAs<UnboundGenericType>()) {
     if (auto *aliasDecl = dyn_cast<TypeAliasDecl>(unboundGeneric->getDecl())) {
@@ -3069,8 +3095,6 @@ ExtendedTypeRequest::evaluate(Evaluator &eval, ExtensionDecl *ext) const {
     }
   }
 
-  auto &diags = ext->getASTContext().Diags;
-
   // Cannot extend a metatype.
   if (extendedType->is<AnyMetatypeType>()) {
     diags.diagnose(ext->getLoc(), diag::extension_metatype, extendedType)
@@ -3087,13 +3111,6 @@ ExtendedTypeRequest::evaluate(Evaluator &eval, ExtensionDecl *ext) const {
     return error();
   }
 
-  // Cannot extend types who contain placeholders.
-  if (extendedType->hasPlaceholder()) {
-    diags.diagnose(ext->getLoc(), diag::extension_placeholder)
-      .highlight(extendedRepr->getSourceRange());
-    return error();
-  }
-
   return extendedType;
 }
 

lib/Sema/TypeCheckType.cpp

@@ -6999,7 +6999,7 @@ Type ExplicitCaughtTypeRequest::evaluate(
 
     return TypeResolution::forInterface(func, options,
                                         /*unboundTyOpener*/ nullptr,
-                                        PlaceholderType::get,
+                                        /*placeholderOpener*/ nullptr,
                                         /*packElementOpener*/ nullptr)
         .resolveType(thrownTypeRepr);
   }
@@ -7011,7 +7011,7 @@ Type ExplicitCaughtTypeRequest::evaluate(
       return TypeResolution::resolveContextualType(
                thrownTypeRepr, closure,
                TypeResolutionOptions(TypeResolverContext::None),
-               /*unboundTyOpener*/ nullptr, PlaceholderType::get,
+               /*unboundTyOpener*/ nullptr, /*placeholderOpener*/ nullptr,
                /*packElementOpener*/ nullptr);
     }
 
@@ -7045,7 +7045,7 @@ Type ExplicitCaughtTypeRequest::evaluate(
     return TypeResolution::resolveContextualType(
         typeRepr, doCatch->getDeclContext(),
         TypeResolutionOptions(TypeResolverContext::None),
-        /*unboundTyOpener*/ nullptr, PlaceholderType::get,
+        /*unboundTyOpener*/ nullptr, /*placeholderOpener*/ nullptr,
         /*packElementOpener*/ nullptr);
   }