Sema: Redo the normalization in TypeResolver::resolveCompositionType()

Author: slavapestov

lib/AST/ASTContext.cpp

@@ -3914,15 +3914,6 @@ ProtocolCompositionType::build(const ASTContext &C, ArrayRef<Type> Members,
                                bool HasExplicitAnyObject) {
   assert(Members.size() != 1 || HasExplicitAnyObject || !Inverses.empty());
 
-#ifndef NDEBUG
-  for (auto member : Members) {
-    if (auto *proto = member->getAs<ProtocolType>()) {
-      assert(!proto->getDecl()->getInvertibleProtocolKind() &&
-             "Should have been folded away");
-    }
-  }
-#endif
-
   // Check to see if we've already seen this protocol composition before.
   void *InsertPos = nullptr;
   llvm::FoldingSetNodeID ID;

lib/AST/Type.cpp

@@ -3777,13 +3777,6 @@ Type ProtocolCompositionType::get(const ASTContext &C,
       }
     }
 
-    // Drop any explicitly provided invertible protocols.
-    if (auto ip = proto->getInvertibleProtocolKind()) {
-      // We diagnose '~Copyable & Copyable' before forming the PCT.
-      assert(!Inverses.contains(*ip) && "opposing invertible constraints!");
-      continue;
-    }
-
     CanTypes.push_back(proto->getDeclaredInterfaceType());
   }
 

lib/Sema/TypeCheckType.cpp

@@ -5485,146 +5485,89 @@ NeverNullType TypeResolver::resolveTupleType(TupleTypeRepr *repr,
   return TupleType::get(elements, ctx);
 }
 
