Merge pull request swiftlang#59454 from slavapestov/lookup-conformance-unavailable-fix-5.7

slavapestov · web-flow · commit 4b716eb66334 · 2022-06-15T11:03:01.000-04:00
AST: Narrow the filtering of unavailable conformances to Sendable only [5.7]
diff --git a/include/swift/AST/Module.h b/include/swift/AST/Module.h
@@ -631,18 +631,11 @@ class ModuleDecl
   /// might include "missing" conformances, which are synthesized for some
   /// protocols as an error recovery mechanism.
   ///
-  /// \param allowUnavailable When \c true, the resulting conformance reference
-  /// might include "unavailable" conformances, meaning that the conformance
-  /// cannot actually be used and will be diagnosed if used later. Pass
-  /// \c false here for queries that want to determine whether the conformance
-  /// is likely to be usable.
-  ///
   /// \returns The result of the conformance search, which will be
   /// None if the type does not conform to the protocol or contain a
   /// ProtocolConformanceRef if it does conform.
   ProtocolConformanceRef lookupConformance(Type type, ProtocolDecl *protocol,
-                                           bool allowMissing = false,
-                                           bool allowUnavailable = true);
+                                           bool allowMissing = false);
 
   /// Look for the conformance of the given existential type to the given
   /// protocol.
diff --git a/lib/AST/ConformanceLookupTable.cpp b/lib/AST/ConformanceLookupTable.cpp
@@ -820,8 +820,9 @@ DeclContext *ConformanceLookupTable::getConformingContext(
         if (superclassTy->is<ErrorType>())
           return nullptr;
         auto inheritedConformance = module->lookupConformance(
-            superclassTy, protocol, /*allowMissing=*/false,
-            /*allowUnavailable=*/false);
+            superclassTy, protocol, /*allowMissing=*/false);
+        if (inheritedConformance.hasUnavailableConformance())
+          inheritedConformance = ProtocolConformanceRef::forInvalid();
         if (inheritedConformance)
           return superclassDecl;
       } while ((superclassDecl = superclassDecl->getSuperclassDecl()));
diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp
@@ -1016,9 +1016,13 @@ ModuleDecl::lookupExistentialConformance(Type type, ProtocolDecl *protocol) {
   // concretely.
   if (auto superclass = layout.explicitSuperclass) {
     if (auto result = lookupConformance(
-            superclass, protocol, /*allowMissing=*/false,
-            /*allowUnavailable=*/false))
+            superclass, protocol, /*allowMissing=*/false)) {
+      if (protocol->isSpecificProtocol(KnownProtocolKind::Sendable) &&
+          result.hasUnavailableConformance())
+        result = ProtocolConformanceRef::forInvalid();
+
       return result;
+    }
   }
 
   // Otherwise, the existential might conform abstractly.
