Merge pull request #72495 from slavapestov/fix-protocol-superclass

AST: Remove ProtocolDecl::getSuperclass()

Author: slavapestov

CHANGELOG.md

@@ -4,6 +4,38 @@
 > This is in reverse chronological order, so newer entries are added to the top.
 
 ## Swift 6.0
+
+* Swift 5.10 missed a semantic check from [SE-0309][]. In type context, a reference to a
+  protocol `P` that has associated types or `Self` requirements should use
+  the `any` keyword, but this was not enforced in nested generic argument positions.
+  This is now an error as required by the proposal:
+
+  ```swift
+  protocol P { associatedtype A }
+  struct Outer<T> { struct Inner<U> { } }
+  let x = Outer<P>.Inner<P>()  // error
+  ```
+  To correct the error, add `any` where appropriate, for example
+  `Outer<any P>.Inner<any P>`.
+
+* Swift 5.10 accepted certain invalid opaque return types from [SE-0346][].
+  If a generic argument of a constrained opaque return type did not
+  satisfy the requirements on the primary associated type, the generic
+  argument was silently ignored and type checking would proceed as if it
+  weren't stated. This now results in a diagnostic:
+
+  ```swift
+  protocol P<A> { associatedtype A: Sequence }
+  struct G<A: Sequence>: P {}
+
+  func f() -> some P<Int> { return G<Array<Int>>() }  // error
+  ```
+
+  The return type above should be written as `some P<Array<Int>>` to match
+  the return statement. The old broken behavior in this situation can also
+  be restored, by removing the erroneous constraint and using the more general
+  upper bound `some P`.
+
 * [SE-0408][]:
   A `for`-`in` loop statement can now accept a pack expansion expression,
   enabling iteration over the elements of its respective value pack. This form

include/swift/AST/Decl.h

@@ -598,7 +598,7 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
     IsComputingSemanticMembers : 1
   );
 