-/// \returns the inverse ~P that is conflicted if a protocol in \c tys
-/// requires P. For example, `Q & ~Copyable` is a conflict, if `Q` requires
-/// or is equal to `Copyable`
-static std::optional<InvertibleProtocolKind>
-hasConflictedInverse(ArrayRef<Type> tys, InvertibleProtocolSet inverses) {
-  // Fast-path: no inverses that could be conflicted!
-  if (inverses.empty())
-    return std::nullopt;
-
-  for (auto ty : tys) {
-    // Handle nested PCT's recursively since we haven't flattened them away yet.
-    if (auto pct = dyn_cast<ProtocolCompositionType>(ty)) {
-      if (auto conflict = hasConflictedInverse(pct->getMembers(), inverses))
-        return conflict;
-      continue;
-    }
-
-    // Dig out a protocol.
-    ProtocolDecl *decl = nullptr;
-    if (auto protoTy = dyn_cast<ProtocolType>(ty))
-      decl = protoTy->getDecl();
-    else if (auto paramProtoTy = dyn_cast<ParameterizedProtocolType>(ty))
-      decl = paramProtoTy->getProtocol();
-
-    if (!decl)
-      continue;
-
-    // If an inverse ~I exists for this protocol member of the PCT that
-    // requires I, then it's a conflict.
-    for (auto inverse : inverses) {
-      if (decl->isSpecificProtocol(getKnownProtocolKind(inverse))
-          || decl->requiresInvertible(inverse)) {
-        return inverse;
-      }
-    }
-  }
-
-  return std::nullopt;
-}
-
 NeverNullType
 TypeResolver::resolveCompositionType(CompositionTypeRepr *repr,
                                      TypeResolutionOptions options) {
 
+  SmallVector<Type, 4> Members;
+
   // Note that the superclass type will appear as part of one of the
   // types in 'Members', so it's not used when constructing the
   // fully-realized type below -- but we just record it to make sure
   // there is only one superclass.
   Type SuperclassType;
-  SmallVector<Type, 4> Members;
-  InvertibleProtocolSet Inverses;
-  bool HasAnyObject = false;
 
-  // Whether we saw at least one protocol. A protocol composition
-  // must either be empty (in which case it is Any or AnyObject),
-  // or if it has a superclass constraint, have at least one protocol.
-  bool HasProtocol = false;
+  // Did we see at least one protocol or inverse?
+  bool HasNonClassMember = false;
 
-  auto checkSuperclass = [&](SourceLoc loc, Type t) -> bool {
-    if (SuperclassType && !SuperclassType->isEqual(t)) {
-      diagnose(loc, diag::protocol_composition_one_class, t,
+  // If true, we cannot form a composition from these members.
+  bool IsInvalid = false;
+
+  std::function<void (SourceLoc, Type)> checkMember
+      = [&](SourceLoc loc, Type ty) {
+    if (auto pct = ty->getAs<ProtocolCompositionType>()) {
+      if (!pct->getInverses().empty())
+        HasNonClassMember = true;
+
+      for (auto member : pct->getMembers())
+        checkMember(loc, member);
+      return;
+    }
+
+    if (ty->is<ProtocolType>() ||
+        ty->is<ParameterizedProtocolType>()) {
+      HasNonClassMember = true;
+      return;
+    }
+
+    assert(isa<ClassDecl>(ty->getAnyNominal()));
+
+    if (SuperclassType && !SuperclassType->isEqual(ty)) {
+      diagnose(loc, diag::protocol_composition_one_class, ty,
                SuperclassType);
-      return true;
+      IsInvalid = true;
+      return;
     }
 
-    SuperclassType = t;
-    return false;
+    SuperclassType = ty;
   };
 
-  bool IsInvalid = false;
-
   for (auto tyR : repr->getTypes()) {
     auto ty = resolveType(tyR,
         options.withContext(TypeResolverContext::GenericRequirement));
     if (ty->hasError()) return ty;
 
-    auto nominalDecl = ty->getAnyNominal();
-    if (isa_and_nonnull<ClassDecl>(nominalDecl)) {
-      if (checkSuperclass(tyR->getStartLoc(), ty))
-        continue;
-
+    if (ty->is<ProtocolType>()) {
+      checkMember(tyR->getStartLoc(), ty);
       Members.push_back(ty);
       continue;
     }
 
     // FIXME: Support compositions involving parameterized protocol types,
     // like 'any Collection<String> & Sendable', etc.
-    if (ty->isConstraintType()) {
-      if (ty->is<ProtocolType>()) {
-        HasProtocol = true;
-        Members.push_back(ty);
-        continue;
-      }
-
-      if (ty->is<ParameterizedProtocolType>() &&
-          !options.isConstraintImplicitExistential() &&
-          options.getContext() != TypeResolverContext::ExistentialConstraint) {
-        HasProtocol = true;
-        Members.push_back(ty);
-        continue;
-      }
+    if (ty->is<ParameterizedProtocolType>() &&
+        !options.isConstraintImplicitExistential() &&
+        options.getContext() != TypeResolverContext::ExistentialConstraint) {
+      checkMember(tyR->getStartLoc(), ty);
+      Members.push_back(ty);
+      continue;
+    }
 
-      if (auto pct = ty->getAs<ProtocolCompositionType>()) {
-        auto layout = ty->getExistentialLayout();
-        if (auto superclass = layout.explicitSuperclass)
-          if (checkSuperclass(tyR->getStartLoc(), superclass))
-            continue;
-        if (!layout.getProtocols().empty())
-          HasProtocol = true;
-        if (layout.hasExplicitAnyObject)
-          HasAnyObject = true;
+    if (ty->is<ProtocolCompositionType>()) {
+      checkMember(tyR->getStartLoc(), ty);
+      Members.push_back(ty);
+      continue;
+    }
 
-        Inverses.insertAll(pct->getInverses());
-        Members.push_back(ty);
-        continue;
-      }
+    if (isa_and_nonnull<ClassDecl>(ty->getAnyNominal())) {
+      checkMember(tyR->getStartLoc(), ty);
+      Members.push_back(ty);
+      continue;
     }
 
     diagnose(tyR->getStartLoc(),
              diag::invalid_protocol_composition_member,
              ty);
-
-    IsInvalid = true;
-  }
-
-  // Cannot combine inverses with Superclass or AnyObject in a composition.
-  if ((SuperclassType || HasAnyObject) && !Inverses.empty()) {
-    diagnose(repr->getStartLoc(),
-             diag::inverse_with_class_constraint,
-             HasAnyObject,
-             getProtocolName(getKnownProtocolKind(*Inverses.begin())),
-             SuperclassType);
-    IsInvalid = true;
-  }
-
-  // Cannot provide an inverse in the same composition requiring the protocol.
-  if (auto conflict = hasConflictedInverse(Members, Inverses)) {
-    diagnose(repr->getLoc(),
-       diag::inverse_conflicts_explicit_composition,
-       getProtocolName(getKnownProtocolKind(*conflict)));
     IsInvalid = true;
   }
 
@@ -5636,17 +5579,58 @@ TypeResolver::resolveCompositionType(CompositionTypeRepr *repr,
   // Avoid confusing diagnostics ('MyClass' not convertible to 'MyClass',
   // etc) by collapsing a composition consisting of a single class down
   // to the class itself.
-  if (SuperclassType && !HasProtocol)
+  if (SuperclassType && !HasNonClassMember)
     return SuperclassType;
 
-  // In user-written types, AnyObject constraints always refer to the
-  // AnyObject type in the standard library.
   auto composition =
-      ProtocolCompositionType::get(getASTContext(), Members, Inverses,
+      ProtocolCompositionType::get(getASTContext(), Members,
+                                   /*Inverses=*/{},
                                    /*HasExplicitAnyObject=*/false);
+
+  // Flatten the composition.
+  if (auto canComposition = dyn_cast<ProtocolCompositionType>(
+          composition->getCanonicalType())) {
+    auto inverses = canComposition->getInverses();
+    auto layout = composition->getExistentialLayout();
+
+    // Cannot provide an inverse in the same composition requiring the protocol.
+    for (auto ip : inverses) {
+      auto kp = getKnownProtocolKind(ip);
+
+      if (layout.requiresClass()) {
+        bool hasExplicitAnyObject = layout.hasExplicitAnyObject;
+        diagnose(repr->getStartLoc(),
+                 diag::inverse_with_class_constraint,
+                 hasExplicitAnyObject,
+                 getProtocolName(kp),
+                 layout.getSuperclass());
+        IsInvalid = true;
+        break;
+      }
+
+      auto *proto = getASTContext().getProtocol(kp);
+      for (auto *otherProto : layout.getProtocols()) {
+        if (proto == otherProto ||
+            otherProto->inheritsFrom(proto)) {
+          diagnose(repr->getLoc(),
+                   diag::inverse_conflicts_explicit_composition,
+                   getProtocolName(kp));
+          IsInvalid = true;
+          break;
+        }
+      }
+    }
+  }
+
+  if (IsInvalid) {
+    repr->setInvalid();
+    return ErrorType::get(getASTContext());
+  }
+
   if (options.isConstraintImplicitExistential()) {
     return ExistentialType::get(composition);
   }
+
   return composition;
 }
 

test/Generics/inverse_generics.swift

@@ -409,7 +409,6 @@ func checkExistentialAndClasses(
     _ a: any AnyObject & ~Copyable, // expected-error {{composition involving 'AnyObject' cannot contain '~Copyable'}}
     _ b: any Soup & Copyable & ~Escapable & ~Copyable,
     // expected-error@-1 {{composition involving class requirement 'Soup' cannot contain '~Copyable'}}
-    // expected-error@-2 {{composition cannot contain '~Copyable' when another member requires 'Copyable'}}
     _ c: some (~Escapable & Removed) & Soup // expected-error {{composition cannot contain '~Escapable' when another member requires 'Escapable'}}
     ) {}
 

test/Interpreter/subclass_existentials.swift

@@ -15,8 +15,6 @@
 // RUN: %target-codesign %t/a.out
 // RUN: %target-run %t/a.out
 
-// XFAIL: noncopyable_generics
-
 // REQUIRES: executable_test
 
 import StdlibUnittest

test/type/subclass_composition.swift

@@ -1,7 +1,5 @@
 // RUN: %target-typecheck-verify-swift
 
-// XFAIL: noncopyable_generics
-
 protocol P1 {
   typealias DependentInConcreteConformance = Self
 }