Merge pull request swiftlang#28147 from slavapestov/devirt-outer-param-requirement

 AST: Fix combineSubstitutionMaps() for requirements placed on outer generic parameters

Author: slavapestov

lib/AST/GenericSignature.cpp

@@ -814,6 +814,7 @@ getBestRequirementSource(GenericSignatureBuilder &builder,
     }
   }
 
+  assert(bestSource && "All sources were self-recursive?");
   return bestSource;
 }
 

lib/AST/SubstitutionMap.cpp

@@ -555,12 +555,8 @@ SubstitutionMap::getOverrideSubstitutions(const ClassDecl *baseClass,
   SubstitutionMap origSubMap;
   if (derivedSubs)
     origSubMap = *derivedSubs;
-  else if (derivedSig) {
-    origSubMap = get(
-        derivedSig,
-        [](SubstitutableType *type) -> Type { return type; },
-        MakeAbstractConformanceForGenericType());
-  }
+  else if (derivedSig)
+    origSubMap = derivedSig->getIdentitySubstitutionMap();
 
   return combineSubstitutionMaps(baseSubMap, origSubMap,
                                  CombineSubstitutionMaps::AtDepth,
@@ -603,17 +599,50 @@ SubstitutionMap::combineSubstitutionMaps(SubstitutionMap firstSubMap,
   return get(
     genericSig,
     [&](SubstitutableType *type) {
-      auto replacement = replaceGenericParameter(type);
-      if (replacement)
+      if (auto replacement = replaceGenericParameter(type))
         return Type(replacement).subst(secondSubMap);
       return Type(type).subst(firstSubMap);
     },
-    [&](CanType type, Type substType, ProtocolDecl *conformedProtocol) {
-      auto replacement = type.transform(replaceGenericParameter);
-      if (replacement)
+    [&](CanType type, Type substType, ProtocolDecl *proto) {
+      if (auto replacement = type.transform(replaceGenericParameter))
         return secondSubMap.lookupConformance(replacement->getCanonicalType(),
-                                              conformedProtocol);
-      return firstSubMap.lookupConformance(type, conformedProtocol);
+                                              proto);
+      if (auto conformance = firstSubMap.lookupConformance(type, proto))
+        return conformance;
+
+      // We might not have enough information in the substitution maps alone.
+      //
+      // Eg,
+      //
+      // class Base<T1> {
+      //   func foo<U1>(_: U1) where T1 : P {}
+      // }
+      //
+      // class Derived<T2> : Base<Foo<T2>> {
+      //   override func foo<U2>(_: U2) where T2 : Q {}
+      // }
+      //
+      // Suppose we're devirtualizing a call to Base.foo() on a value whose
+      // type is known to be Derived<Bar>. We start with substitutions written
+      // in terms of Base.foo()'s generic signature:
+      //
+      // <T1, U1 where T1 : P>
+      // T1 := Foo<Bar>
+      // T1 : P := Foo<Bar> : P
+      //
+      // We want to build substitutions in terms of Derived.foo()'s
+      // generic signature:
+      //
+      // <T2, U2 where T2 : Q>
+      // T2 := Bar
+      // T2 : Q := Bar : Q
+      //
+      // The conformance Bar : Q is difficult to recover in the general case.
+      //
+      // Some combination of storing substitution maps in BoundGenericTypes
+      // as well as for method overrides would solve this, but for now, just
+      // punt to module lookup.
+      return proto->getParentModule()->lookupConformance(substType, proto);
     });
 }
 

test/SILOptimizer/devirt_outer_requirements.swift

@@ -0,0 +1,42 @@
+
+// RUN: %target-swift-frontend -emit-sil -O %s | %FileCheck %s
+
+public protocol P {}
+
+public class C1<U> {
+  // CHECK-LABEL: sil @$s25devirt_outer_requirements2C1C3fooyyqd__AA1PRzlF : $@convention(method) <U where U : P><V> (@in_guaranteed V, @guaranteed C1<U>) -> () {
+  // CHECK: function_ref @$s25devirt_outer_requirements2C1C3baryyqd__AA1PRzlF : $@convention(method) <τ_0_0 where τ_0_0 : P><τ_1_0> (@in_guaranteed τ_1_0, @guaranteed C1<τ_0_0>) -> ()
+  // CHECK: return
+  public func foo<V>(_ z: V) where U : P {
+    bar(z)
+  }
+
+  @_optimize(none)
+  public func bar<V>(_ z: V) where U : P {}
+}
+
+public class Base<T> {
+  public func foo<V>(_: V) where T : P {}
+}
+
+public protocol Q {}
+
+public struct Foo<T> {}
+extension Foo : P where T : Q {}
+
+public class Derived<T> : Base<Foo<T>> {
+  public override func foo<V>(_: V) where T : Q {}
+}
+
+@_transparent
+public func takesBase<T, V>(_ b: Base<T>, _ v: V) where T : P {
+  b.foo(v)
+}
+
+// CHECK-LABEL: sil @$s25devirt_outer_requirements12takesDerivedyyAA0E0CyxG_q_tAA1QRzr0_lF : $@convention(thin) <T, V where T : Q> (@guaranteed Derived<T>, @in_guaranteed V) -> () {
+public func takesDerived<T, V>(_ d: Derived<T>, _ v: V) where T : Q {
+  // CHECK: function_ref @$s25devirt_outer_requirements12takesDerivedyyAA0E0CyxG_q_tAA1QRzr0_lF : $@convention(thin) <τ_0_0, τ_0_1 where τ_0_0 : Q> (@guaranteed Derived<τ_0_0>, @in_guaranteed τ_0_1) -> ()
+  takesDerived(d, v)
+
+  // CHECK: return
+}

validation-test/compiler_crashers_2_fixed/sr8469.swift

@@ -0,0 +1,21 @@
+// RUN: %target-swift-frontend -emit-ir %s
+
+final class Baz {}
+
+final class Bar {
+    private let x: Baz
+    init(x: Baz) {
+        self.x = x
+    }
+}
+
+final class Foo {
+    private var bar: Bar?
+
+    private func navigate(with baz: Baz?) {
+        bar = nil
+        guard let baz = baz else { return }
+        let bar = Bar(x: baz)
+        self.bar = bar
+    }
+}