Merge pull request #41650 from slavapestov/rqm-inferred-signatures-on-by-default

RequirementMachine: Enable -requirement-machine-inferred-signatures=verify by default

Author: slavapestov

lib/AST/RequirementMachine/RequirementMachineRequests.cpp

@@ -243,6 +243,59 @@ static bool isCanonicalRequest(GenericSignature baseSignature,
   return true;
 }
 
+/// Hack for GenericSignatureBuilder compatibility. We might end up with a
+/// same-type requirement between type parameters where one of them has an
+/// implied concrete type requirement. In this case, split it up into two
+/// concrete type requirements.
+static bool shouldSplitConcreteEquivalenceClass(Requirement req,
+                                                GenericSignature sig) {
+  return (req.getKind() == RequirementKind::SameType &&
+          req.getSecondType()->isTypeParameter() &&
+          sig->isConcreteType(req.getSecondType()));
+}
+
+static bool shouldSplitConcreteEquivalenceClasses(GenericSignature sig) {
+  for (auto req : sig.getRequirements()) {
+    if (shouldSplitConcreteEquivalenceClass(req, sig))
+      return true;
+  }
+
+  return false;
+}
+
+static GenericSignature splitConcreteEquivalenceClasses(
+    GenericSignature sig, ASTContext &ctx) {
+  SmallVector<Requirement, 2> reqs;
+
+  for (auto req : sig.getRequirements()) {
+    if (shouldSplitConcreteEquivalenceClass(req, sig)) {
+      auto canType = sig->getSugaredType(
+        sig.getCanonicalTypeInContext(
+          req.getSecondType()));
+
+      reqs.emplace_back(RequirementKind::SameType,
+                        req.getFirstType(),
+                        canType);
+      reqs.emplace_back(RequirementKind::SameType,
+                        req.getSecondType(),
+                        canType);
+    } else {
+      reqs.push_back(req);
+    }
+  }
+
+  SmallVector<GenericTypeParamType *, 2> genericParams;
+  genericParams.append(sig.getGenericParams().begin(),
+                       sig.getGenericParams().end());
+
+  return evaluateOrDefault(
+      ctx.evaluator,
+      AbstractGenericSignatureRequestRQM{
+        /*baseSignature=*/nullptr,
+        genericParams, reqs},
+      GenericSignatureWithError()).getPointer();
+}
+
 GenericSignatureWithError
 AbstractGenericSignatureRequestRQM::evaluate(
          Evaluator &evaluator,
@@ -391,8 +444,13 @@ AbstractGenericSignatureRequestRQM::evaluate(
   auto result = GenericSignature::get(genericParams, minimalRequirements);
   auto errorFlags = machine->getErrors();
 
-  if (!errorFlags)
+  if (!errorFlags) {
+    if (shouldSplitConcreteEquivalenceClasses(result))
+      result = splitConcreteEquivalenceClasses(result, ctx);
+
+    // Check invariants.
     result.verify();
+  }
 
   return GenericSignatureWithError(result, errorFlags);
 }
@@ -543,9 +601,13 @@ InferredGenericSignatureRequestRQM::evaluate(
 
   // FIXME: Handle allowConcreteGenericParams
 
-  // Check invariants.
-  if (!errorFlags)
+  if (!errorFlags) {
+    if (shouldSplitConcreteEquivalenceClasses(result))
+      result = splitConcreteEquivalenceClasses(result, ctx);
+
+    // Check invariants.
     result.verify();
+  }
 
   return GenericSignatureWithError(result, errorFlags);
 }

lib/Frontend/CompilerInvocation.cpp

@@ -913,6 +913,7 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
       Args.hasArg(OPT_disable_subst_sil_function_types);
 
   Opts.RequirementMachineProtocolSignatures = RequirementMachineMode::Verify;
+  Opts.RequirementMachineInferredSignatures = RequirementMachineMode::Verify;
   Opts.RequirementMachineAbstractSignatures = RequirementMachineMode::Verify;
 
   if (auto A = Args.getLastArg(OPT_requirement_machine_protocol_signatures_EQ)) {

test/Generics/rdar90402519.swift

@@ -0,0 +1,27 @@
+// RUN: %target-typecheck-verify-swift
+
+// This needs a better diagnostic. The real problem is the 'C == G<I>'
+// requirement in P2 conflicts with the one in P1.
+
+protocol P {
+  associatedtype I
+}
+
+struct G1<I> : P {}
+struct G2<T> : P {
+  typealias I = G<T>
+}
+
+struct G<T> {}
+
+protocol P0 {
+  associatedtype C : P
+}
+
+protocol P1 : P0 where C == G1<I> {
+  associatedtype I
+}
+
+protocol P2 : P1 where C == G2<I> {}
+// expected-error@-1 {{cannot build rewrite system for protocol; concrete nesting limit exceeded}}
+// expected-note@-2 {{failed rewrite rule is [P2:I].[concrete: G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<[P2:I]>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>] => [P2:I]}}

test/Generics/split_concrete_equivalence_class.swift

@@ -0,0 +1,37 @@
+// RUN: %target-swift-frontend -typecheck %s -debug-generic-signatures -requirement-machine-inferred-signatures=on 2>&1 | %FileCheck %s
+
+// CHECK-LABEL: split_concrete_equivalence_class.(file).f01@
+// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Collection]SubSequence == Substring, R.[Sequence]Element == Character>
+func f01<C : Collection, R : Collection>(_: C, _: R) where C.SubSequence == Substring, C.Element == R.Element {}
+
+// CHECK-LABEL: split_concrete_equivalence_class.(file).f02@
+// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Sequence]Element == Character, R.[Collection]SubSequence == Substring>
+func f02<C : Collection, R : Collection>(_: C, _: R) where R.SubSequence == Substring, C.Element == R.Element {}
+
+// CHECK-LABEL: split_concrete_equivalence_class.(file).f03@
+// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Collection]SubSequence == Substring, R.[Sequence]Element == Character>
+func f03<C : Collection, R : Collection>(_: C, _: R) where C.SubSequence == Substring, C.Element == R.Element, C.Element == Character {}
+
+// CHECK-LABEL: split_concrete_equivalence_class.(file).f04@
+// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Sequence]Element == Character, R.[Collection]SubSequence == Substring>
+func f04<C : Collection, R : Collection>(_: C, _: R) where R.SubSequence == Substring, C.Element == R.Element, C.Element == Character {}
+
+// CHECK-LABEL: split_concrete_equivalence_class.(file).f05@
+// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Collection]SubSequence == Substring, R.[Sequence]Element == Character>
+func f05<C : Collection, R : Collection>(_: C, _: R) where C.SubSequence == Substring, C.Element == R.Element, R.Element == Character {}
+
+// CHECK-LABEL: split_concrete_equivalence_class.(file).f06@
+// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Sequence]Element == Character, R.[Collection]SubSequence == Substring>
+func f06<C : Collection, R : Collection>(_: C, _: R) where R.SubSequence == Substring, C.Element == R.Element, R.Element == Character {}
+
+// CHECK-LABEL: split_concrete_equivalence_class.(file).f07@
+// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Collection]SubSequence == Substring, R.[Collection]SubSequence == Substring>
+func f07<C : Collection, R : Collection>(_: C, _: R) where C.SubSequence == Substring, R.SubSequence == Substring, C.Element == R.Element {}
+
+// CHECK-LABEL: split_concrete_equivalence_class.(file).f08@
+// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Collection]SubSequence == Substring, R.[Collection]SubSequence == Substring>
+func f08<C : Collection, R : Collection>(_: C, _: R) where C.SubSequence == Substring, R.SubSequence == Substring, C.Element == R.Element, C.Element == Character {}
+
+// CHECK-LABEL: split_concrete_equivalence_class.(file).f09@
+// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Collection]SubSequence == Substring, R.[Collection]SubSequence == Substring>
+func f09<C : Collection, R : Collection>(_: C, _: R) where C.SubSequence == Substring, R.SubSequence == Substring, C.Element == R.Element, R.Element == Character {}

test/Generics/unify_superclass_types_2.swift

@@ -1,8 +1,12 @@
-// RUN: %target-typecheck-verify-swift -dump-requirement-machine 2>&1 | %FileCheck %s
+// RUN: %target-typecheck-verify-swift -debug-generic-signatures -requirement-machine-inferred-signatures=off 2>&1 | %FileCheck %s
+// RUN: %target-typecheck-verify-swift -dump-requirement-machine -requirement-machine-inferred-signatures=off 2>&1 | %FileCheck %s --check-prefix=CHECK-RULE
 
 // Note: The GSB fails this test, because it doesn't implement unification of
 // superclass type constructor arguments.
 
+// FIXME: The Requirement Machine also fails to minimize the signature of
+// unifySuperclassTest(). rdar://90469643
+
 class Generic<T, U, V> {}
 
 protocol P1 {
@@ -21,30 +25,32 @@ func sameType<T>(_: T.Type, _: T.Type) {}
 
 func takesGenericIntString<U>(_: Generic<Int, String, U>.Type) {}
 
+// CHECK-LABEL: .unifySuperclassTest@
+// CHECK-NEXT: Generic signature: <T where T : P1, T : P2>
 func unifySuperclassTest<T : P1 & P2>(_: T) {
   sameType(T.A1.self, String.self)
   sameType(T.A2.self, Int.self)
   sameType(T.B1.self, T.B2.self)
   takesGenericIntString(T.X.self)
 }
 
-// CHECK-LABEL: Requirement machine for <τ_0_0 where τ_0_0 : P1, τ_0_0 : P2>
-// CHECK-NEXT: Rewrite system: {
-// CHECK:      - τ_0_0.[P2:X] => τ_0_0.[P1:X]
-// CHECK:      - τ_0_0.[P1:X].[superclass: Generic<τ_0_0.[P2:A2], String, τ_0_0.[P2:B2]>] => τ_0_0.[P1:X]
-// CHECK:      - τ_0_0.[P1:X].[superclass: Generic<Int, String, τ_0_0.[P1:B1]>] => τ_0_0.[P1:X]
-// CHECK:      - τ_0_0.[P1:A1].[concrete: String] => τ_0_0.[P1:A1]
-// CHECK:      - τ_0_0.[P2:A2].[concrete: Int] => τ_0_0.[P2:A2]
-// CHECK:      - τ_0_0.[P2:B2] => τ_0_0.[P1:B1]
-// CHECK:      - τ_0_0.B2 => τ_0_0.[P1:B1]
-// CHECK:      }
-// CHECK: Property map: {
-// CHECK-NEXT:   [P1] => { conforms_to: [P1] }
-// CHECK-NEXT:   [P1:X] => { layout: _NativeClass superclass: [superclass: Generic<Int, [P1:A1], [P1:B1]>] }
-// CHECK-NEXT:   [P2] => { conforms_to: [P2] }
-// CHECK-NEXT:   [P2:X] => { layout: _NativeClass superclass: [superclass: Generic<[P2:A2], String, [P2:B2]>] }
-// CHECK-NEXT:   τ_0_0 => { conforms_to: [P1 P2] }
-// CHECK-NEXT:   τ_0_0.[P1:X] => { layout: _NativeClass superclass: [superclass: Generic<Int, String, τ_0_0.[P1:B1]>] }
-// CHECK-NEXT:   τ_0_0.[P1:A1] => { concrete_type: [concrete: String] }
-// CHECK-NEXT:   τ_0_0.[P2:A2] => { concrete_type: [concrete: Int] }
-// CHECK-NEXT: }
+// CHECK-RULE-LABEL: Requirement machine for <τ_0_0 where τ_0_0 : P1, τ_0_0 : P2>
+// CHECK-RULE-NEXT: Rewrite system: {
+// CHECK-RULE:      - τ_0_0.[P2:X] => τ_0_0.[P1:X]
+// CHECK-RULE:      - τ_0_0.[P1:X].[superclass: Generic<τ_0_0.[P2:A2], String, τ_0_0.[P2:B2]>] => τ_0_0.[P1:X]
+// CHECK-RULE:      - τ_0_0.[P1:X].[superclass: Generic<Int, String, τ_0_0.[P1:B1]>] => τ_0_0.[P1:X]
+// CHECK-RULE:      - τ_0_0.[P1:A1].[concrete: String] => τ_0_0.[P1:A1]
+// CHECK-RULE:      - τ_0_0.[P2:A2].[concrete: Int] => τ_0_0.[P2:A2]
+// CHECK-RULE:      - τ_0_0.[P2:B2] => τ_0_0.[P1:B1]
+// CHECK-RULE:      - τ_0_0.B2 => τ_0_0.[P1:B1]
+// CHECK-RULE:      }
+// CHECK-RULE: Property map: {
+// CHECK-RULE-NEXT:   [P1] => { conforms_to: [P1] }
+// CHECK-RULE-NEXT:   [P1:X] => { layout: _NativeClass superclass: [superclass: Generic<Int, [P1:A1], [P1:B1]>] }
+// CHECK-RULE-NEXT:   [P2] => { conforms_to: [P2] }
+// CHECK-RULE-NEXT:   [P2:X] => { layout: _NativeClass superclass: [superclass: Generic<[P2:A2], String, [P2:B2]>] }
+// CHECK-RULE-NEXT:   τ_0_0 => { conforms_to: [P1 P2] }
+// CHECK-RULE-NEXT:   τ_0_0.[P1:X] => { layout: _NativeClass superclass: [superclass: Generic<Int, String, τ_0_0.[P1:B1]>] }
+// CHECK-RULE-NEXT:   τ_0_0.[P1:A1] => { concrete_type: [concrete: String] }
+// CHECK-RULE-NEXT:   τ_0_0.[P2:A2] => { concrete_type: [concrete: Int] }
+// CHECK-RULE-NEXT: }

test/Generics/unify_superclass_types_3.swift

@@ -1,8 +1,12 @@
-// RUN: %target-typecheck-verify-swift -dump-requirement-machine 2>&1 | %FileCheck %s
+// RUN: %target-typecheck-verify-swift -debug-generic-signatures -requirement-machine-inferred-signatures=off 2>&1 | %FileCheck %s
+// RUN: %target-typecheck-verify-swift -dump-requirement-machine -requirement-machine-inferred-signatures=off 2>&1 | %FileCheck %s --check-prefix=CHECK-RULE
 
 // Note: The GSB fails this test, because it doesn't implement unification of
 // superclass type constructor arguments.
 
+// FIXME: The Requirement Machine also fails to minimize the signature of
+// unifySuperclassTest(). rdar://90469643
+
 class Generic<T, U, V> {}
 
 class Derived<TT, UU> : Generic<Int, TT, UU> {}
@@ -23,32 +27,34 @@ func sameType<T>(_: T.Type, _: T.Type) {}
 
 func takesDerivedString<U>(_: Derived<String, U>.Type) {}
 
+// CHECK-LABEL: .unifySuperclassTest@
+// CHECK-NEXT: Generic signature: <T where T : P1, T : P2>
 func unifySuperclassTest<T : P1 & P2>(_: T) {
   sameType(T.A1.self, String.self)
   sameType(T.A2.self, Int.self)
   sameType(T.B1.self, T.B2.self)
   takesDerivedString(T.X.self)
 }
 
-// CHECK-LABEL: Requirement machine for <τ_0_0 where τ_0_0 : P1, τ_0_0 : P2>
-// CHECK-NEXT: Rewrite system: {
-// CHECK:      - [P1:X].[layout: _NativeClass] => [P1:X]
-// CHECK:      - [P2:X].[layout: _NativeClass] => [P2:X]
-// CHECK:      - τ_0_0.[P2:X] => τ_0_0.[P1:X]
-// CHECK:      - τ_0_0.[P1:X].[superclass: Generic<Int, τ_0_0.[P1:A1], τ_0_0.[P1:B1]>] => τ_0_0.[P1:X]
-// CHECK:      - τ_0_0.[P1:X].[superclass: Generic<Int, String, τ_0_0.[P1:B1]>] => τ_0_0.[P1:X]
-// CHECK:      - τ_0_0.[P2:A2].[concrete: Int] => τ_0_0.[P2:A2]
-// CHECK:      - τ_0_0.[P2:B2] => τ_0_0.[P1:B1]
-// CHECK:      - τ_0_0.[P1:A1].[concrete: String] => τ_0_0.[P1:A1]
-// CHECK:      - τ_0_0.B2 => τ_0_0.[P1:B1]
-// CHECK:      }
-// CHECK: Property map: {
-// CHECK-NEXT:   [P1] => { conforms_to: [P1] }
-// CHECK-NEXT:   [P1:X] => { layout: _NativeClass superclass: [superclass: Derived<[P1:A1], [P1:B1]>] }
-// CHECK-NEXT:   [P2] => { conforms_to: [P2] }
-// CHECK-NEXT:   [P2:X] => { layout: _NativeClass superclass: [superclass: Generic<[P2:A2], String, [P2:B2]>] }
-// CHECK-NEXT:   τ_0_0 => { conforms_to: [P1 P2] }
-// CHECK-NEXT:   τ_0_0.[P1:X] => { layout: _NativeClass superclass: [superclass: Derived<τ_0_0.[P1:A1], τ_0_0.[P1:B1]>] }
-// CHECK-NEXT:   τ_0_0.[P2:A2] => { concrete_type: [concrete: Int] }
-// CHECK-NEXT:   τ_0_0.[P1:A1] => { concrete_type: [concrete: String] }
-// CHECK-NEXT: }
+// CHECK-RULE-LABEL: Requirement machine for <τ_0_0 where τ_0_0 : P1, τ_0_0 : P2>
+// CHECK-RULE-NEXT: Rewrite system: {
+// CHECK-RULE:      - [P1:X].[layout: _NativeClass] => [P1:X]
+// CHECK-RULE:      - [P2:X].[layout: _NativeClass] => [P2:X]
+// CHECK-RULE:      - τ_0_0.[P2:X] => τ_0_0.[P1:X]
+// CHECK-RULE:      - τ_0_0.[P1:X].[superclass: Generic<Int, τ_0_0.[P1:A1], τ_0_0.[P1:B1]>] => τ_0_0.[P1:X]
+// CHECK-RULE:      - τ_0_0.[P1:X].[superclass: Generic<Int, String, τ_0_0.[P1:B1]>] => τ_0_0.[P1:X]
+// CHECK-RULE:      - τ_0_0.[P2:A2].[concrete: Int] => τ_0_0.[P2:A2]
+// CHECK-RULE:      - τ_0_0.[P2:B2] => τ_0_0.[P1:B1]
+// CHECK-RULE:      - τ_0_0.[P1:A1].[concrete: String] => τ_0_0.[P1:A1]
+// CHECK-RULE:      - τ_0_0.B2 => τ_0_0.[P1:B1]
+// CHECK-RULE:      }
+// CHECK-RULE: Property map: {
+// CHECK-RULE-NEXT:   [P1] => { conforms_to: [P1] }
+// CHECK-RULE-NEXT:   [P1:X] => { layout: _NativeClass superclass: [superclass: Derived<[P1:A1], [P1:B1]>] }
+// CHECK-RULE-NEXT:   [P2] => { conforms_to: [P2] }
+// CHECK-RULE-NEXT:   [P2:X] => { layout: _NativeClass superclass: [superclass: Generic<[P2:A2], String, [P2:B2]>] }
+// CHECK-RULE-NEXT:   τ_0_0 => { conforms_to: [P1 P2] }
+// CHECK-RULE-NEXT:   τ_0_0.[P1:X] => { layout: _NativeClass superclass: [superclass: Derived<τ_0_0.[P1:A1], τ_0_0.[P1:B1]>] }
+// CHECK-RULE-NEXT:   τ_0_0.[P2:A2] => { concrete_type: [concrete: Int] }
+// CHECK-RULE-NEXT:   τ_0_0.[P1:A1] => { concrete_type: [concrete: String] }
+// CHECK-RULE-NEXT: }

test/attr/accessibility.swift

@@ -207,31 +207,3 @@ extension GenericStruct where Param: InternalProto {
   public func foo() {} // expected-error {{cannot declare a public instance method in an extension with internal requirements}} {{3-9=internal}}
 }
 
-
-public class OuterClass {
-  class InnerClass {}
-}
-
-public protocol PublicProto2 {
-  associatedtype T
-  associatedtype U
-}
-
-// FIXME: With the current design, the below should not diagnose.
-//
-// However, it does, because we look at the bound decl in the
-// TypeRepr first, and it happens to already be set.
-//
-// FIXME: Once we no longer do that, come up with another strategy
-// to make the above diagnose.
-
-extension PublicProto2 where Self.T : OuterClass, Self.U == Self.T.InnerClass {
-  public func cannotBePublic() {}
-  // expected-error@-1 {{cannot declare a public instance method in an extension with internal requirements}}
-}
-
-public extension OuterClass {
-  open convenience init(x: ()) { self.init() }
-  // expected-warning@-1 {{'open' modifier conflicts with extension's default access of 'public'}}
-  // expected-error@-2 {{only classes and overridable class members can be declared 'open'; use 'public'}}
-}

test/attr/accessibility_where_clause.swift

@@ -0,0 +1,32 @@
+// RUN: %target-typecheck-verify-swift -requirement-machine-inferred-signatures=off
+
+public class OuterClass {
+  class InnerClass {}
+}
+
+public protocol PublicProto2 {
+  associatedtype T
+  associatedtype U
+}
+
+// FIXME: With the current design, the below should not diagnose.
+//
+// However, it does, because we look at the bound decl in the
+// TypeRepr first, and it happens to already be set.
+//
+// FIXME: Once we no longer do that, come up with another strategy
+// to make the above diagnose.
+
+// FIXME: Get this working with the Requirement Machine, or decide that it should
+// be unsupported: rdar://90469477
+
+extension PublicProto2 where Self.T : OuterClass, Self.U == Self.T.InnerClass {
+  public func cannotBePublic() {}
+  // expected-error@-1 {{cannot declare a public instance method in an extension with internal requirements}}
+}
+
+public extension OuterClass {
+  open convenience init(x: ()) { self.init() }
+  // expected-warning@-1 {{'open' modifier conflicts with extension's default access of 'public'}}
+  // expected-error@-2 {{only classes and overridable class members can be declared 'open'; use 'public'}}
+}