Merge pull request #78705 from jckarter/explicit-conditional-invertible-extension-requirements

Require explicit statement of all `Copyable`/`Escapable` requirements for conditional `Copyable`/`Escapable` conformances.

Author: jckarter

include/swift/AST/Decl.h

@@ -2090,7 +2090,12 @@ class ExtensionDecl final : public GenericContext, public Decl,
 
   /// Does this extension add conformance to an invertible protocol for the
   /// extended type?
-  bool isAddingConformanceToInvertible() const;
+  ///
+  /// Returns \c nullopt if the extension does not add conformance to any
+  /// invertible protocol. Returns one of the invertible protocols being
+  /// conformed to otherwise.
+  std::optional<InvertibleProtocolKind>
+  isAddingConformanceToInvertible() const;
 
   /// If this extension represents an imported Objective-C category, returns the
   /// category's name. Otherwise returns the empty identifier.

include/swift/AST/DiagnosticsSema.def

@@ -7864,6 +7864,16 @@ ERROR(extension_conforms_to_invertible_and_others, none,
 ERROR(invertible_conformance_other_source_file,none,
       "conformance to '%0' must occur in the same source file as %kind1",
       (StringRef, const ValueDecl *))
+ERROR(inverse_conditional_must_be_fully_explicit,none,
+        "conditional conformance to %0 must explicitly "
+        "state whether %1 is required to conform to %2 or not",
+        (const ProtocolDecl *, Type, const ProtocolDecl *))
+NOTE(make_inverse_conditional_non_requirement_explicit,none,
+     "specify that the conformance does not require %0 to conform to %1",
+     (Type, const ProtocolDecl *))
+NOTE(make_inverse_conditional_requirement_explicit,none,
+     "specify that the conformance requires %0 to conform to %1",
+     (Type, const ProtocolDecl *))
 
 // -- older ones below --
 ERROR(noncopyable_parameter_requires_ownership, none,

include/swift/AST/TypeCheckRequests.h

@@ -2118,7 +2118,8 @@ class InferredGenericSignatureRequest :
                                                     WhereClauseOwner,
                                                     SmallVector<Requirement, 2>,
                                                     SmallVector<TypeBase *, 2>,
-                                                    SourceLoc, bool, bool),
+                                                    SourceLoc, ExtensionDecl *,
+                                                    bool),
                          RequestFlags::Uncached> {
 public:
   using SimpleRequest::SimpleRequest;
@@ -2134,7 +2135,8 @@ class InferredGenericSignatureRequest :
            WhereClauseOwner whereClause,
            SmallVector<Requirement, 2> addedRequirements,
            SmallVector<TypeBase *, 2> inferenceSources,
-           SourceLoc loc, bool isExtension, bool allowInverses) const;
+           SourceLoc loc, ExtensionDecl *forExtension,
+           bool allowInverses) const;
 
 public:
   /// Inferred generic signature requests don't have source-location info.

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -206,7 +206,7 @@ SWIFT_REQUEST(TypeChecker, InferredGenericSignatureRequest,
                                          WhereClauseOwner,
                                          SmallVector<Requirement, 2>,
                                          SmallVector<TypeBase *, 2>,
-                                         SourceLoc, bool, bool),
+                                         SourceLoc, ExtensionDecl *, bool),
               Uncached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, DistributedModuleIsAvailableRequest,
               bool(const ValueDecl *), Cached, NoLocationInfo)

lib/AST/ASTPrinter.cpp

@@ -1004,6 +1004,7 @@ class PrintAST : public ASTVisitor<PrintAST> {
     SwapSelfAndDependentMemberType = 8,
     PrintInherited = 16,
     PrintInverseRequirements = 32,
+    PrintExplicitInvertibleRequirements = 64,
   };
 
   /// The default generic signature flags for printing requirements.
