Merge pull request swiftlang#27666 from aschwaighofer/fix_replace_opaque_types_with_underlying

aschwaighofer · web-flow · commit da0099e83f81 · 2019-10-17T12:40:06.000-07:00
ReplaceOpaqueTypesWithUnderlyingTypes: We can't look through opaque archetypes if the underlying type contains a type not accessible from the current context
diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h
@@ -4876,17 +4876,33 @@ class OpaqueTypeArchetypeType final : public ArchetypeType,
 BEGIN_CAN_TYPE_WRAPPER(OpaqueTypeArchetypeType, ArchetypeType)
 END_CAN_TYPE_WRAPPER(OpaqueTypeArchetypeType, ArchetypeType)
 
+enum OpaqueSubstitutionKind {
+  // Don't substitute the opaque type for the underlying type.
+  DontSubstitute,
+  // Substitute without looking at the type and context.
+  // Can be done because the underlying type is from a minimally resilient
+  // function (therefore must not contain private or internal types).
+  AlwaysSubstitute,
+  // Substitute in the same module into a maximal resilient context.
+  // Can be done if the underlying type is accessible from the context we
+  // substitute into. Private types cannot be accesses from a different TU.
+  SubstituteSameModuleMaximalResilience,
+  // Substitute in a different module from the opaque definining decl. Can only
+  // be done if the underlying type is public.
+  SubstituteNonResilientModule
+};
+
 /// A function object that can be used as a \c TypeSubstitutionFn and
 /// \c LookupConformanceFn for \c Type::subst style APIs to map opaque
 /// archetypes with underlying types visible at a given resilience expansion
 /// to their underlying types.
 class ReplaceOpaqueTypesWithUnderlyingTypes {
 public:
-  ModuleDecl *contextModule;
+  DeclContext *inContext;
   ResilienceExpansion contextExpansion;
-  ReplaceOpaqueTypesWithUnderlyingTypes(ModuleDecl *contextModule,
+  ReplaceOpaqueTypesWithUnderlyingTypes(DeclContext *inContext,
                                         ResilienceExpansion contextExpansion)
-      : contextModule(contextModule), contextExpansion(contextExpansion) {}
+      : inContext(inContext), contextExpansion(contextExpansion) {}
 
   /// TypeSubstitutionFn
   Type operator()(SubstitutableType *maybeOpaqueType) const;
@@ -4896,11 +4912,12 @@ class ReplaceOpaqueTypesWithUnderlyingTypes {
                                               Type replacementType,
                                               ProtocolDecl *protocol) const;
 
-  bool shouldPerformSubstitution(OpaqueTypeDecl *opaque) const;
+  OpaqueSubstitutionKind
+  shouldPerformSubstitution(OpaqueTypeDecl *opaque) const;
 
-  static bool shouldPerformSubstitution(OpaqueTypeDecl *opaque,
-                                        ModuleDecl *contextModule,
-                                        ResilienceExpansion contextExpansion);
+  static OpaqueSubstitutionKind
+  shouldPerformSubstitution(OpaqueTypeDecl *opaque, ModuleDecl *contextModule,
+                            ResilienceExpansion contextExpansion);
 };
 
 /// An archetype that represents the dynamic type of an opened existential.
diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp
@@ -2601,31 +2601,33 @@ getArchetypeAndRootOpaqueArchetype(Type maybeOpaqueType) {
   return std::make_pair(archetype, opaqueRoot);
 }
 
-bool ReplaceOpaqueTypesWithUnderlyingTypes::shouldPerformSubstitution(
+OpaqueSubstitutionKind
+ReplaceOpaqueTypesWithUnderlyingTypes::shouldPerformSubstitution(
     OpaqueTypeDecl *opaque) const {
-  return shouldPerformSubstitution(opaque, contextModule, contextExpansion);
+  return shouldPerformSubstitution(opaque, inContext->getParentModule(),
+                                   contextExpansion);
 }
-
-bool ReplaceOpaqueTypesWithUnderlyingTypes::shouldPerformSubstitution(
+OpaqueSubstitutionKind
+ReplaceOpaqueTypesWithUnderlyingTypes::shouldPerformSubstitution(
     OpaqueTypeDecl *opaque, ModuleDecl *contextModule,
     ResilienceExpansion contextExpansion) {
   auto namingDecl = opaque->getNamingDecl();
   
   // Don't allow replacement if the naming decl is dynamically replaceable.
   if (namingDecl && namingDecl->isDynamic())
-    return false;
+    return OpaqueSubstitutionKind::DontSubstitute;
 
   // Allow replacement of opaque result types of inlineable function regardless
   // of resilience and in which context.
   if (auto *afd = dyn_cast<AbstractFunctionDecl>(namingDecl)) {
     if (afd->getResilienceExpansion() == ResilienceExpansion::Minimal) {
-      return true;
+      return OpaqueSubstitutionKind::AlwaysSubstitute;
     }
   } else if (auto *asd = dyn_cast<AbstractStorageDecl>(namingDecl)) {
     auto *getter = asd->getOpaqueAccessor(AccessorKind::Get);
     if (getter &&
         getter->getResilienceExpansion() == ResilienceExpansion::Minimal) {
-      return true;
+      return OpaqueSubstitutionKind::AlwaysSubstitute;
     }
   }
 
@@ -2635,20 +2637,49 @@ bool ReplaceOpaqueTypesWithUnderlyingTypes::shouldPerformSubstitution(
   auto module = namingDecl->getModuleContext();
   if (contextExpansion == ResilienceExpansion::Maximal &&
       module == contextModule)
-    return true;
+    return OpaqueSubstitutionKind::SubstituteSameModuleMaximalResilience;
 
   // Allow general replacement from non resilient modules. Otherwise, disallow.
-  return !module->isResilient();
+  if (module->isResilient())
+    return OpaqueSubstitutionKind::DontSubstitute;
+
+  return OpaqueSubstitutionKind::SubstituteNonResilientModule;
 }
 
 static Type
-substOpaqueTypesWithUnderlyingTypes(Type ty, ModuleDecl *contextModule,
+substOpaqueTypesWithUnderlyingTypes(Type ty, DeclContext *inContext,
                                     ResilienceExpansion contextExpansion) {
-  ReplaceOpaqueTypesWithUnderlyingTypes replacer(contextModule,
-                                                 contextExpansion);
+  ReplaceOpaqueTypesWithUnderlyingTypes replacer(inContext, contextExpansion);
   return ty.subst(replacer, replacer, SubstFlags::SubstituteOpaqueArchetypes);
 }
 
+bool canSubstituteTypeInto(Type ty, DeclContext *dc,
+                           OpaqueSubstitutionKind kind) {
+  auto nominal = ty->getAnyNominal();
+  if (!nominal)
+    return true;
+
+  switch (kind) {
+  case DontSubstitute:
+    return false;
+
+  case AlwaysSubstitute:
+    return true;
+
+  case SubstituteSameModuleMaximalResilience:
+    // In the same file any visibility is okay.
+    if (!dc->isModuleContext() &&
+        nominal->getDeclContext()->getParentSourceFile() ==
+        dc->getParentSourceFile())
+      return true;
+    return nominal->getEffectiveAccess() > AccessLevel::FilePrivate;
+
+  case SubstituteNonResilientModule:
+    // Can't access types that are not public from a different module.
+    return nominal->getEffectiveAccess() > AccessLevel::Internal;
+  }
+}
+
 Type ReplaceOpaqueTypesWithUnderlyingTypes::
 operator()(SubstitutableType *maybeOpaqueType) const {
   auto archetypeAndRoot = getArchetypeAndRootOpaqueArchetype(maybeOpaqueType);
@@ -2658,7 +2689,8 @@ operator()(SubstitutableType *maybeOpaqueType) const {
   auto archetype = archetypeAndRoot->first;
   auto opaqueRoot = archetypeAndRoot->second;
 
-  if (!shouldPerformSubstitution(opaqueRoot->getDecl())) {
+  auto substitutionKind = shouldPerformSubstitution(opaqueRoot->getDecl());
+  if (substitutionKind == OpaqueSubstitutionKind::DontSubstitute) {
     return maybeOpaqueType;
   }
 
@@ -2676,20 +2708,30 @@ operator()(SubstitutableType *maybeOpaqueType) const {
   // 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;
+  if (substTy.findIf([inContext, substitutionKind](Type t) -> bool {
+        if (!canSubstituteTypeInto(t, inContext, substitutionKind))
+          return true;
+        return false;
+      }))
+    return maybeOpaqueType;
+
   // If the type still contains opaque types, recur.
   if (substTy->hasOpaqueArchetype()) {
-    return substOpaqueTypesWithUnderlyingTypes(substTy, contextModule,
+    return substOpaqueTypesWithUnderlyingTypes(substTy, inContext,
                                                contextExpansion);
   }
+
   return substTy;
 }
 
 static ProtocolConformanceRef
 substOpaqueTypesWithUnderlyingTypes(ProtocolConformanceRef ref, Type origType,
-                                    ModuleDecl *contextModule,
+                                    DeclContext *inContext,
                                     ResilienceExpansion contextExpansion) {
-  ReplaceOpaqueTypesWithUnderlyingTypes replacer(contextModule,
-                                                 contextExpansion);
+  ReplaceOpaqueTypesWithUnderlyingTypes replacer(inContext, contextExpansion);
   return ref.subst(origType, replacer, replacer,
                    SubstFlags::SubstituteOpaqueArchetypes);
 }
@@ -2709,7 +2751,8 @@ operator()(CanType maybeOpaqueType, Type replacementType,
   auto archetype = archetypeAndRoot->first;
   auto opaqueRoot = archetypeAndRoot->second;
 
-  if (!shouldPerformSubstitution(opaqueRoot->getDecl())) {
+  auto substitutionKind = shouldPerformSubstitution(opaqueRoot->getDecl());
+  if (substitutionKind == OpaqueSubstitutionKind::DontSubstitute) {
     return abstractRef;
   }
 
@@ -2729,12 +2772,23 @@ operator()(CanType maybeOpaqueType, Type replacementType,
   // 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;
+  if (substTy.findIf([inContext, substitutionKind](Type t) -> bool {
+        if (!canSubstituteTypeInto(t, inContext, substitutionKind))
+          return true;
+        return false;
+      }))
+    return abstractRef;
+
   auto substRef =
       partialSubstRef.subst(partialSubstTy, opaqueRoot->getSubstitutions());
 
   // If the type still contains opaque types, recur.
   if (substTy->hasOpaqueArchetype()) {
-    return substOpaqueTypesWithUnderlyingTypes(substRef, substTy, contextModule,
+    return substOpaqueTypesWithUnderlyingTypes(substRef, substTy, inContext,
                                                contextExpansion);
   }
   return substRef;
diff --git a/lib/IRGen/MetadataRequest.cpp b/lib/IRGen/MetadataRequest.cpp
@@ -518,9 +518,10 @@ CanType IRGenModule::substOpaqueTypesWithUnderlyingTypes(CanType type) {
   if (type->hasOpaqueArchetype()) {
     ReplaceOpaqueTypesWithUnderlyingTypes replacer(getSwiftModule(),
                                                   ResilienceExpansion::Maximal);
-    type = type.subst(replacer, replacer,
-                      SubstFlags::SubstituteOpaqueArchetypes)
-      ->getCanonicalType();
+    auto underlyingTy =
+        type.subst(replacer, replacer, SubstFlags::SubstituteOpaqueArchetypes)
+            ->getCanonicalType();
+    return underlyingTy;
   }
 
   return type;
@@ -533,8 +534,10 @@ SILType IRGenModule::substOpaqueTypesWithUnderlyingTypes(
   if (type.getASTType()->hasOpaqueArchetype()) {
     ReplaceOpaqueTypesWithUnderlyingTypes replacer(getSwiftModule(),
                                                   ResilienceExpansion::Maximal);
-    type = type.subst(getSILModule(), replacer, replacer, genericSig,
-                      /*substitute opaque*/ true);
+    auto underlyingTy =
+        type.subst(getSILModule(), replacer, replacer, genericSig,
+                   /*substitute opaque*/ true);
+    return underlyingTy;
   }
 
   return type;
@@ -548,11 +551,12 @@ IRGenModule::substOpaqueTypesWithUnderlyingTypes(CanType type,
   if (type->hasOpaqueArchetype()) {
     ReplaceOpaqueTypesWithUnderlyingTypes replacer(getSwiftModule(),
                                                   ResilienceExpansion::Maximal);
-    conformance = conformance.subst(type, replacer, replacer,
-                                    SubstFlags::SubstituteOpaqueArchetypes);
-    type = type.subst(replacer, replacer,
-                      SubstFlags::SubstituteOpaqueArchetypes)
-      ->getCanonicalType();
+    auto substConformance = conformance.subst(
+        type, replacer, replacer, SubstFlags::SubstituteOpaqueArchetypes);
+    auto underlyingTy =
+        type.subst(replacer, replacer, SubstFlags::SubstituteOpaqueArchetypes)
+            ->getCanonicalType();
+    return std::make_pair(underlyingTy, substConformance);
   }
 
   return std::make_pair(type, conformance);
diff --git a/lib/SIL/SILFunctionType.cpp b/lib/SIL/SILFunctionType.cpp
@@ -2804,9 +2804,11 @@ static bool areABICompatibleParamsOrReturns(SILType a, SILType b,
     // Opaque types are compatible with their substitution.
     if (inFunction) {
       auto opaqueTypesSubsituted = aa;
+      auto *dc = inFunction->getDeclContext();
+      if (!dc)
+        dc = inFunction->getModule().getSwiftModule();
       ReplaceOpaqueTypesWithUnderlyingTypes replacer(
-          inFunction->getModule().getSwiftModule(),
-          inFunction->getResilienceExpansion());
+          dc, inFunction->getResilienceExpansion());
       if (aa.getASTType()->hasOpaqueArchetype())
         opaqueTypesSubsituted = aa.subst(inFunction->getModule(), replacer,
                                          replacer, CanGenericSignature(), true);
diff --git a/lib/SILOptimizer/Transforms/SpecializeOpaqueArchetypes.cpp b/lib/SILOptimizer/Transforms/SpecializeOpaqueArchetypes.cpp
@@ -33,15 +33,22 @@ using namespace swift;
 
 static Type substOpaqueTypesWithUnderlyingTypes(
     Type ty, SILFunction *context) {
+  auto *dc = context->getDeclContext();
+  if (!dc)
+    dc = context->getModule().getSwiftModule();
+
   ReplaceOpaqueTypesWithUnderlyingTypes replacer(
-      context->getModule().getSwiftModule(), context->getResilienceExpansion());
+      dc, context->getResilienceExpansion());
   return ty.subst(replacer, replacer, SubstFlags::SubstituteOpaqueArchetypes);
 }
 
 static SubstitutionMap
 substOpaqueTypesWithUnderlyingTypes(SubstitutionMap map, SILFunction *context) {
+  auto *dc = context->getDeclContext();
+  if (!dc)
+    dc = context->getModule().getSwiftModule();
   ReplaceOpaqueTypesWithUnderlyingTypes replacer(
-      context->getModule().getSwiftModule(), context->getResilienceExpansion());
+      dc, context->getResilienceExpansion());
   return map.subst(replacer, replacer, SubstFlags::SubstituteOpaqueArchetypes);
 }
 
@@ -324,11 +331,13 @@ class OpaqueSpecializerCloner
     SILType &Sty = TypeCache[Ty];
     if (Sty)
       return Sty;
+    auto *dc = Original.getDeclContext();
+    if (!dc)
+      dc = Original.getModule().getSwiftModule();
 
-   // Apply the opaque types substitution.
+    // Apply the opaque types substitution.
     ReplaceOpaqueTypesWithUnderlyingTypes replacer(
-        Original.getModule().getSwiftModule(),
-        Original.getResilienceExpansion());
+        dc, Original.getResilienceExpansion());
     Sty = Ty.subst(Original.getModule(), replacer, replacer,
                    CanGenericSignature(), true);
     return Sty;
@@ -342,10 +351,12 @@ class OpaqueSpecializerCloner
 
   ProtocolConformanceRef remapConformance(Type type,
                                           ProtocolConformanceRef conf) {
+    auto *dc = Original.getDeclContext();
+    if (!dc)
+      dc = Original.getModule().getSwiftModule();
     // Apply the opaque types substitution.
     ReplaceOpaqueTypesWithUnderlyingTypes replacer(
-        Original.getModule().getSwiftModule(),
-        Original.getResilienceExpansion());
+        dc, Original.getResilienceExpansion());
     return conf.subst(type, replacer, replacer,
                       SubstFlags::SubstituteOpaqueArchetypes);
   }
@@ -493,6 +504,9 @@ class OpaqueArchetypeSpecializer : public SILFunctionTransform {
 
       return ty.findIf([=](Type type) -> bool {
         if (auto opaqueTy = type->getAs<OpaqueTypeArchetypeType>()) {
+          auto *dc = context->getDeclContext();
+          if (!dc)
+            dc = context->getModule().getSwiftModule();
           auto opaque = opaqueTy->getDecl();
           return ReplaceOpaqueTypesWithUnderlyingTypes::
               shouldPerformSubstitution(opaque,
diff --git a/test/IRGen/Inputs/opaque_result_type_private_underlying_2.swift b/test/IRGen/Inputs/opaque_result_type_private_underlying_2.swift
diff --git a/test/IRGen/opaque_result_type_private_underlying.swift b/test/IRGen/opaque_result_type_private_underlying.swift
