Sema: Allow T == NonCopyableOrEscapable same-type constraints without a redundant T: ~Copyable.

Enhance the logic in `applyInverses` to also take into account same-type constraints spelled in
the generic signature, so that same-type-constraining a type parameter to a type that is itself
not `Copyable` or `Escapable` suppresses the default application of those constraints on the
type parameter. Fixes rdar://147757973.

Author: jckarter

lib/AST/RequirementMachine/RequirementLowering.cpp

@@ -159,6 +159,7 @@
 #include "swift/AST/TypeMatcher.h"
 #include "swift/AST/TypeRepr.h"
 #include "swift/Basic/Assertions.h"
+#include "swift/Basic/Defer.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/SetVector.h"
 #include "Diagnostics.h"
@@ -802,15 +803,122 @@ void swift::rewriting::applyInverses(
     ASTContext &ctx,
     ArrayRef<Type> gps,
     ArrayRef<InverseRequirement> inverseList,
+    ArrayRef<StructuralRequirement> explicitRequirements,
     SmallVectorImpl<StructuralRequirement> &result,
     SmallVectorImpl<RequirementError> &errors) {
 
-  // No inverses to even validate.
-  if (inverseList.empty())
+  // Are there even any inverses or same-type requirements to validate?
+  if (inverseList.empty() && explicitRequirements.empty()) {
     return;
+  }
 
   const bool allowInverseOnAssocType =
       ctx.LangOpts.hasFeature(Feature::SuppressedAssociatedTypes);
+      
+  llvm::DenseMap<CanType, CanType> representativeGPs;
+  
+  // Start with an identity mapping.
+  for (auto gp : gps) {
+    auto canGP = gp->getCanonicalType();
+    representativeGPs.insert({canGP, canGP});
+  }
+  bool hadSameTypeConstraintInScope = false;
+
+  // Return the in-scope generic parameter that represents the equivalence class
+  // for `gp`, or return null if the parameter is constrained out of scope.
+  auto representativeGPFor = [&](CanType gp) -> CanType {
+    while (true) {
+      auto found = representativeGPs.find(gp);
+      if (found == representativeGPs.end()) {
+        return CanType();
+      }
+      if (found->second == CanType()) {
+        return CanType();
+      }
+      
+      if (found->second == gp) {
+        return gp;
+      }
+      
+      gp = found->second;
+    }
+  };
+
+  // Look for same-type constraints that equate multiple generic parameters
+  // within the scope so we can treat the equivalence class as a unit.
+  for (auto &explicitReqt : explicitRequirements) {
+    if (explicitReqt.req.getKind() != RequirementKind::SameType) {
+      continue;
+    }
+
+    // If one end of the same-type requirement is in scope, and the other is
+    // a concrete type or out-of-scope generic parameter, then the other
+    // parameter is also effectively out of scope.
+    auto firstTy = explicitReqt.req.getFirstType()->getCanonicalType();
+    auto secondTy = explicitReqt.req.getSecondType()->getCanonicalType();
+    if (!representativeGPs.count(firstTy)
+        && !representativeGPs.count(secondTy)) {
+      // Same type constraint doesn't involve any in-scope generic parameters.
+      continue;
+    }
+
+    CanType typeInScope;
+    CanType typeOutOfScope;
+
+    if (representativeGPs.count(firstTy)
+        && !representativeGPs.count(secondTy)){
+      // First type is constrained out of scope.
+      typeInScope = firstTy;
+      typeOutOfScope = secondTy;
+    } else if (!representativeGPs.count(firstTy)
+               && representativeGPs.count(secondTy)) {
+      // Second type is constrained out of scope.
+      typeInScope = secondTy;
+      typeOutOfScope = firstTy;
+    } else {
+      // Otherwise, both ends of the same-type constraint are in scope.
+      // Fold the lexicographically-greater parameter with the lesser.
+      auto firstGP = cast<GenericTypeParamType>(firstTy);
+      auto secondGP = cast<GenericTypeParamType>(secondTy);
+      
+      if (firstGP == secondGP) {
+        // `T == T` has no effect.
+        continue;
+      }
+      
+      if (firstGP->getDepth() > secondGP->getDepth()
+          || (firstGP->getDepth() == secondGP->getDepth()
+              && firstGP->getIndex() > secondGP->getIndex())) {
+        std::swap(firstGP, secondGP);
+      }
+      
+      hadSameTypeConstraintInScope = true;
+      representativeGPs.insert_or_assign(secondGP, representativeGPFor(firstGP));
+      continue;
+    }
+
+    // If the out-of-scope type is another type parameter or associated type,
+    // then ignore this same-type constraint and allow defaulting to continue.
+    //
+    // It would probably have been more principled to suppress any defaulting
+    // in this case, but this behavior shipped in Swift 6.0 and 6.1, so we
+    // need to maintain source compatibility.
+    if (typeOutOfScope->isTypeParameter()) {
+      continue;
+    }
+    
+    // If the out-of-scope type contains errors, then similarly, ignore the
+    // same type constraint. Any additional diagnostics arising from the type
+    // parameter being left ~Copyable or ~Escapable might be misleading if the
+    // corrected code is attempting to refer to a Copyable or Escapable type.
+    if (typeOutOfScope->hasError()) {
+      continue;
+    }
+    
+    representativeGPs.insert_or_assign(representativeGPFor(typeInScope),
+                                       CanType());
+    hadSameTypeConstraintInScope = true;
+  }
 
   // Summarize the inverses and diagnose ones that are incorrect.
   llvm::DenseMap<CanType, InvertibleProtocolSet> inverses;
@@ -837,11 +945,6 @@ void swift::rewriting::applyInverses(
       continue;
     }
 
-    // WARNING: possible quadratic behavior, but should be OK in practice.
-    auto notInScope = llvm::none_of(gps, [=](Type t) {
-      return t->getCanonicalType() == canSubject;
-    });
-
     // If the inverse is on a subject that wasn't permitted by our caller, then
     // remove and diagnose as an error. This can happen when an inner context
     // has a constraint on some outer generic parameter, e.g.,
@@ -850,27 +953,40 @@ void swift::rewriting::applyInverses(
     //       func f() where Self: ~Copyable
     //     }
     //
-    if (notInScope) {
+    if (representativeGPs.find(canSubject) == representativeGPs.end()) {
       errors.push_back(
           RequirementError::forInvalidInverseOuterSubject(inverse));
       continue;
     }
 
-    auto state = inverses.getOrInsertDefault(canSubject);
+    auto representativeSubject = representativeGPFor(canSubject);
+    
+    // If the subject is in scope, but same-type constrained to a type out of
+    // scope, then allow inverses to be stated even though they are redundant.
+    // This is because older versions of Swift not only accepted but required
+    // `extension Foo where T == NonCopyableType, T: ~Copyable {}` to be
+    // written, so we need to continue to accept that formulation for source
+    // compatibility.
+    if (!representativeSubject) {
+      continue;
+    }
+
+    auto state = inverses.getOrInsertDefault(representativeSubject);
 
     // Check if this inverse has already been seen.
     auto inverseKind = inverse.getKind();
     if (state.contains(inverseKind))
       continue;
 
     state.insert(inverseKind);
-    inverses[canSubject] = state;
+    inverses[representativeSubject] = state;
   }
 
-  // Fast-path: if there are no valid inverses, then there are no requirements
-  // to be removed.
-  if (inverses.empty())
+  // Fast-path: if there are no valid inverses or same-type constraints, then
+  // there are no requirements to be removed.
+  if (inverses.empty() && !hadSameTypeConstraintInScope) {
     return;
+  }
 
   // Scan the structural requirements and cancel out any inferred requirements
   // based on the inverses we saw.
@@ -882,18 +998,30 @@ void swift::rewriting::applyInverses(
 
     // Only consider requirements involving an invertible protocol.
     auto proto = req.getProtocolDecl()->getInvertibleProtocolKind();
-    if (!proto)
+    if (!proto) {
       return false;
+    }
 
     // See if this subject is in-scope.
     auto subject = req.getFirstType()->getCanonicalType();
-    auto result = inverses.find(subject);
-    if (result == inverses.end())
+    auto representative = representativeGPs.find(subject);
+    if (representative == representativeGPs.end()) {
       return false;
+    }
+      
+    // If this type is same-type constrained into another equivalence class,
+    // then it doesn't need its own defaulted requirements.
+    if (representative->second != subject) {
+      return true;
+    }
 
     // We now have found the inferred constraint 'Subject : Proto'.
     // So, remove it if we have recorded a 'Subject : ~Proto'.
-    auto recordedInverses = result->getSecond();
+    auto foundInverses = inverses.find(subject);
+    if (foundInverses == inverses.end()) {
+      return false;
+    }
+    auto recordedInverses = foundInverses->getSecond();
     return recordedInverses.contains(*proto);
   }), result.end());
 }
