Merge pull request #36940 from slavapestov/opaque-type-substitution-screwup

AST: Fix issue where SIL type lowering did not commute with subst()

Author: slavapestov

lib/AST/Type.cpp

@@ -3181,15 +3181,12 @@ operator()(SubstitutableType *maybeOpaqueType) const {
   // archetype in question. This will map the inner generic signature of the
   // opaque type to its outer signature.
   auto partialSubstTy = archetype->getInterfaceType().subst(*subs);
-  // Then apply the substitutions from the root opaque archetype, to specialize
-  // for its type arguments.
-  auto substTy = partialSubstTy.subst(opaqueRoot->getSubstitutions());
 
   // Check that we are allowed to substitute the underlying type into the
   // context.
   auto inContext = this->inContext;
   auto isContextWholeModule = this->isContextWholeModule;
-  if (substTy.findIf(
+  if (partialSubstTy.findIf(
           [inContext, substitutionKind, isContextWholeModule](Type t) -> bool {
             if (!canSubstituteTypeInto(t, inContext, substitutionKind,
                                        isContextWholeModule))
@@ -3198,6 +3195,12 @@ operator()(SubstitutableType *maybeOpaqueType) const {
           }))
     return maybeOpaqueType;
 
+  // Then apply the substitutions from the root opaque archetype, to specialize
+  // for its type arguments. We perform this substitution after checking for
+  // visibility, since we do not want the result of the visibility check to
+  // depend on the substitutions previously applied.
+  auto substTy = partialSubstTy.subst(opaqueRoot->getSubstitutions());
+
   // If the type changed, but still contains opaque types, recur.
   if (!substTy->isEqual(maybeOpaqueType) && substTy->hasOpaqueArchetype()) {
     return ::substOpaqueTypesWithUnderlyingTypes(
@@ -3264,18 +3267,12 @@ operator()(CanType maybeOpaqueType, Type replacementType,
   // archetype in question. This will map the inner generic signature of the
   // opaque type to its outer signature.
   auto partialSubstTy = archetype->getInterfaceType().subst(*subs);
-  auto partialSubstRef =
-      abstractRef.subst(archetype->getInterfaceType(), *subs);
-
-  // Then apply the substitutions from the root opaque archetype, to specialize
-  // for its type arguments.
-  auto substTy = partialSubstTy.subst(opaqueRoot->getSubstitutions());
 
   // Check that we are allowed to substitute the underlying type into the
   // context.
   auto inContext = this->inContext;
   auto isContextWholeModule = this->isContextWholeModule;
-  if (substTy.findIf(
+  if (partialSubstTy.findIf(
           [inContext, substitutionKind, isContextWholeModule](Type t) -> bool {
             if (!canSubstituteTypeInto(t, inContext, substitutionKind,
                                        isContextWholeModule))
@@ -3284,6 +3281,14 @@ operator()(CanType maybeOpaqueType, Type replacementType,
           }))
     return abstractRef;
 
+  // Then apply the substitutions from the root opaque archetype, to specialize
+  // for its type arguments. We perform this substitution after checking for
+  // visibility, since we do not want the result of the visibility check to
+  // depend on the substitutions previously applied.
+  auto substTy = partialSubstTy.subst(opaqueRoot->getSubstitutions());
+
+  auto partialSubstRef =
+      abstractRef.subst(archetype->getInterfaceType(), *subs);
   auto substRef =
       partialSubstRef.subst(partialSubstTy, opaqueRoot->getSubstitutions());
 

test/SILGen/Inputs/opaque_result_type_private_substitution_other.swift

@@ -0,0 +1,15 @@
+public protocol P {
+  associatedtype A
+  var v: A { get }
+}
+
+public extension P {
+  func foo() -> some P {
+    return self
+  }
+}
+
+public struct S: P {
+  public init() {}
+  public var v: some P { return self }
+}

test/SILGen/opaque_result_type_private_substitution.swift

@@ -0,0 +1,23 @@
+// RUN: %empty-directory(%t)
+
+// RUN: %target-swift-frontend -disable-availability-checking -emit-module %S/Inputs/opaque_result_type_private_substitution_other.swift -emit-module-path %t/opaque_result_type_private_substitution_other.swiftmodule
+// RUN: %target-swift-frontend -disable-availability-checking -emit-silgen %s -I %t -DMODULE
+
+// RUN: %target-swift-frontend -disable-availability-checking -emit-module %S/Inputs/opaque_result_type_private_substitution_other.swift -emit-module-path %t/opaque_result_type_private_substitution_other.swiftmodule -enable-library-evolution
+// RUN: %target-swift-frontend -disable-availability-checking -emit-silgen %s -I %t -DMODULE
+
+// RUN: %target-swift-frontend -disable-availability-checking -emit-silgen %s %S/Inputs/opaque_result_type_private_substitution_other.swift
+// RUN: %target-swift-frontend -disable-availability-checking -emit-silgen -primary-file %s %S/Inputs/opaque_result_type_private_substitution_other.swift
+// RUN: %target-swift-frontend -disable-availability-checking -emit-silgen -primary-file %s -primary-file %S/Inputs/opaque_result_type_private_substitution_other.swift
+
+#if MODULE
+import opaque_result_type_private_substitution_other
+#endif
+
+struct S1: P {
+  var v: some P { S2().foo() }
+}
+
+private struct S2: P {
+  var v: some P { S() }
+}