@@ -1773,7 +1774,18 @@ void PrintAST::printGenericSignature(
   SmallVector<Requirement, 2> requirements;
   SmallVector<InverseRequirement, 2> inverses;
 
-  if (flags & PrintInverseRequirements) {
+  if (flags & PrintExplicitInvertibleRequirements) {
+    // Collect the inverted requirements…
+    SmallVector<Requirement, 2> filteredRequirements;
+    genericSig->getRequirementsWithInverses(filteredRequirements, inverses);
+    llvm::erase_if(inverses, [&](InverseRequirement inverse) -> bool {
+      return !inverseFilter(inverse);
+    });
+    // …and also all of the unfiltered requirements, including the ones for
+    // invertible protocols.
+    requirements.append(genericSig.getRequirements().begin(),
+                        genericSig.getRequirements().end());
+  } else if (flags & PrintInverseRequirements) {
     genericSig->getRequirementsWithInverses(requirements, inverses);
     llvm::erase_if(inverses, [&](InverseRequirement inverse) -> bool {
       return !inverseFilter(inverse);
@@ -3123,7 +3135,7 @@ void PrintAST::printExtension(ExtensionDecl *decl) {
       // in such an extension. We need to print the whole signature:
       //     extension S: Copyable where T: Copyable
       if (decl->isAddingConformanceToInvertible())
-        genSigFlags &= ~PrintInverseRequirements;
+        genSigFlags |= PrintExplicitInvertibleRequirements;
 
       printGenericSignature(genericSig,
                             genSigFlags,

lib/AST/Decl.cpp

@@ -1987,7 +1987,8 @@ Type ExtensionDecl::getExtendedType() const {
   return ErrorType::get(ctx);
 }
 
-bool ExtensionDecl::isAddingConformanceToInvertible() const {
+std::optional<InvertibleProtocolKind>
+ExtensionDecl::isAddingConformanceToInvertible() const {
   const unsigned numEntries = getInherited().size();
   for (unsigned i = 0; i < numEntries; ++i) {
     InheritedTypeRequest request{this, i, TypeResolutionStage::Structural};
@@ -2005,10 +2006,10 @@ bool ExtensionDecl::isAddingConformanceToInvertible() const {
 
     if (inheritedTy)
       if (auto kp = inheritedTy->getKnownProtocol())
-        if (getInvertibleProtocolKind(*kp))
-          return true;
+        if (auto kind = getInvertibleProtocolKind(*kp))
+          return kind;
   }
-  return false;
+  return std::nullopt;
 }
 
 bool Decl::isObjCImplementation() const {

lib/AST/RequirementMachine/RequirementMachineRequests.cpp

@@ -776,15 +776,15 @@ InferredGenericSignatureRequest::evaluate(
         WhereClauseOwner whereClause,
         SmallVector<Requirement, 2> addedRequirements,
         SmallVector<TypeBase *, 2> inferenceSources,
-        SourceLoc loc, bool isExtension, bool allowInverses) const {
+        SourceLoc loc, ExtensionDecl *forExtension, bool allowInverses) const {
   GenericSignature parentSig(parentSigImpl);
 
   SmallVector<GenericTypeParamType *, 4> genericParams(
       parentSig.getGenericParams().begin(),
       parentSig.getGenericParams().end());
 
   unsigned numOuterParams = genericParams.size();
-  if (isExtension) {
+  if (forExtension) {
     numOuterParams = 0;
   }
 
@@ -889,6 +889,68 @@ InferredGenericSignatureRequest::evaluate(
   SmallVector<StructuralRequirement, 2> defaults;
   InverseRequirement::expandDefaults(ctx, paramTypes, defaults);
   applyInverses(ctx, paramTypes, inverses, defaults, errors);
+  
+  // Any remaining implicit defaults in a conditional inverse requirement
+  // extension must be made explicit.
+  if (forExtension) {
+    auto invertibleProtocol = forExtension->isAddingConformanceToInvertible();
+    // FIXME: to workaround a reverse condfail, always infer the requirements if
+    //  the extension is in a swiftinterface file. This is temporary and should
+    //  be removed soon. (rdar://130424971)
+    if (auto *sf = forExtension->getOutermostParentSourceFile()) {
+      if (sf->Kind == SourceFileKind::Interface
+          && !ctx.LangOpts.hasFeature(Feature::SE427NoInferenceOnExtension)) {
+        invertibleProtocol = std::nullopt;
+      }
+    }
+    if (invertibleProtocol) {
+      for (auto &def : defaults) {
+        // Check whether a corresponding explicit requirement was provided.
+        for (auto &req : requirements) {
+          // An explicit requirement can match the default exactly.
+          if (req.req.getCanonical() == def.req.getCanonical()) {
+            goto next;
+          }
+          
+          // Disregard requirements on other parameters.
+          if (!req.req.getFirstType()->isEqual(def.req.getFirstType())) {
+            continue;
+          }
+          
+          // Or it can be implied by a requirement on something that's inherently
+          // copyable.
+          if (req.req.getKind() == RequirementKind::Superclass) {
+            // classes are currently always escapable and copyable
+            goto next;
+          }
+          if (req.req.getKind() == RequirementKind::Layout) {
+            // layout constraints currently always imply escapable and copyable
+            goto next;
+          }
+          if (req.req.getKind() == RequirementKind::Conformance
+              && req.req.getProtocolDecl()
+                        ->inheritsFrom(def.req.getProtocolDecl())) {
+            goto next;
+          }
+
+          // A same-type constraint removes the ability for the copyability
+          // to vary independently at all.
+          if (req.req.getKind() == RequirementKind::SameType) {
+            goto next;
+          }
+        }
+        ctx.Diags.diagnose(loc,diag::inverse_conditional_must_be_fully_explicit,
+           ctx.getProtocol(getKnownProtocolKind(*invertibleProtocol)),
+                           def.req.getFirstType(),
+                           def.req.getProtocolDecl());
+      next:;
+      }
+      // Don't actually apply the inferred requirements since they should be
+      // stated explicitly.
+      defaults.clear();
+    }
+  }
+  
   requirements.append(defaults);
 
   auto &rewriteCtx = ctx.getRewriteContext();
@@ -954,7 +1016,7 @@ InferredGenericSignatureRequest::evaluate(
     if (attempt == 0) {
       machine->computeRequirementDiagnostics(errors, inverses, loc);
       diagnoseRequirementErrors(ctx, errors,
-                                (isExtension || !genericParamList)
+                                (forExtension || !genericParamList)
                                 ? AllowConcreteTypePolicy::All
                                 : AllowConcreteTypePolicy::AssocTypes);
     }
@@ -985,7 +1047,7 @@ InferredGenericSignatureRequest::evaluate(
                                            std::move(machine));
     }
 
-    if (genericParamList && !isExtension) {
+    if (genericParamList && !forExtension) {
       for (auto genericParam : result.getInnermostGenericParams()) {
         auto reduced = result.getReducedType(genericParam);
 

lib/Sema/TypeCheckAttr.cpp

@@ -3564,7 +3564,7 @@ SerializeAttrGenericSignatureRequest::evaluate(Evaluator &evaluator,
       /*addedRequirements=*/{},
       /*inferenceSources=*/{},
       attr->getLocation(),
-      /*isExtension=*/false,
+      /*forExtension=*/nullptr,
       /*allowInverses=*/true};
 
   auto specializedSig = evaluateOrDefault(Ctx.evaluator, request,
@@ -6161,7 +6161,7 @@ bool resolveDifferentiableAttrDerivativeGenericSignature(
         /*addedRequirements=*/{},
         /*inferenceSources=*/{},
         attr->getLocation(),
-        /*isExtension=*/false,
+        /*forExtension=*/nullptr,
         /*allowInverses=*/true};
 
     // Compute generic signature for derivative functions.

lib/Sema/TypeCheckGeneric.cpp

@@ -119,7 +119,7 @@ OpaqueResultTypeRequest::evaluate(Evaluator &evaluator,
         /*addedRequirements=*/{},
         /*inferenceSources=*/{},
         repr->getLoc(),
-        /*isExtension=*/false,
+        /*forExtension=*/nullptr,
         /*allowInverses=*/true};
 
     interfaceSignature = evaluateOrDefault(
@@ -689,7 +689,6 @@ GenericSignatureRequest::evaluate(Evaluator &evaluator,
   SmallVector<TypeBase *, 2> inferenceSources;
   SmallVector<Requirement, 2> extraReqs;
   SourceLoc loc;
-  bool inferInvertibleReqs = true;
 
   if (auto VD = dyn_cast<ValueDecl>(GC->getAsDecl())) {
     loc = VD->getLoc();
@@ -787,20 +786,6 @@ GenericSignatureRequest::evaluate(Evaluator &evaluator,
   } else if (auto *ext = dyn_cast<ExtensionDecl>(GC)) {
     loc = ext->getLoc();
 
-    // If the extension introduces conformance to invertible protocol IP, do not
-    // infer any conditional requirements that the generic parameters to conform
-    // to invertible protocols. This forces people to write out the conditions.
-    inferInvertibleReqs = !ext->isAddingConformanceToInvertible();
-
-    // FIXME: to workaround a reverse condfail, always infer the requirements if
-    //  the extension is in a swiftinterface file. This is temporary and should
-    //  be removed soon. (rdar://130424971)
-    if (auto *sf = ext->getOutermostParentSourceFile()) {
-      if (sf->Kind == SourceFileKind::Interface
-          && !ctx.LangOpts.hasFeature(Feature::SE427NoInferenceOnExtension))
-        inferInvertibleReqs = true;
-    }
-
     collectAdditionalExtensionRequirements(ext->getExtendedType(), extraReqs);
 
     auto *extendedNominal = ext->getExtendedNominal();
@@ -838,8 +823,8 @@ GenericSignatureRequest::evaluate(Evaluator &evaluator,
       parentSig.getPointer(),
       genericParams, WhereClauseOwner(GC),
       extraReqs, inferenceSources, loc,
-      /*isExtension=*/isa<ExtensionDecl>(GC),
-      /*allowInverses=*/inferInvertibleReqs};
+      /*forExtension=*/dyn_cast<ExtensionDecl>(GC),
+      /*allowInverses=*/true};
   return evaluateOrDefault(ctx.evaluator, request,
                            GenericSignatureWithError()).getPointer();
 }

lib/Sema/TypeChecker.cpp

@@ -516,7 +516,7 @@ swift::handleSILGenericParams(GenericParamList *genericParams,
       /*parentSig=*/nullptr,
       nestedList.back(), WhereClauseOwner(),
       {}, {}, genericParams->getLAngleLoc(),
-      /*isExtension=*/false,
+      /*forExtension=*/nullptr,
       allowInverses};
   return evaluateOrDefault(DC->getASTContext().evaluator, request,
                            GenericSignatureWithError()).getPointer();

test/Generics/inverse_conditional_conformance_copyable_and_escapable.swift

@@ -0,0 +1,88 @@
+// RUN: %target-swift-frontend -typecheck -verify %s
+
+struct C_C1<T: ~Copyable>: ~Copyable {}
+extension C_C1: Copyable where T: Copyable {}
+
+struct C_C2<T: ~Copyable>: ~Copyable {}
+// expected-error @+2 {{conditional conformance to 'Copyable' must explicitly state whether 'T' is required to conform to 'Copyable'}}
+// expected-error @+1 {{marked with '~Copyable'}}
+extension C_C2: Copyable {}
+
+struct C_CE1<T: ~Copyable & ~Escapable>: ~Copyable {}
+extension C_CE1: Copyable where T: Copyable, T: ~Escapable {}
+
+struct C_CE2<T: ~Copyable & ~Escapable>: ~Copyable {}
+// expected-error @+1 {{conditional conformance to 'Copyable' must explicitly state whether 'T' is required to conform to 'Escapable'}}
+extension C_CE2: Copyable where T: Copyable {}
+
+struct C_CE3<T: ~Copyable & ~Escapable>: ~Copyable {}
+// expected-error @+2 {{conditional conformance to 'Copyable' must explicitly state whether 'T' is required to conform to 'Copyable'}}
+// expected-error @+1 {{marked with '~Copyable'}}
+extension C_CE3: Copyable where T: ~Escapable {}
+
+struct CE_C<T: ~Copyable>: ~Copyable, ~Escapable {}
+extension CE_C: Copyable where T: Copyable {}
+
+struct CE_E<T: ~Escapable>: ~Copyable, ~Escapable {}
+extension CE_E: Escapable where T: Escapable {}
+
+struct CE_CE1<T: ~Copyable & ~Escapable>: ~Copyable, ~Escapable {}
+extension CE_CE1: Copyable where T: Copyable, T: ~Escapable {}
+extension CE_CE1: Escapable where T: ~Copyable, T: Escapable {}
+
+struct CE_CE2<T: ~Copyable & ~Escapable>: ~Copyable, ~Escapable {}
+// expected-error @+1 {{conditional conformance to 'Copyable' must explicitly state whether 'T' is required to conform to 'Escapable'}}
+extension CE_CE2: Copyable where T: Copyable {}
+// expected-error @+1 {{conditional conformance to 'Escapable' must explicitly state whether 'T' is required to conform to 'Copyable'}}
+extension CE_CE2: Escapable where T: Escapable {}
+
+struct CE_CE3<T: ~Copyable & ~Escapable>: ~Copyable, ~Escapable {}
+// expected-error @+2 {{conditional conformance to 'Copyable' must explicitly state whether 'T' is required to conform to 'Copyable'}}
+// expected-error @+1 {{marked with '~Copyable'}}
+extension CE_CE3: Copyable where T: ~Escapable {}
+// expected-error @+2 {{conditional conformance to 'Escapable' must explicitly state whether 'T' is required to conform to 'Escapable'}}
+// expected-error @+1 {{marked with '~Escapable'}}
+extension CE_CE3: Escapable where T: ~Copyable {}
+
+struct C_C_X<T: ~Copyable, U>: ~Copyable {}
+extension C_C_X: Copyable where T: Copyable {}
+
+protocol Floppyable {} // implied Copyable requirement
+
+struct C_C_P<T: ~Copyable, U: Floppyable>: ~Copyable {}
+extension C_C_P: Copyable where T: Copyable {}
+
+class Sloppyable {} // implied Copyable requirement since it's a class
+
+struct C_C_K<T: ~Copyable, U: Sloppyable>: ~Copyable {}
+extension C_C_K: Copyable where T: Copyable {}
+
+protocol Ploppyable: ~Copyable {}
+
+struct C_C_PC1<T: ~Copyable, U: Ploppyable & ~Copyable>: ~Copyable {}
+// expected-error @+1 {{conditional conformance to 'Copyable' must explicitly state whether 'U' is required to conform to 'Copyable'}}
+extension C_C_PC1: Copyable where T: Copyable {}
+
+struct C_C_PC2<T: ~Copyable, U: Ploppyable & ~Copyable>: ~Copyable {}
+extension C_C_PC2: Copyable where T: Copyable, U: Copyable {}
+
+struct C_C_X2<T: ~Copyable, U: Ploppyable /*& Copyable*/>: ~Copyable {}
+extension C_C_X2: Copyable where T: Copyable {}
+
+struct C_C_L<T: ~Copyable, U: AnyObject>: ~Copyable {}
+extension C_C_L: Copyable where T: Copyable {}
+
+struct C_C_C<T: ~Copyable, U: ~Copyable> {}
+extension C_C_C where T: ~Copyable, U == Int {
+    struct SubInt: ~Copyable {}
+}
+
+extension C_C_C.SubInt: Copyable where T: Copyable {}
+
+struct NC: ~Copyable {}
+
+extension C_C_C where T: ~Copyable, U: ~Copyable, U == NC {
+    struct SubNC: ~Copyable {}
+}
+
+extension C_C_C.SubNC: Copyable where T: Copyable {}