-  SWIFT_INLINE_BITFIELD_FULL(ProtocolDecl, NominalTypeDecl, 1+1+1+1+1+1+1+1+1+1+1+1+1+8,
+  SWIFT_INLINE_BITFIELD_FULL(ProtocolDecl, NominalTypeDecl, 1+1+1+1+1+1+1+1+1+1+1+1+1+1+8,
     /// Whether the \c RequiresClass bit is valid.
     RequiresClassValid : 1,
 
@@ -625,6 +625,9 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
     /// Whether we've computed the inherited protocols list yet.
     InheritedProtocolsValid : 1,
 
+    /// Whether we have computed a requirement signature.
+    HasRequirementSignature : 1,
+
     /// Whether we have a lazy-loaded requirement signature.
     HasLazyRequirementSignature : 1,
 
@@ -5122,15 +5125,11 @@ class ProtocolDecl final : public NominalTypeDecl {
     /// The superclass decl and a bit to indicate whether the
     /// superclass was computed yet or not.
     llvm::PointerIntPair<ClassDecl *, 1, bool> SuperclassDecl;
-
-    /// The superclass type and a bit to indicate whether the
-    /// superclass was computed yet or not.
-    llvm::PointerIntPair<Type, 1, bool> SuperclassType;
   } LazySemanticInfo;
 
   /// The generic signature representing exactly the new requirements introduced
   /// by this protocol.
-  std::optional<RequirementSignature> RequirementSig;
+  RequirementSignature RequirementSig;
 
   /// Returns the cached result of \c requiresClass or \c None if it hasn't yet
   /// been computed.
@@ -5217,16 +5216,10 @@ class ProtocolDecl final : public NominalTypeDecl {
   /// Determine whether this protocol has a superclass.
   bool hasSuperclass() const { return (bool)getSuperclassDecl(); }
 
-  /// Retrieve the superclass of this protocol, or null if there is no superclass.
-  Type getSuperclass() const;
-
   /// Retrieve the ClassDecl for the superclass of this protocol, or null if there
   /// is no superclass.
   ClassDecl *getSuperclassDecl() const;
 
-  /// Set the superclass of this protocol.
-  void setSuperclass(Type superclass);
-
   /// Retrieve the set of AssociatedTypeDecl members of this protocol; this
   /// saves loading the set of members in cases where there's no possibility of
   /// a protocol having nested types (ObjC protocols).
@@ -5445,7 +5438,7 @@ class ProtocolDecl final : public NominalTypeDecl {
 
   /// Has the requirement signature been computed yet?
   bool isRequirementSignatureComputed() const {
-    return RequirementSig.has_value();
+    return Bits.ProtocolDecl.HasRequirementSignature;
   }
 
   void setRequirementSignature(RequirementSignature requirementSig);

include/swift/AST/TypeCheckRequests.h

@@ -113,7 +113,7 @@ class InheritedTypeRequest
 /// Request the superclass type for the given class.
 class SuperclassTypeRequest
     : public SimpleRequest<
-          SuperclassTypeRequest, Type(NominalTypeDecl *, TypeResolutionStage),
+          SuperclassTypeRequest, Type(ClassDecl *, TypeResolutionStage),
           RequestFlags::SeparatelyCached | RequestFlags::DependencySink> {
 public:
   using SimpleRequest::SimpleRequest;
@@ -123,7 +123,7 @@ class SuperclassTypeRequest
 
   // Evaluation.
   Type
-  evaluate(Evaluator &evaluator, NominalTypeDecl *classDecl,
+  evaluate(Evaluator &evaluator, ClassDecl *classDecl,
            TypeResolutionStage stage) const;
 
 public:

lib/AST/ASTPrinter.cpp

@@ -7766,7 +7766,9 @@ static void getSyntacticInheritanceClause(const ProtocolDecl *proto,
                                           llvm::SmallVectorImpl<InheritedEntry> &Results) {
   auto &ctx = proto->getASTContext();
 
-  if (auto superclassTy = proto->getSuperclass()) {
+  auto genericSig = proto->getGenericSignature();
+  if (auto superclassTy = genericSig->getSuperclassBound(
+        proto->getSelfInterfaceType())) {
     Results.emplace_back(TypeLoc::withoutLoc(superclassTy),
                          /*isUnchecked=*/false,
                          /*isRetroactive=*/false,

lib/AST/ConformanceLookup.cpp

@@ -76,8 +76,8 @@ ModuleDecl::lookupExistentialConformance(Type type, ProtocolDecl *protocol) {
     return type;
   };
 
-  auto lookupSuperclassConformance = [&](ExistentialLayout &layout) {
-    if (auto superclass = layout.explicitSuperclass) {
+  auto lookupSuperclassConformance = [&](Type superclass) {
+    if (superclass) {
       if (auto result =
               lookupConformance(superclass, protocol, /*allowMissing=*/false)) {
         if (protocol->isSpecificProtocol(KnownProtocolKind::Sendable) &&
@@ -101,7 +101,7 @@ ModuleDecl::lookupExistentialConformance(Type type, ProtocolDecl *protocol) {
       auto layout = type->getExistentialLayout();
       if (llvm::all_of(layout.getProtocols(),
                        [](const auto *P) { return P->isMarkerProtocol(); })) {
-        if (auto conformance = lookupSuperclassConformance(layout)) {
+        if (auto conformance = lookupSuperclassConformance(layout.explicitSuperclass)) {
           return ProtocolConformanceRef(
               ctx.getInheritedConformance(type, conformance.getConcrete()));
         }
@@ -113,15 +113,12 @@ ModuleDecl::lookupExistentialConformance(Type type, ProtocolDecl *protocol) {
 
   auto layout = type->getExistentialLayout();
 
-  // Due to an IRGen limitation, witness tables cannot be passed from an
-  // existential to an archetype parameter, so for now we restrict this to
-  // @objc protocols and marker protocols.
+  // If the existential contains non-@objc protocols and the protocol we're
+  // conforming to needs a witness table, the existential must have a
+  // self-conformance witness table. For now, Swift.Error is the only one.
   if (!layout.isObjC() && !protocol->isMarkerProtocol()) {
     auto constraint = getConstraintType();
-    // There's a specific exception for protocols with self-conforming
-    // witness tables, but the existential has to be *exactly* that type.
-    // TODO: synthesize witness tables on-demand for protocol compositions
-    // that can satisfy the requirement.
+    // The existential has to be *exactly* that type.
     if (protocol->requiresSelfConformanceWitnessTable() &&
         constraint->is<ProtocolType>() &&
         constraint->castTo<ProtocolType>()->getDecl() == protocol)
@@ -130,31 +127,23 @@ ModuleDecl::lookupExistentialConformance(Type type, ProtocolDecl *protocol) {
     return ProtocolConformanceRef::forInvalid();
   }
 
-  // If the existential is class-constrained, the class might conform
-  // concretely.
-  if (auto conformance = lookupSuperclassConformance(layout))
-    return conformance;
-
-  // Otherwise, the existential might conform abstractly.
+  // The existential might conform abstractly.
   for (auto protoDecl : layout.getProtocols()) {
-
     // If we found the protocol we're looking for, return an abstract
     // conformance to it.
     if (protoDecl == protocol)
       return ProtocolConformanceRef(ctx.getSelfConformance(protocol));
 
-    // If the protocol has a superclass constraint, we might conform
-    // concretely.
-    if (auto superclass = protoDecl->getSuperclass()) {
-      if (auto result = lookupConformance(superclass, protocol))
-        return result;
-    }
-
     // Now check refined protocols.
     if (protoDecl->inheritsFrom(protocol))
       return ProtocolConformanceRef(ctx.getSelfConformance(protocol));
   }
 
+  // If the existential is class-constrained, the class might conform
+  // concretely.
+  if (auto conformance = lookupSuperclassConformance(layout.getSuperclass()))
+    return conformance;
+
   // We didn't find our protocol in the existential's list; it doesn't
   // conform.
   return ProtocolConformanceRef::forInvalid();

lib/AST/Decl.cpp

@@ -6436,6 +6436,8 @@ ProtocolDecl::ProtocolDecl(DeclContext *DC, SourceLoc ProtocolLoc,
   Bits.ProtocolDecl.KnownProtocol = 0;
   Bits.ProtocolDecl.HasAssociatedTypes = 0;
   Bits.ProtocolDecl.HasLazyAssociatedTypes = 0;
+  Bits.ProtocolDecl.HasRequirementSignature = 0;
+  Bits.ProtocolDecl.HasLazyRequirementSignature = 0;
   Bits.ProtocolDecl.ProtocolRequirementsValid = false;
   setTrailingWhereClause(TrailingWhere);
 }
@@ -6545,29 +6547,12 @@ AssociatedTypeDecl *ProtocolDecl::getAssociatedType(Identifier name) const {
   return nullptr;
 }
 
-Type ProtocolDecl::getSuperclass() const {
-  ASTContext &ctx = getASTContext();
-  return evaluateOrDefault(ctx.evaluator,
-    SuperclassTypeRequest{const_cast<ProtocolDecl *>(this),
-                          TypeResolutionStage::Interface},
-    Type());
-}
-
 ClassDecl *ProtocolDecl::getSuperclassDecl() const {
   ASTContext &ctx = getASTContext();
   return evaluateOrDefault(ctx.evaluator,
     SuperclassDeclRequest{const_cast<ProtocolDecl *>(this)}, nullptr);
 }
 
-void ProtocolDecl::setSuperclass(Type superclass) {
-  assert((!superclass || !superclass->hasArchetype())
-         && "superclass must be interface type");
-  LazySemanticInfo.SuperclassType.setPointerAndInt(superclass, true);
-  LazySemanticInfo.SuperclassDecl.setPointerAndInt(
-    superclass ? superclass->getClassOrBoundGenericClass() : nullptr,
-    true);
-}
-
 bool ProtocolDecl::walkInheritedProtocols(
               llvm::function_ref<TypeWalker::Action(ProtocolDecl *)> fn) const {
   auto self = const_cast<ProtocolDecl *>(this);
@@ -6699,12 +6684,13 @@ bool ProtocolDecl::isComputingRequirementSignature() const {
 
 void ProtocolDecl::setRequirementSignature(RequirementSignature requirementSig) {
   RequirementSig = requirementSig;
+  Bits.ProtocolDecl.HasRequirementSignature = 1;
 }
 
 void
 ProtocolDecl::setLazyRequirementSignature(LazyMemberLoader *lazyLoader,
                                           uint64_t requirementSignatureData) {
-  assert(!RequirementSig && "requirement signature already set");
+  assert(!isRequirementSignatureComputed() && "requirement signature already set");
 
   auto contextData = static_cast<LazyProtocolData *>(
       getASTContext().getOrCreateLazyContextData(this, lazyLoader));

lib/AST/NameLookup.cpp

@@ -3219,16 +3219,8 @@ SuperclassDeclRequest::evaluate(Evaluator &evaluator,
   // Protocols may get their superclass bound from a `where Self : Superclass`
   // clause.
   if (auto *proto = dyn_cast<ProtocolDecl>(subject)) {
-    // If the protocol came from a serialized module, compute the superclass via
-    // its generic signature.
-    if (proto->wasDeserialized()) {
-      auto superTy = proto->getGenericSignature()
-          ->getSuperclassBound(proto->getSelfInterfaceType());
-      if (superTy)
-        return superTy->getClassOrBoundGenericClass();
-    }
+    assert(!proto->wasDeserialized());
 
-    // Otherwise check the where clause.
     auto selfBounds = getSelfBoundsFromWhereClause(proto);
     for (auto inheritedNominal : selfBounds.decls)
       if (auto classDecl = dyn_cast<ClassDecl>(inheritedNominal))

lib/AST/RequirementMachine/ConcreteContraction.cpp

@@ -624,19 +624,26 @@ bool ConcreteContraction::performConcreteContraction(
 
     auto superclassTy = *found->second.begin();
 
-    for (const auto *proto : pair.second) {
-      if (auto otherSuperclassTy = proto->getSuperclass()) {
-        if (Debug) {
-          llvm::dbgs() << "@ Subject type of superclass requirement "
-                       << subjectType << " : " << superclassTy
-                       << " conforms to "<< proto->getName()
-                       << " which has a superclass bound "
-                       << otherSuperclassTy << "\n";
-        }
-
-        if (superclassTy->isEqual(otherSuperclassTy)) {
-          Superclasses.erase(subjectType);
-          break;
+    for (auto *proto : pair.second) {
+      auto *module = proto->getParentModule();
+      if (module->lookupConformance(superclassTy, proto)) {
+        auto genericSig = proto->getGenericSignature();
+        // FIXME: If we end up here while building the requirement
+        // signature of `proto`, we will hit a request cycle.
+        if (auto otherSuperclassTy = genericSig->getSuperclassBound(
+                proto->getSelfInterfaceType())) {
+          if (Debug) {
+            llvm::dbgs() << "@ Subject type of superclass requirement "
+                         << subjectType << " : " << superclassTy
+                         << " conforms to "<< proto->getName()
+                         << " which has a superclass bound "
+                         << otherSuperclassTy << "\n";
+          }
+
+          if (superclassTy->isEqual(otherSuperclassTy)) {
+            Superclasses.erase(subjectType);
+            break;
+          }
         }
       }
     }

lib/AST/Type.cpp

@@ -407,14 +407,9 @@ Type ExistentialLayout::getSuperclass() const {
     return explicitSuperclass;
 
   for (auto protoDecl : getProtocols()) {
-    // If we have a generic signature, check there, because it
-    // will pick up superclass constraints from protocols that we
-    // refine as well.
-    if (auto genericSig = protoDecl->getGenericSignature()) {
-      if (auto superclass = genericSig->getSuperclassBound(
-            protoDecl->getSelfInterfaceType()))
-        return superclass;
-    } else if (auto superclass = protoDecl->getSuperclass())
+    auto genericSig = protoDecl->getGenericSignature();
+    if (auto superclass = genericSig->getSuperclassBound(
+          protoDecl->getSelfInterfaceType()))
       return superclass;
   }
 

lib/AST/TypeCheckRequests.cpp

@@ -143,27 +143,16 @@ bool SuperclassTypeRequest::isCached() const {
 }
 
 std::optional<Type> SuperclassTypeRequest::getCachedResult() const {
-  auto nominalDecl = std::get<0>(getStorage());
-
-  if (auto *classDecl = dyn_cast<ClassDecl>(nominalDecl))
-    if (classDecl->LazySemanticInfo.SuperclassType.getInt())
-      return classDecl->LazySemanticInfo.SuperclassType.getPointer();
-
-  if (auto *protocolDecl = dyn_cast<ProtocolDecl>(nominalDecl))
-    if (protocolDecl->LazySemanticInfo.SuperclassType.getInt())
-      return protocolDecl->LazySemanticInfo.SuperclassType.getPointer();
+  auto classDecl = std::get<0>(getStorage());
+  if (classDecl->LazySemanticInfo.SuperclassType.getInt())
+    return classDecl->LazySemanticInfo.SuperclassType.getPointer();
 
   return std::nullopt;
 }
 
 void SuperclassTypeRequest::cacheResult(Type value) const {
-  auto nominalDecl = std::get<0>(getStorage());
-
-  if (auto *classDecl = dyn_cast<ClassDecl>(nominalDecl))
-    classDecl->LazySemanticInfo.SuperclassType.setPointerAndInt(value, true);
-
-  if (auto *protocolDecl = dyn_cast<ProtocolDecl>(nominalDecl))
-    protocolDecl->LazySemanticInfo.SuperclassType.setPointerAndInt(value, true);
+  auto classDecl = std::get<0>(getStorage());
+  classDecl->LazySemanticInfo.SuperclassType.setPointerAndInt(value, true);
 }
 
 void SuperclassTypeRequest::writeDependencySink(
@@ -341,7 +330,7 @@ std::optional<RequirementSignature>
 RequirementSignatureRequest::getCachedResult() const {
   auto proto = std::get<0>(getStorage());
   if (proto->isRequirementSignatureComputed())
-    return *proto->RequirementSig;
+    return proto->RequirementSig;
 
   return std::nullopt;
 }

lib/Sema/CSSimplify.cpp

@@ -4118,14 +4118,6 @@ ConstraintSystem::matchExistentialTypes(Type type1, Type type2,
   }
 
   for (auto *protoDecl : layout.getProtocols()) {
-    if (auto superclass = protoDecl->getSuperclass()) {
-      auto subKind = std::min(ConstraintKind::Subtype, kind);
-      auto result = matchTypes(type1, superclass, subKind,
-                               subflags, locator);
-      if (result.isFailure())
-        return result;
-    }
-
     switch (simplifyConformsToConstraint(type1, protoDecl, kind, locator,
                                          subflags)) {
       case SolutionKind::Solved: