Merge pull request #74322 from kavon/se427-noimplicit-for-invertible

SE-427: Make conditional conformances to Copyable more explicit.

Author: kavon

include/swift/AST/DiagnosticsSema.def

@@ -7728,8 +7728,8 @@ ERROR(inverse_conflicts_explicit_composition, none,
       "composition cannot contain '~%0' when another member requires '%0'",
       (StringRef))
 ERROR(inverse_extension, none,
-      "cannot suppress %0 in extension",
-      (Type))
+      "cannot suppress '%0' in extension",
+      (StringRef))
 ERROR(copyable_illegal_deinit, none,
       "deinitializer cannot be declared in %kind0 that conforms to 'Copyable'",
       (const ValueDecl *))
@@ -7799,6 +7799,9 @@ ERROR(suppress_nonsuppressable_protocol,none,
       "conformance to %0 cannot be suppressed", (const ProtocolDecl *))
 WARNING(suppress_already_suppressed_protocol,none,
       "already suppressed conformance to %0", (const ProtocolDecl *))
+ERROR(extension_conforms_to_invertible_and_others, none,
+      "conformance to '%0' must be declared in a separate extension",
+      (StringRef))
 
 // -- older ones below --
 ERROR(noncopyable_parameter_requires_ownership, none,

lib/AST/ASTPrinter.cpp

@@ -2915,6 +2915,19 @@ void PrintAST::printExtendedTypeName(TypeLoc ExtendedTypeLoc) {
   printTypeLoc(TypeLoc(ExtendedTypeLoc.getTypeRepr(), Ty));
 }
 
+/// If an extension adds a conformance for an invertible protocol, then we
+/// should not print inverses for its generic signature, because no conditional
+/// requirements are inferred by default for such an extension.
+static bool isExtensionAddingInvertibleConformance(const ExtensionDecl *ext) {
+  auto conformances = ext->getLocalConformances();
+  for (auto *conf : conformances) {
+    if (conf->getProtocol()->getInvertibleProtocolKind()) {
+      assert(conformances.size() == 1 && "expected solo conformance");
+      return true;
+    }
+  }
+  return false;
+}
 
 void PrintAST::printSynthesizedExtension(Type ExtendedType,
                                          ExtensionDecl *ExtDecl) {
@@ -3039,9 +3052,21 @@ void PrintAST::printExtension(ExtensionDecl *decl) {
       auto baseGenericSig = decl->getExtendedNominal()->getGenericSignature();
       assert(baseGenericSig &&
              "an extension can't be generic if the base type isn't");
+
+      auto genSigFlags = defaultGenericRequirementFlags()
+                       | IncludeOuterInverses;
+
+      // Disable printing inverses if the extension is adding a conformance
+      // for an invertible protocol itself, as we do not infer any requirements
+      // in such an extension. We need to print the whole signature:
+      //     extension S: Copyable where T: Copyable
+      if (isExtensionAddingInvertibleConformance(decl)) {
+        genSigFlags &= ~PrintInverseRequirements;
+        genSigFlags &= ~IncludeOuterInverses;
+      }
+
       printGenericSignature(genericSig,
-                            defaultGenericRequirementFlags()
-                              | IncludeOuterInverses,
+                            genSigFlags,
                             [baseGenericSig](const Requirement &req) -> bool {
         // Only include constraints that are not satisfied by the base type.
         return !baseGenericSig->isRequirementSatisfied(req);

lib/AST/RequirementMachine/RequirementMachineRequests.cpp

@@ -782,7 +782,6 @@ InferredGenericSignatureRequest::evaluate(
 
   unsigned numOuterParams = genericParams.size();
   if (isExtension) {
-    assert(allowInverses);
     numOuterParams = 0;
   }
 

lib/Frontend/ModuleInterfaceSupport.cpp

@@ -802,7 +802,7 @@ class InheritedProtocolCollector {
   }
 
   /// If there were any conditional conformances that couldn't be printed,
-  /// make a dummy extension that conforms to all of them, constrained by a
+  /// make dummy extension(s) that conforms to all of them, constrained by a
   /// fake protocol.
   bool printInaccessibleConformanceExtensionIfNeeded(
       raw_ostream &out, const PrintOptions &printOptions,
@@ -811,20 +811,35 @@ class InheritedProtocolCollector {
       return false;
     assert(nominal->isGenericContext());
 
-    if (!printOptions.printPublicInterface())
-      out << "@_spi(" << DummyProtocolName << ")\n";
-    out << "@available(*, unavailable)\nextension ";
-    nominal->getDeclaredType().print(out, printOptions);
-    out << " : ";
-    llvm::interleave(
-        ConditionalConformanceProtocols,
-        [&out, &printOptions](const ProtocolType *protoTy) {
-          protoTy->print(out, printOptions);
-        },
-        [&out] { out << ", "; });
-    out << " where "
-        << nominal->getGenericSignature().getGenericParams().front()->getName()
-        << " : " << DummyProtocolName << " {}\n";
+    auto emitExtension =
+        [&](ArrayRef<const ProtocolType *> conformanceProtos) {
+      if (!printOptions.printPublicInterface())
+        out << "@_spi(" << DummyProtocolName << ")\n";
+      out << "@available(*, unavailable)\nextension ";
+      nominal->getDeclaredType().print(out, printOptions);
+      out << " : ";
+      llvm::interleave(
+          conformanceProtos,
+          [&out, &printOptions](const ProtocolType *protoTy) {
+            protoTy->print(out, printOptions);
+          },
+          [&out] { out << ", "; });
+      out << " where "
+          << nominal->getGenericSignature().getGenericParams()[0]->getName()
+          << " : " << DummyProtocolName << " {}\n";
+    };
+
+    // We have to print conformances for invertible protocols in separate
+    // extensions, so do those first and save the rest for one extension.
+    SmallVector<const ProtocolType *, 8> regulars;
+    for (auto *proto : ConditionalConformanceProtocols) {
+      if (proto->getDecl()->getInvertibleProtocolKind()) {
+        emitExtension(proto);
+        continue;
+      }
+      regulars.push_back(proto);
+    }
+    emitExtension(regulars);
     return true;
   }
 

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -154,6 +154,28 @@ class CheckRepressions {
   }
 };
 
+/// If the extension adds a conformance to an invertible protocol, ensure that
+/// it does not add a conformance to any other protocol. So these are illegal:
+///
+///     extension S: Copyable & P {}
+///     extension S: Q, Copyable {}
+///
+/// This policy is in place because extensions adding a conformance to an
+/// invertible protocol do _not_ add default requirements on generic parameters,
+/// so it would be confusing to mix them together in the same extension.
+static void checkExtensionAddsSoloInvertibleProtocol(const ExtensionDecl *ext) {
+  auto localConfs = ext->getLocalConformances();
+  if (localConfs.size() <= 1)
+    return;
+
+  for (auto *conf : localConfs) {
+    if (auto ip = conf->getProtocol()->getInvertibleProtocolKind()) {
+      ext->diagnose(diag::extension_conforms_to_invertible_and_others,
+                    getInvertibleProtocolKindName(*ip));
+    }
+  }
+}
+
 /// Check the inheritance clause of a type declaration or extension thereof.
 ///
 /// This routine performs detailed checking of the inheritance clause of the
@@ -295,10 +317,15 @@ static void checkInheritanceClause(
       auto layout = inheritedTy->getExistentialLayout();
 
       // An inverse on an extension is an error.
-      if (isa<ExtensionDecl>(decl))
-        if (auto pct = inheritedTy->getAs<ProtocolCompositionType>())
-          if (!pct->getInverses().empty())
-            decl->diagnose(diag::inverse_extension, inheritedTy);
+      if (isa<ExtensionDecl>(decl)) {
+        auto canInheritedTy = inheritedTy->getCanonicalType();
+        if (auto pct = canInheritedTy->getAs<ProtocolCompositionType>()) {
+          for (auto inverse : pct->getInverses()) {
+            decl->diagnose(diag::inverse_extension,
+                           getProtocolName(getKnownProtocolKind(inverse)));
+          }
+        }
+      }
 
       // Subclass existentials are not allowed except on classes and
       // non-@objc protocols.
@@ -3974,6 +4001,8 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
     diagnoseExtensionOfMarkerProtocol(ED);
 
     checkTupleExtension(ED);
+
+    checkExtensionAddsSoloInvertibleProtocol(ED);
   }
 
   void visitTopLevelCodeDecl(TopLevelCodeDecl *TLCD) {

lib/Sema/TypeCheckGeneric.cpp

@@ -684,6 +684,7 @@ 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();
@@ -781,6 +782,35 @@ GenericSignatureRequest::evaluate(Evaluator &evaluator,
   } else if (auto *ext = dyn_cast<ExtensionDecl>(GC)) {
     loc = ext->getLoc();
 
+    // The inherited entries influence the generic signature of the extension,
+    // because if it introduces conformance to invertible protocol IP, we do not
+    // we do not infer any requirements that the generic parameters to conform
+    // to invertible protocols. This forces people to write out the conditions.
+    const unsigned numEntries = ext->getInherited().size();
+    for (unsigned i = 0; i < numEntries; ++i) {
+      InheritedTypeRequest request{ext, i, TypeResolutionStage::Structural};
+      auto result = evaluateOrDefault(ctx.evaluator, request,
+                                      InheritedTypeResult::forDefault());
+      Type inheritedTy;
+      switch (result) {
+      case InheritedTypeResult::Inherited:
+        inheritedTy = result.getInheritedType();
+        break;
+      case InheritedTypeResult::Suppressed:
+      case InheritedTypeResult::Default:
+        continue;
+      }
+
+      if (inheritedTy) {
+        if (auto kp = inheritedTy->getKnownProtocol()) {
+          if (getInvertibleProtocolKind(*kp)) {
+            inferInvertibleReqs = false;
+            break;
+          }
+        }
+      }
+    }
+
     collectAdditionalExtensionRequirements(ext->getExtendedType(), extraReqs);
 
     auto *extendedNominal = ext->getExtendedNominal();
@@ -800,7 +830,7 @@ GenericSignatureRequest::evaluate(Evaluator &evaluator,
       genericParams, WhereClauseOwner(GC),
       extraReqs, inferenceSources, loc,
       /*isExtension=*/isa<ExtensionDecl>(GC),
-      /*allowInverses=*/true};
+      /*allowInverses=*/inferInvertibleReqs};
   return evaluateOrDefault(ctx.evaluator, request,
                            GenericSignatureWithError()).getPointer();
 }

stdlib/public/core/Optional.swift

@@ -132,7 +132,7 @@ public enum Optional<Wrapped: ~Copyable>: ~Copyable {
   case some(Wrapped)
 }
 
-extension Optional: Copyable /* where Wrapped: Copyable */ {}
+extension Optional: Copyable where Wrapped: Copyable {}
 
 extension Optional: Sendable where Wrapped: ~Copyable & Sendable { }
 

stdlib/public/core/Result.swift

@@ -21,7 +21,7 @@ public enum Result<Success: ~Copyable, Failure: Error> {
   case failure(Failure)
 }
 
-extension Result: Copyable /* where Success: Copyable */ {}
+extension Result: Copyable where Success: Copyable {}
 
 extension Result: Sendable where Success: Sendable & ~Copyable {}
 

test/Generics/inverse_conditional_conformance_restriction.swift

@@ -7,12 +7,15 @@ protocol Q {}
 class DoggoClass {}
 
 struct Blah<T: ~Copyable & ~Escapable>: ~Copyable, ~Escapable {}
-extension Blah: Copyable {} // expected-error {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'T: Escapable'}}
-extension Blah: Escapable {} // expected-error {{conditional conformance to suppressible protocol 'Escapable' cannot depend on 'T: Copyable'}}
+extension Blah: Copyable where T: Copyable & Escapable {}
+// expected-error@-1 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'T: Escapable'}}
 
-struct Yuck<T: ~Copyable & ~Escapable>: ~Copyable, ~Escapable {}
-extension Yuck: Copyable where T: ~Escapable {}
-extension Yuck: Escapable where T: ~Copyable {}
+extension Blah: Escapable where T: Copyable, T: Escapable {}
+// expected-error@-1 {{conditional conformance to suppressible protocol 'Escapable' cannot depend on 'T: Copyable'}}
+
+struct Fixed<T: ~Copyable & ~Escapable>: ~Copyable, ~Escapable {}
+extension Fixed: Copyable where T: Copyable {}
+extension Fixed: Escapable where T: Escapable {}
 
 struct TryConformance<Whatever: ~Copyable>: ~Copyable {}
 extension TryConformance: Copyable

test/Generics/inverse_generics.swift

@@ -2,11 +2,26 @@
 // RUN: -enable-experimental-feature NonescapableTypes \
 // RUN: -enable-experimental-feature SuppressedAssociatedTypes
 
+// expected-note@+1 {{'T' has '~Copyable' constraint preventing implicit 'Copyable' conformance}}
+struct AttemptImplicitConditionalConformance<T: ~Copyable>: ~Copyable {
+  var t: T // expected-error {{stored property 't' of 'Copyable'-conforming generic struct 'AttemptImplicitConditionalConformance' has non-Copyable type 'T'}}
+}
+extension AttemptImplicitConditionalConformance: Copyable {}
+// expected-error@-1 {{generic struct 'AttemptImplicitConditionalConformance' required to be 'Copyable' but is marked with '~Copyable'}}
+
+enum Hello<T: ~Escapable & ~Copyable>: ~Escapable & ~Copyable {}
+extension Hello: Escapable {} // expected-error {{generic enum 'Hello' required to be 'Escapable' but is marked with '~Escapable'}}
+extension Hello: Copyable {} // expected-error {{generic enum 'Hello' required to be 'Copyable' but is marked with '~Copyable'}}
+
+enum HelloExplicitlyFixed<T: ~Escapable & ~Copyable>: Escapable, Copyable {}
 
+struct NoInverseBecauseNoDefault<T: ~Copyable & ~Escapable>: ~Copyable {}
+extension NoInverseBecauseNoDefault: Copyable where T: Copyable, T: ~Escapable {}
+// expected-error@-1 {{cannot suppress '~Escapable' on generic parameter 'T' defined in outer scope}}
 
 // Check support for explicit conditional conformance
 public struct ExplicitCond<T: ~Copyable>: ~Copyable {}
-extension ExplicitCond: Copyable {}
+extension ExplicitCond: Copyable where T: Copyable {}
 // expected-note@-1 {{requirement from conditional conformance}}
 // expected-note@-2 {{requirement from conditional conformance of 'ExplicitCondAlias<NC>' (aka 'ExplicitCond<NC>') to 'Copyable'}}
 
@@ -80,7 +95,7 @@ struct ConditionalContainment<T: ~Copyable>: ~Copyable {
   var y: NC // expected-error {{stored property 'y' of 'Copyable'-conforming generic struct 'ConditionalContainment' has non-Copyable type 'NC'}}
 }
 
-extension ConditionalContainment: Copyable {}
+extension ConditionalContainment: Copyable where T: Copyable {}
 
 func chk(_ T: RequireCopyable<ConditionalContainment<Int>>) {}
 
@@ -126,7 +141,7 @@ enum Maybe<Wrapped: ~Copyable>: ~Copyable {
   deinit {} // expected-error {{deinitializer cannot be declared in generic enum 'Maybe' that conforms to 'Copyable'}}
 }
 
-extension Maybe: Copyable {}
+extension Maybe: Copyable where Wrapped: Copyable {}
 
 // expected-note@+4{{requirement specified as 'NC' : 'Copyable'}}
 // expected-note@+3{{requirement from conditional conformance of 'Maybe<NC>' to 'Copyable'}}
@@ -173,11 +188,11 @@ class NiceTry: ~Copyable, Copyable {} // expected-error {{classes cannot be '~Co
 
 
 struct Extendo: ~Copyable {}
-extension Extendo: Copyable, ~Copyable {} // expected-error {{cannot suppress '~Copyable' in extension}}
+extension Extendo: Copyable, ~Copyable {} // expected-error {{cannot suppress 'Copyable' in extension}}
 // expected-error@-1 {{struct 'Extendo' required to be 'Copyable' but is marked with '~Copyable'}}
 
 enum EnumExtendo {}
-extension EnumExtendo: ~Copyable {} // expected-error {{cannot suppress '~Copyable' in extension}}
+extension EnumExtendo: ~Copyable {} // expected-error {{cannot suppress 'Copyable' in extension}}
 
 extension NeedsCopyable where Self: ~Copyable {}
 // expected-error@-1 {{'Self' required to be 'Copyable' but is marked with '~Copyable'}}
@@ -265,7 +280,7 @@ enum MaybeEscapes<T: ~Escapable>: ~Escapable { // expected-note {{generic enum '
   case none
 }
 
-extension MaybeEscapes: Escapable {}
+extension MaybeEscapes: Escapable where T: Escapable {}
 
 struct Escapes { // expected-note {{consider adding '~Escapable' to struct 'Escapes'}}
   let t: MaybeEscapes<NonescapingType> // expected-error {{stored property 't' of 'Escapable'-conforming struct 'Escapes' has non-Escapable type 'MaybeEscapes<NonescapingType>'}}