Merge pull request #72820 from kavon/conditional-copyable-restriction

kavon · web-flow · commit 179bc4b6051b · 2024-04-09T17:56:34.000-07:00
NCGenerics: restrict conditional Copyable reqs
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -2807,7 +2807,7 @@ ERROR(protocol_has_missing_requirements_versioned,none,
       (Type, Type, llvm::VersionTuple, llvm::VersionTuple))
 ERROR(requirement_restricts_self,none,
       "%kindonly0 requirement %0 cannot add constraint "
-      "'%1%select{:|:| ==|:}2 %3' on 'Self'",
+      "'%1%select{:|:| ==|:| has same shape as}2 %3' on 'Self'",
       (const ValueDecl *, StringRef, unsigned, StringRef))
 ERROR(witness_argument_name_mismatch,none,
       "%kind0 has different argument labels "
@@ -7665,6 +7665,10 @@ ERROR(inverse_extension, none,
 ERROR(copyable_illegal_deinit, none,
       "deinitializer cannot be declared in %kind0 that conforms to 'Copyable'",
       (const ValueDecl *))
+ERROR(inverse_cannot_be_conditional_on_requirement, none,
+      "conditional conformance to suppressible %kind0 cannot depend on "
+      "'%noformat1%select{:|:| ==|:| has same shape as}2 %noformat3'",
+      (const ProtocolDecl *, Type, unsigned, Type))
 ERROR(inverse_type_member_in_conforming_type,none,
       "%select{stored property %2|associated value %2}1 of "
       "'%4'-conforming %kind3 has non-%4 type %0",
diff --git a/include/swift/AST/RequirementKind.h b/include/swift/AST/RequirementKind.h
@@ -36,10 +36,11 @@ enum class RequirementKind : unsigned {
   Layout,
   /// A same-shape requirement shape(T) == shape(U), where T and U are pack
   /// parameters.
-  SameShape
+  SameShape,
 
   // Note: there is code that packs this enum in a 3-bit bitfield.  Audit users
   // when adding enumerators.
+  LAST_KIND=SameShape
 };
 
 } // namespace swift
diff --git a/lib/AST/DiagnosticEngine.cpp b/lib/AST/DiagnosticEngine.cpp
@@ -757,7 +757,13 @@ static void formatDiagnosticArgument(StringRef Modifier,
   case DiagnosticArgumentKind::FullyQualifiedType:
   case DiagnosticArgumentKind::Type:
   case DiagnosticArgumentKind::WitnessType: {
-    assert(Modifier.empty() && "Improper modifier for Type argument");
+    std::optional<DiagnosticFormatOptions> TypeFormatOpts;
+    if (Modifier == "noformat") {
+      TypeFormatOpts.emplace(DiagnosticFormatOptions::formatForFixIts());
+    } else {
+      assert(Modifier.empty() && "Improper modifier for Type argument");
+      TypeFormatOpts.emplace(FormatOpts);
+    }
     
     // Strip extraneous parentheses; they add no value.
     Type type;
@@ -829,7 +835,7 @@ static void formatDiagnosticArgument(StringRef Modifier,
 
       auto descriptiveKind = opaqueTypeDecl->getDescriptiveKind();
 
-      Out << llvm::format(FormatOpts.OpaqueResultFormatString.c_str(),
+      Out << llvm::format(TypeFormatOpts->OpaqueResultFormatString.c_str(),
                           type->getString(printOptions).c_str(),
                           Decl::getDescriptiveKindName(descriptiveKind).data(),
                           NamingDeclText.c_str());
@@ -843,11 +849,11 @@ static void formatDiagnosticArgument(StringRef Modifier,
         llvm::raw_svector_ostream OutAka(AkaText);
 
         getAkaTypeForDisplay(type)->print(OutAka, printOptions);
-        Out << llvm::format(FormatOpts.AKAFormatString.c_str(),
+        Out << llvm::format(TypeFormatOpts->AKAFormatString.c_str(),
                             typeName.c_str(), AkaText.c_str());
       } else {
-        Out << FormatOpts.OpeningQuotationMark << typeName
-            << FormatOpts.ClosingQuotationMark;
+        Out << TypeFormatOpts->OpeningQuotationMark << typeName
+            << TypeFormatOpts->ClosingQuotationMark;
       }
     }
     break;
diff --git a/lib/Sema/TypeCheckGeneric.cpp b/lib/Sema/TypeCheckGeneric.cpp
@@ -302,6 +302,8 @@ void TypeChecker::checkProtocolSelfRequirements(ValueDecl *decl) {
           req.getFirstType()->is<GenericTypeParamType>())
         continue;
 
+      static_assert((unsigned)RequirementKind::LAST_KIND == 4,
+                    "update %select in diagnostic!");
       ctx.Diags.diagnose(decl, diag::requirement_restricts_self, decl,
                          req.getFirstType().getString(),
                          static_cast<unsigned>(req.getKind()),
diff --git a/lib/Sema/TypeCheckInvertible.cpp b/lib/Sema/TypeCheckInvertible.cpp
@@ -140,10 +140,51 @@ static void checkInvertibleConformanceCommon(DeclContext *dc,
   if (conformance.isConcrete()) {
     auto concrete = conformance.getConcrete();
     if (auto *normalConf = dyn_cast<NormalProtocolConformance>(concrete)) {
-      hasUnconditionalConformance =
-          normalConf->getConditionalRequirements().empty();
       conformanceLoc = normalConf->getLoc();
       assert(conformanceLoc);
+
+      auto condReqs = normalConf->getConditionalRequirements();
+      hasUnconditionalConformance = condReqs.empty();
+      auto *thisProto = normalConf->getProtocol();
+
+      // Ensure that conditional conformance to an invertible protocol IP only
+      // depends conformance requirements involving IP, and its subject is not
+      // a dependent member type.
+      //
+      // In theory, it could depend on any invertible protocol, but it may be
+      // confusing if we permitted that and this simplifies the model a bit.
+      for (auto req : condReqs) {
+        Type illegalSecondType;
+
+        // If we are diagnosing, fill-in the second-type string of this req.
+        switch (req.getKind()) {
+        case RequirementKind::Layout:
+          assert(req.getLayoutConstraint()->isClass());
+          illegalSecondType = ctx.getAnyObjectType();
+          break;
+        case RequirementKind::Conformance:
+          if (req.getProtocolDecl() == thisProto
+              && !req.getFirstType()->is<DependentMemberType>())
+            break; // permitted, don't fill-in.
+        LLVM_FALLTHROUGH;
+        case RequirementKind::Superclass:
+        case RequirementKind::SameType:
+        case RequirementKind::SameShape:
+          illegalSecondType = req.getSecondType();
+          break;
+        }
+
+        static_assert((unsigned)RequirementKind::LAST_KIND == 4,
+                      "update %select in diagnostic!");
+        if (illegalSecondType) {
+          auto t = ctx.Diags.diagnose(conformanceLoc,
+                             diag::inverse_cannot_be_conditional_on_requirement,
+                             thisProto,
+                             req.getFirstType(),
+                             static_cast<unsigned>(req.getKind()),
+                             illegalSecondType);
+        }
+      }
     }
   }
   assert(!conformance.isPack() && "not handled");
diff --git a/test/Generics/inverse_conditional_conformance_restriction.swift b/test/Generics/inverse_conditional_conformance_restriction.swift
@@ -0,0 +1,39 @@
+// RUN: %target-typecheck-verify-swift \
+// RUN:   -enable-experimental-feature NoncopyableGenerics \
+// RUN:   -enable-experimental-feature NonescapableTypes \
+// RUN:   -enable-experimental-feature SuppressedAssociatedTypes
+
+protocol P {}
+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'}}
+
+struct Yuck<T: ~Copyable & ~Escapable>: ~Copyable, ~Escapable {}
+extension Yuck: Copyable where T: ~Escapable {}
+extension Yuck: Escapable where T: ~Copyable {}
+
+struct TryConformance<Whatever: ~Copyable>: ~Copyable {}
+extension TryConformance: Copyable
+    where Whatever: P, Whatever: Q, Whatever: Sendable {}
+// expected-error@-2 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Whatever: P'}}
+// expected-error@-3 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Whatever: Q'}}
+// expected-error@-4 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Whatever: Sendable'}}
+
+struct TrySameType<Whatever: ~Copyable>: ~Copyable {}
+extension TrySameType: Copyable
+    where Whatever == Int {}
+// expected-error@-2 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Whatever == Int'}}
+
+struct TryClassAndLayoutConstraints<Whatever: ~Copyable, Heckin>: ~Copyable {}
+extension TryClassAndLayoutConstraints: Copyable
+    where Heckin: DoggoClass, Whatever: AnyObject {}
+// expected-error@-2 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Whatever: AnyObject'}}
+// expected-error@-3 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Heckin: DoggoClass'}}
+
+protocol Queue: ~Copyable { associatedtype Job: ~Copyable }
+struct Scheduler<Q: Queue>: ~Copyable {}
+extension Scheduler: Copyable where Q.Job: Copyable {}
+// expected-error@-1 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Q.Job: Copyable'}}
diff --git a/test/ModuleInterface/Inputs/NoncopyableGenerics_Misc.swift b/test/ModuleInterface/Inputs/NoncopyableGenerics_Misc.swift
@@ -98,7 +98,7 @@ extension Outer: Copyable {}
 extension Outer.InnerStruct: Copyable {}
 
 extension Outer.InnerVariation1: Copyable {}
-extension Outer.InnerVariation2: Escapable {}
+extension Outer.InnerVariation2: Escapable where A: ~Copyable {}
 
 extension Outer.InnerStruct {
     public func hello<T: ~Escapable>(_ t: T) {}
diff --git a/test/ModuleInterface/noncopyable_generics.swift b/test/ModuleInterface/noncopyable_generics.swift
@@ -139,7 +139,7 @@ import NoncopyableGenerics_Misc
 // CHECK-MISC: #endif
 
 // CHECK-MISC: #if compiler(>=5.3) && $NoncopyableGenerics
-// CHECK-MISC-NEXT: extension {{.*}}.Outer.InnerVariation2 : Swift.Escapable {
+// CHECK-MISC-NEXT: extension {{.*}}.Outer.InnerVariation2 : Swift.Escapable where A : ~Copyable {
 // CHECK-MISC: #endif
 
 // CHECK-MISC: #if compiler(>=5.3) && $NoncopyableGenerics