@@ -1073,8 +1077,7 @@ ProtocolConformanceRef ProtocolConformanceRef::forMissingOrInvalid(
 
 ProtocolConformanceRef ModuleDecl::lookupConformance(Type type,
                                                      ProtocolDecl *protocol,
-                                                     bool allowMissing,
-                                                     bool allowUnavailable) {
+                                                     bool allowMissing) {
   // If we are recursively checking for implicit conformance of a nominal
   // type to Sendable, fail without evaluating this request. This
   // squashes cycles.
@@ -1098,11 +1101,6 @@ ProtocolConformanceRef ModuleDecl::lookupConformance(Type type,
       result.hasMissingConformance(this))
     return ProtocolConformanceRef::forInvalid();
 
-  // If we aren't supposed to allow unavailable conformances but we have one,
-  // replace the result with an "invalid" result.
-  if (!allowUnavailable && result.hasUnavailableConformance())
-    return ProtocolConformanceRef::forInvalid();
-
   return result;
 }
 
@@ -1256,9 +1254,12 @@ LookupConformanceInModuleRequest::evaluate(
     // able to be resolved by a substitution that makes the archetype
     // concrete.
     if (auto super = archetype->getSuperclass()) {
-      if (auto inheritedConformance = mod->lookupConformance(
-              super, protocol, /*allowMissing=*/false,
-              /*allowUnavailable=*/false)) {
+      auto inheritedConformance = mod->lookupConformance(
+          super, protocol, /*allowMissing=*/false);
+      if (protocol->isSpecificProtocol(KnownProtocolKind::Sendable) &&
+          inheritedConformance.hasUnavailableConformance())
+        inheritedConformance = ProtocolConformanceRef::forInvalid();
+      if (inheritedConformance) {
         return ProtocolConformanceRef(ctx.getInheritedConformance(
             type, inheritedConformance.getConcrete()));
       }
diff --git a/lib/AST/ProtocolConformance.cpp b/lib/AST/ProtocolConformance.cpp
@@ -1758,6 +1758,9 @@ SourceLoc swift::extractNearestSourceLoc(const ProtocolConformanceRef conformanc
 }
 
 bool ProtocolConformanceRef::hasUnavailableConformance() const {
+  if (isInvalid())
+    return false;
+
   // Abstract conformances are never unavailable.
   if (!isConcrete())
     return false;
diff --git a/lib/AST/RequirementMachine/ConcreteContraction.cpp b/lib/AST/RequirementMachine/ConcreteContraction.cpp
@@ -271,16 +271,18 @@ Optional<Type> ConcreteContraction::substTypeParameterRec(
       auto *proto = assocType->getProtocol();
       auto *module = proto->getParentModule();
 
-      bool allowUnavailable =
-          !proto->isSpecificProtocol(KnownProtocolKind::Sendable);
       // The 'Sendable' protocol does not declare any associated types, so the
       // 'allowMissing' value here is actually irrelevant.
       auto conformance = ((*substBaseType)->isTypeParameter()
                           ? ProtocolConformanceRef(proto)
                           : module->lookupConformance(
                               *substBaseType, proto,
-                              /*allowMissing=*/false,
-                              allowUnavailable));
+                              /*allowMissing=*/false));
+
+      if (proto->isSpecificProtocol(KnownProtocolKind::Sendable) &&
+          conformance.hasUnavailableConformance()) {
+        conformance = ProtocolConformanceRef::forInvalid();
+      }
 
       // The base type doesn't conform, in which case the requirement remains
       // unsubstituted.
@@ -393,16 +395,22 @@ ConcreteContraction::substRequirement(const Requirement &req) const {
     if (ConcreteTypes.count(stripBoundDependentMemberTypes(firstType)) > 0)
       allowMissing = true;
 
-    bool allowUnavailable =
-        !proto->isSpecificProtocol(KnownProtocolKind::Sendable);
-    if (!substFirstType->isTypeParameter() &&
-        !module->lookupConformance(substFirstType, proto,
-                                   allowMissing, allowUnavailable)) {
-      // Handle the case of <T where T : P, T : C> where C is a class and
-      // C does not conform to P and only substitute the parent type of T
-      // by pretending we have a same-type requirement here.
-      substFirstType = substTypeParameter(
-          firstType, Position::SameTypeRequirement);
+    if (!substFirstType->isTypeParameter()) {
+      auto conformance = module->lookupConformance(substFirstType, proto,
+                                                   allowMissing);
+
+      if (proto->isSpecificProtocol(KnownProtocolKind::Sendable) &&
+          conformance.hasUnavailableConformance()) {
+        conformance = ProtocolConformanceRef::forInvalid();
+      }
+
+      if (!conformance) {
+        // Handle the case of <T where T : P, T : C> where C is a class and
+        // C does not conform to P and only substitute the parent type of T
+        // by pretending we have a same-type requirement here.
+        substFirstType = substTypeParameter(
+            firstType, Position::SameTypeRequirement);
+      }
     }
 
     // Otherwise, replace the generic parameter in the conformance
diff --git a/lib/AST/RequirementMachine/ConcreteTypeWitness.cpp b/lib/AST/RequirementMachine/ConcreteTypeWitness.cpp
@@ -153,12 +153,14 @@ void PropertyMap::concretizeNestedTypesFromConcreteParent(
     // subclasses of 'C' which are 'Sendable'.
     bool allowMissing = (requirementKind == RequirementKind::SameType);
 
-    bool allowUnavailable =
-        !proto->isSpecificProtocol(KnownProtocolKind::Sendable);
     auto conformance = module->lookupConformance(concreteType,
                                                  const_cast<ProtocolDecl *>(proto),
-                                                 allowMissing,
-                                                 allowUnavailable);
+                                                 allowMissing);
+    if (proto->isSpecificProtocol(KnownProtocolKind::Sendable) &&
+        conformance.hasUnavailableConformance()) {
+      conformance = ProtocolConformanceRef::forInvalid();
+    }
+
     if (conformance.isInvalid()) {
       // For superclass rules, it is totally fine to have a signature like:
       //
diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp
@@ -4357,10 +4357,13 @@ ProtocolConformance *GetImplicitSendableRequest::evaluate(
   if (classDecl) {
     if (Type superclass = classDecl->getSuperclass()) {
       auto classModule = classDecl->getParentModule();
-      if (auto inheritedConformance = TypeChecker::conformsToProtocol(
-              classDecl->mapTypeIntoContext(superclass),
-              proto, classModule, /*allowMissing=*/false,
-              /*allowUnavailable=*/false)) {
+      auto inheritedConformance = TypeChecker::conformsToProtocol(
+          classDecl->mapTypeIntoContext(superclass),
+          proto, classModule, /*allowMissing=*/false);
+      if (inheritedConformance.hasUnavailableConformance())
+        inheritedConformance = ProtocolConformanceRef::forInvalid();
+
+      if (inheritedConformance) {
         inheritedConformance = inheritedConformance
             .mapConformanceOutOfContext();
         if (inheritedConformance.isConcrete()) {
diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
@@ -3423,8 +3423,7 @@ void ConformanceChecker::recordTypeWitness(AssociatedTypeDecl *assocType,
     auto overriddenConformance =
       DC->getParentModule()->lookupConformance(Adoptee,
                                                overridden->getProtocol(),
-                                               /*allowMissing=*/true,
-                                               /*allowUnavailable=*/false);
+                                               /*allowMissing=*/true);
     if (overriddenConformance.isInvalid() ||
         !overriddenConformance.isConcrete())
       continue;
@@ -5654,10 +5653,11 @@ TypeChecker::containsProtocol(Type T, ProtocolDecl *Proto, ModuleDecl *M,
 
 ProtocolConformanceRef
 TypeChecker::conformsToProtocol(Type T, ProtocolDecl *Proto, ModuleDecl *M,
-                                bool allowMissing, bool allowUnavailable) {
+                                bool allowMissing) {
   // Look up conformance in the module.
   auto lookupResult = M->lookupConformance(
-      T, Proto, allowMissing, allowUnavailable);
+      T, Proto, allowMissing);
+
   if (lookupResult.isInvalid()) {
     return ProtocolConformanceRef::forInvalid();
   }
diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h
@@ -807,8 +807,7 @@ ProtocolConformanceRef containsProtocol(Type T, ProtocolDecl *Proto,
 /// protocol \c Proto, or \c None.
 ProtocolConformanceRef conformsToProtocol(Type T, ProtocolDecl *Proto,
                                           ModuleDecl *M,
-                                          bool allowMissing = true,
-                                          bool allowUnavailable = true);
+                                          bool allowMissing = true);
 
 /// Check whether the type conforms to a given known protocol.
 bool conformsToKnownProtocol(Type type, KnownProtocolKind protocol,
diff --git a/test/Generics/unavailable_conformances.swift b/test/Generics/unavailable_conformances.swift
@@ -33,3 +33,20 @@ protocol Base {
 
 @available(*, unavailable)
 extension Base where T == ConcreteP {}
+
+// Hashable conformance synthesis ran into problems if the conformance was
+// unavailable (which is legal if the type is unavailable also).
+@available(*, unavailable)
+struct Foo {
+  class Bar {}
+}
+
+@available(*, unavailable)
+extension Foo.Bar: Equatable {
+    static func == (lhs: Foo.Bar, rhs: Foo.Bar) -> Bool { return false }
+}
+
+@available(*, unavailable)
+extension Foo.Bar: Hashable {
+    func hash(into hasher: inout Hasher) {}
+}

Original file line number	Diff line number	Diff line change
`@@ -1758,6 +1758,9 @@ SourceLoc swift::extractNearestSourceLoc(const ProtocolConformanceRef conformanc`
`1758`	`1758`	`}`
`1759`	`1759`
`1760`	`1760`	`bool ProtocolConformanceRef::hasUnavailableConformance() const {`
	`1761`	`+ if (isInvalid())`
	`1762`	`+ return false;`
	`1763`	`+`
`1761`	`1764`	`// Abstract conformances are never unavailable.`
`1762`	`1765`	`if (!isConcrete())`
`1763`	`1766`	`return false;`