@@ -1002,7 +1130,8 @@ StructuralRequirementsRequest::evaluate(Evaluator &evaluator,
 
     SmallVector<StructuralRequirement, 2> defaults;
     InverseRequirement::expandDefaults(ctx, needsDefaultRequirements, defaults);
-    applyInverses(ctx, needsDefaultRequirements, inverses, defaults, errors);
+    applyInverses(ctx, needsDefaultRequirements, inverses, result,
+                  defaults, errors);
     result.append(defaults);
 
     diagnoseRequirementErrors(ctx, errors,
@@ -1076,7 +1205,8 @@ StructuralRequirementsRequest::evaluate(Evaluator &evaluator,
       && !proto->isSpecificProtocol(KnownProtocolKind::Sendable))
     InverseRequirement::expandDefaults(ctx, needsDefaultRequirements, defaults);
 
-  applyInverses(ctx, needsDefaultRequirements, inverses, defaults, errors);
+  applyInverses(ctx, needsDefaultRequirements, inverses, result,
+                defaults, errors);
   result.append(defaults);
 
   diagnoseRequirementErrors(ctx, errors,

lib/AST/RequirementMachine/RequirementLowering.h

@@ -64,6 +64,7 @@ void realizeInheritedRequirements(TypeDecl *decl, Type type,
 void applyInverses(ASTContext &ctx,
                    ArrayRef<Type> gps,
                    ArrayRef<InverseRequirement> inverseList,
+                   ArrayRef<StructuralRequirement> explicitRequirements,
                    SmallVectorImpl<StructuralRequirement> &result,
                    SmallVectorImpl<RequirementError> &errors);
 

lib/AST/RequirementMachine/RequirementMachineRequests.cpp

@@ -671,7 +671,8 @@ AbstractGenericSignatureRequest::evaluate(
 
   SmallVector<StructuralRequirement, 2> defaults;
   InverseRequirement::expandDefaults(ctx, paramsAsTypes, defaults);
-  applyInverses(ctx, paramsAsTypes, inverses, defaults, errors);
+  applyInverses(ctx, paramsAsTypes, inverses, requirements,
+                defaults, errors);
   requirements.append(defaults);
 
   auto &rewriteCtx = ctx.getRewriteContext();
@@ -884,7 +885,8 @@ InferredGenericSignatureRequest::evaluate(
 
   SmallVector<StructuralRequirement, 2> defaults;
   InverseRequirement::expandDefaults(ctx, paramTypes, defaults);
-  applyInverses(ctx, paramTypes, inverses, defaults, errors);
+  applyInverses(ctx, paramTypes, inverses, requirements,
+                defaults, errors);
 
   // Any remaining implicit defaults in a conditional inverse requirement
   // extension must be made explicit.

test/Generics/inverse_generics.swift

@@ -365,9 +365,9 @@ func conflict13<T>(_ t: T)
         {}
 
 // expected-warning@+1 {{same-type requirement makes generic parameters 'U' and 'T' equivalent}}
-func conflict14<T, U>(_ t: T, _ u: U)
-  where T: ~Copyable, // expected-error {{'T' required to be 'Copyable' but is marked with '~Copyable'}}
-        U: ~Escapable, // expected-error {{'U' required to be 'Escapable' but is marked with '~Escapable'}}
+func conflict14<T, U>(_ t: borrowing T, _ u: borrowing U)
+  where T: ~Copyable,
+        U: ~Escapable,
         T == U {}
 
 protocol Conflict15 {

test/Sema/copyable_constraint_defaulting_with_same_type_constraints.swift

@@ -0,0 +1,77 @@
+// RUN: %target-swift-frontend -typecheck -verify %s
+
+struct Bar: ~Copyable {}
+
+struct Foo<T: ~Copyable> {
+    func foo() -> T { fatalError() }
+}
+
+extension Foo where T == Bar {
+    func bar() -> Bar {
+        return foo()
+    }
+}
+
+struct Foo2<T: ~Copyable, U: ~Copyable> {
+    func foo1() -> T { fatalError() }
+    func foo2() -> U { fatalError() }
+}
+
+extension Foo2 where T == Bar, U == Bar {
+    func bar1_1() -> Bar {
+        return foo1()
+    }
+    func bar1_2() -> Bar {
+        return foo2()
+    }
+}
+
+extension Foo2 where T == U, U == Bar {
+    func bar2_1() -> Bar {
+        return foo1()
+    }
+    func bar2_2() -> Bar {
+        return foo2()
+    }
+}
+
+extension Foo2 where T == Bar, U == T {
+    func bar3_1() -> Bar {
+        return foo1()
+    }
+    func bar3_2() -> Bar {
+        return foo2()
+    }
+}
+
+func needsCopyable<T>(_: T) {}
+
+// T and U are still Copyable by default here, since the equivalence class of
+// the same-type-constrained parameters is entirely within the scope of the
+// extension, and there is no explicit suppression of the default.
+extension Foo2 where T == U {
+    func test(_ x: T) {
+        needsCopyable(x)
+    }
+}
+
+// Explicitly suppressing Copyable on one type parameter also prevents the other
+// parameters in the equivalence class from defaulting to Copyable.
+extension Foo2 where T == U, T: ~Copyable {
+    func bar4_1() -> T {
+        return foo1()
+    }
+    func bar4_2() -> T {
+        return foo2()
+    }
+}
+
+extension Foo2 where T == U, U: ~Copyable {
+    func bar5_1() -> T {
+        return foo1()
+    }
+    func bar5_2() -> T {
+        return foo2()
+    }
+}
+