Merge pull request #60523 from slavapestov/fix-opened-generic-environment-identity

Serialization: Preserve identity of opened generic environments

Author: slavapestov

include/swift/AST/GenericEnvironment.h

@@ -54,6 +54,7 @@ class QueryInterfaceTypeSubstitutions {
 /// Extra data in a generic environment for an opened existential.
 struct OpenedGenericEnvironmentData {
   Type existential;
+  GenericSignature parentSig;
   UUID uuid;
 };
 
@@ -73,7 +74,7 @@ class alignas(1 << DeclAlignInBits) GenericEnvironment final
   enum class Kind {
     /// A normal generic environment, determined only by its generic
     /// signature.
-    Normal,
+    Primary,
     /// A generic environment describing an opened existential archetype.
     OpenedExistential,
     /// A generic environment describing an opaque type archetype.
@@ -84,7 +85,7 @@ class alignas(1 << DeclAlignInBits) GenericEnvironment final
 
 private:
   mutable llvm::PointerIntPair<GenericSignature, 2, Kind> SignatureAndKind{
-      GenericSignature(), Kind::Normal};
+      GenericSignature(), Kind::Primary};
   NestedTypeStorage *nestedTypeStorage = nullptr;
 
   friend TrailingObjects;
@@ -110,7 +111,8 @@ class alignas(1 << DeclAlignInBits) GenericEnvironment final
 
   explicit GenericEnvironment(GenericSignature signature);
   explicit GenericEnvironment(
-      GenericSignature signature, Type existential, UUID uuid);
+      GenericSignature signature,
+      Type existential, GenericSignature parentSig, UUID uuid);
   explicit GenericEnvironment(
       GenericSignature signature, OpaqueTypeDecl *opaque, SubstitutionMap subs);
 
@@ -143,6 +145,9 @@ class alignas(1 << DeclAlignInBits) GenericEnvironment final
   /// Retrieve the UUID for an opened existential environment.
   UUID getOpenedExistentialUUID() const;
 
+  /// Retrieve the parent signature for an opened existential environment.
+  GenericSignature getOpenedExistentialParentSignature() const;
+
   /// Retrieve the opaque type declaration for a generic environment describing
   /// opaque types.
   OpaqueTypeDecl *getOpaqueTypeDecl() const;
@@ -151,36 +156,17 @@ class alignas(1 << DeclAlignInBits) GenericEnvironment final
   /// create a generic environment.
   SubstitutionMap getOpaqueSubstitutions() const;
 
-  /// Create a new, "incomplete" generic environment that will be populated
-  /// by calls to \c addMapping().
-  static
-  GenericEnvironment *getIncomplete(GenericSignature signature);
+  /// Create a new, primary generic environment.
+  static GenericEnvironment *forPrimary(GenericSignature signature);
 
   /// Create a new generic environment for an opened existential.
   ///
-  /// This function uses the provided parent signature to construct a new
-  /// signature suitable for use with an opened archetype. If you have an
-  /// existing generic signature from e.g. deserialization use
-  /// \c GenericEnvironment::forOpenedArchetypeSignature instead.
-  ///
   /// \param existential The subject existential type
   /// \param parentSig The signature of the context where this existential type is being opened
   /// \param uuid The unique identifier for this opened existential
   static GenericEnvironment *
   forOpenedExistential(Type existential, GenericSignature parentSig, UUID uuid);
 
-  /// Create a new generic environment for an opened existential.
-  ///
-  /// It is unlikely you want to use this function.
-  /// Call \c GenericEnvironment::forOpenedExistential instead.
-  ///
-  /// \param existential The subject existential type
-  /// \param signature The signature of the opened archetype
-  /// \param uuid The unique identifier for this opened existential
-  static GenericEnvironment *
-  forOpenedArchetypeSignature(Type existential,
-                              GenericSignature signature, UUID uuid);
-
   /// Create a new generic environment for an opaque type with the given set of
   /// outer substitutions.
   static GenericEnvironment *forOpaqueType(

include/swift/SIL/SILCloner.h

@@ -254,13 +254,10 @@ class SILCloner : protected SILInstructionVisitor<ImplClass> {
     assert(archetypeTy->isRoot());
 
     auto sig = Builder.getFunction().getGenericSignature();
-    auto existentialTy = archetypeTy->getExistentialType()->getCanonicalType();
-    auto env = GenericEnvironment::forOpenedExistential(
-        getOpASTType(existentialTy), sig, UUID::fromTime());
-    auto interfaceTy = OpenedArchetypeType::getSelfInterfaceTypeFromContext(sig, existentialTy->getASTContext());
-    auto replacementTy =
-        env->mapTypeIntoContext(interfaceTy)
-            ->template castTo<OpenedArchetypeType>();
+    auto origExistentialTy = archetypeTy->getExistentialType()
+        ->getCanonicalType();
+    auto substExistentialTy = getOpASTType(origExistentialTy);
+    auto replacementTy = OpenedArchetypeType::get(substExistentialTy, sig);
     registerOpenedExistentialRemapping(archetypeTy, replacementTy);
   }
 

lib/AST/ASTContext.cpp

@@ -4436,42 +4436,13 @@ CanOpenedArchetypeType OpenedArchetypeType::get(CanType existential,
                                                 Type interfaceType,
                                                 GenericSignature parentSig,
                                                 Optional<UUID> knownID) {
-  assert(existential->isExistentialType());
   assert(!interfaceType->hasArchetype() && "must be interface type");
-  // FIXME: Opened archetypes can't be transformed because the
-  // the identity of the archetype has to be preserved. This
-  // means that simplifying an opened archetype in the constraint
-  // system to replace type variables with fixed types is not
-  // yet supported. For now, assert that an opened archetype never
-  // contains type variables to catch cases where type variables
-  // would be applied to the type-checked AST.
-  assert(!existential->hasTypeVariable() &&
-         "opened existentials containing type variables cannot be simplified");
 
-  auto &ctx = existential->getASTContext();
-  auto &openedExistentialEnvironments =
-      ctx.getImpl().OpenedExistentialEnvironments;
-  // If we know the ID already...
-  if (knownID) {
-    // ... and we already have an archetype for that ID, return it.
-    auto found = openedExistentialEnvironments.find(*knownID);
-    
-    if (found != openedExistentialEnvironments.end()) {
-      assert(found->second->getOpenedExistentialType()->isEqual(existential) &&
-             "Retrieved the wrong generic environment?");
-      auto result = found->second->mapTypeIntoContext(interfaceType)
-          ->castTo<OpenedArchetypeType>();
-      return CanOpenedArchetypeType(result);
-    }
-  } else {
-    // Create a new ID.
+  if (!knownID)
     knownID = UUID::fromTime();
-  }
 
-  /// Create a generic environment for this opened archetype.
-  auto genericEnv =
+  auto *genericEnv =
       GenericEnvironment::forOpenedExistential(existential, parentSig, *knownID);
-  openedExistentialEnvironments[*knownID] = genericEnv;
 
   // Map the interface type into that environment.
   auto result = genericEnv->mapTypeIntoContext(interfaceType)
@@ -4637,8 +4608,7 @@ GenericSignature::get(TypeArrayView<GenericTypeParamType> params,
   return newSig;
 }
 
-GenericEnvironment *GenericEnvironment::getIncomplete(
-                                             GenericSignature signature) {
+GenericEnvironment *GenericEnvironment::forPrimary(GenericSignature signature) {
   auto &ctx = signature->getASTContext();
 
   // Allocate and construct the new environment.
@@ -4654,21 +4624,46 @@ GenericEnvironment *GenericEnvironment::getIncomplete(
 GenericEnvironment *
 GenericEnvironment::forOpenedExistential(
     Type existential, GenericSignature parentSig, UUID uuid) {
+  assert(existential->isExistentialType());
+  // FIXME: Opened archetypes can't be transformed because the
+  // the identity of the archetype has to be preserved. This
+  // means that simplifying an opened archetype in the constraint
+  // system to replace type variables with fixed types is not
+  // yet supported. For now, assert that an opened archetype never
+  // contains type variables to catch cases where type variables
+  // would be applied to the type-checked AST.
+  assert(!existential->hasTypeVariable() &&
+         "opened existentials containing type variables cannot be simplified");
+
   auto &ctx = existential->getASTContext();
+
+  auto &openedExistentialEnvironments =
+      ctx.getImpl().OpenedExistentialEnvironments;
+  auto found = openedExistentialEnvironments.find(uuid);
+
+  if (found != openedExistentialEnvironments.end()) {
+    auto *existingEnv = found->second;
+    assert(existingEnv->getOpenedExistentialType()->isEqual(existential));
+    assert(existingEnv->getOpenedExistentialParentSignature().getPointer() == parentSig.getPointer());
+    assert(existingEnv->getOpenedExistentialUUID() == uuid);
+
+    return existingEnv;
+  }
+
   auto signature = ctx.getOpenedExistentialSignature(existential, parentSig);
-  return GenericEnvironment::forOpenedArchetypeSignature(existential, signature, uuid);
-}
 
-GenericEnvironment *GenericEnvironment::forOpenedArchetypeSignature(
-    Type existential, GenericSignature signature, UUID uuid) {
   // Allocate and construct the new environment.
-  auto &ctx = existential->getASTContext();
   unsigned numGenericParams = signature.getGenericParams().size();
   size_t bytes = totalSizeToAlloc<OpaqueTypeDecl *, SubstitutionMap,
                                   OpenedGenericEnvironmentData, Type>(
       0, 0, 1, numGenericParams);
   void *mem = ctx.Allocate(bytes, alignof(GenericEnvironment));
-  return new (mem) GenericEnvironment(signature, existential, uuid);
+  auto *genericEnv =
+      new (mem) GenericEnvironment(signature, existential, parentSig, uuid);
+
+  openedExistentialEnvironments[uuid] = genericEnv;
+
+  return genericEnv;
 }
 
 /// Create a new generic environment for an opaque type with the given set of

lib/AST/GenericEnvironment.cpp

@@ -26,7 +26,7 @@ using namespace swift;
 size_t GenericEnvironment::numTrailingObjects(
     OverloadToken<OpaqueTypeDecl *>) const {
   switch (getKind()) {
-  case Kind::Normal:
+  case Kind::Primary:
   case Kind::OpenedExistential:
     return 0;
 
@@ -38,7 +38,7 @@ size_t GenericEnvironment::numTrailingObjects(
 size_t GenericEnvironment::numTrailingObjects(
     OverloadToken<SubstitutionMap>) const {
   switch (getKind()) {
-  case Kind::Normal:
+  case Kind::Primary:
   case Kind::OpenedExistential:
     return 0;
 
@@ -50,7 +50,7 @@ size_t GenericEnvironment::numTrailingObjects(
 size_t GenericEnvironment::numTrailingObjects(
     OverloadToken<OpenedGenericEnvironmentData>) const {
   switch (getKind()) {
-  case Kind::Normal:
+  case Kind::Primary:
   case Kind::Opaque:
     return 0;
 
@@ -104,20 +104,27 @@ UUID GenericEnvironment::getOpenedExistentialUUID() const {
   return getTrailingObjects<OpenedGenericEnvironmentData>()->uuid;
 }
 
+GenericSignature
+GenericEnvironment::getOpenedExistentialParentSignature() const {
+  assert(getKind() == Kind::OpenedExistential);
+  return getTrailingObjects<OpenedGenericEnvironmentData>()->parentSig;
+}
+
 GenericEnvironment::GenericEnvironment(GenericSignature signature)
-  : SignatureAndKind(signature, Kind::Normal)
+  : SignatureAndKind(signature, Kind::Primary)
 {
   // Clear out the memory that holds the context types.
   std::uninitialized_fill(getContextTypes().begin(), getContextTypes().end(),
                           Type());
 }
 
 GenericEnvironment::GenericEnvironment(
-    GenericSignature signature, Type existential, UUID uuid)
+    GenericSignature signature,
+    Type existential, GenericSignature parentSig, UUID uuid)
   : SignatureAndKind(signature, Kind::OpenedExistential)
 {
   new (getTrailingObjects<OpenedGenericEnvironmentData>())
-    OpenedGenericEnvironmentData{ existential, uuid };
+    OpenedGenericEnvironmentData{ existential, parentSig, uuid };
 
   // Clear out the memory that holds the context types.
   std::uninitialized_fill(getContextTypes().begin(), getContextTypes().end(),
@@ -209,7 +216,7 @@ struct SubstituteOuterFromSubstitutionMap {
 
 Type GenericEnvironment::maybeApplyOpaqueTypeSubstitutions(Type type) const {
   switch (getKind()) {
-  case Kind::Normal:
+  case Kind::Primary:
   case Kind::OpenedExistential:
     return type;
 
@@ -273,15 +280,6 @@ auto GenericEnvironment::getOrCreateNestedTypeStorage() -> NestedTypeStorage & {
   return *nestedTypeStorage;
 }
 
-static Type stripBoundDependentMemberTypes(Type t) {
-  if (auto *depMemTy = t->getAs<DependentMemberType>()) {
-    return DependentMemberType::get(
-      stripBoundDependentMemberTypes(depMemTy->getBase()),
-      depMemTy->getName());
-  }
-
-  return t;
-}
 Type
 GenericEnvironment::getOrCreateArchetypeFromInterfaceType(Type depType) {
   auto genericSig = getGenericSignature();
@@ -292,7 +290,7 @@ GenericEnvironment::getOrCreateArchetypeFromInterfaceType(Type depType) {
   /// Substitute a type for the purpose of requirements.
   auto substForRequirements = [&](Type type) {
     switch (getKind()) {
-    case Kind::Normal:
+    case Kind::Primary:
     case Kind::OpenedExistential:
       if (type->hasTypeParameter()) {
         return mapTypeIntoContext(type, conformanceLookupFn);
@@ -315,12 +313,11 @@ GenericEnvironment::getOrCreateArchetypeFromInterfaceType(Type depType) {
   // First, write an ErrorType to the location where this type is cached,
   // to catch re-entrant lookups that might arise from an invalid generic
   // signature (eg, <X where X == Array<X>>).
-  CanDependentMemberType nestedDependentMemberType;
+  CanDependentMemberType nestedType;
   GenericTypeParamType *genericParam = nullptr;
   if (auto depMemTy = requirements.anchor->getAs<DependentMemberType>()) {
-    nestedDependentMemberType = cast<DependentMemberType>(
-        stripBoundDependentMemberTypes(depMemTy)->getCanonicalType());
-    auto &entry = getOrCreateNestedTypeStorage()[nestedDependentMemberType];
+    nestedType = cast<DependentMemberType>(depMemTy->getCanonicalType());
+    auto &entry = getOrCreateNestedTypeStorage()[nestedType];
     if (entry)
       return entry;
 
@@ -342,13 +339,15 @@ GenericEnvironment::getOrCreateArchetypeFromInterfaceType(Type depType) {
 
   Type result;
 
-  if (requirements.anchor->getRootGenericParam()->isTypeSequence()) {
+  auto rootGP = requirements.anchor->getRootGenericParam();
+  if (rootGP->isTypeSequence()) {
+    assert(getKind() == Kind::Primary);
     result = SequenceArchetypeType::get(ctx, this, requirements.anchor,
                                         requirements.protos, superclass,
                                         requirements.layout);
   } else {
     switch (getKind()) {
-    case Kind::Normal:
+    case Kind::Primary:
       result = PrimaryArchetypeType::getNew(ctx, this, requirements.anchor,
                                             requirements.protos, superclass,
                                             requirements.layout);
@@ -384,7 +383,6 @@ GenericEnvironment::getOrCreateArchetypeFromInterfaceType(Type depType) {
       // If the anchor type isn't rooted in a generic parameter that
       // represents an opaque declaration, then apply the outer substitutions.
       // It would be incorrect to build an opaque type archetype here.
-      auto rootGP = requirements.anchor->getRootGenericParam();
       unsigned opaqueDepth =
           getOpaqueTypeDecl()->getOpaqueGenericParams().front()->getDepth();
       if (rootGP->getDepth() < opaqueDepth) {
@@ -403,7 +401,7 @@ GenericEnvironment::getOrCreateArchetypeFromInterfaceType(Type depType) {
   if (genericParam)
     addMapping(genericParam, result);
   else
-    getOrCreateNestedTypeStorage()[nestedDependentMemberType] = result;
+    getOrCreateNestedTypeStorage()[nestedType] = result;
 
   return result;
 }

lib/AST/GenericSignature.cpp

@@ -267,7 +267,7 @@ GenericEnvironment *GenericSignature::getGenericEnvironment() const {
 GenericEnvironment *GenericSignatureImpl::getGenericEnvironment() const {
   if (GenericEnv == nullptr) {
     const auto impl = const_cast<GenericSignatureImpl *>(this);
-    impl->GenericEnv = GenericEnvironment::getIncomplete(this);
+    impl->GenericEnv = GenericEnvironment::forPrimary(this);
   }
 
   return GenericEnv;

lib/Serialization/DeclTypeRecordNodes.def

@@ -167,7 +167,7 @@ OTHER(TOP_LEVEL_CODE_DECL_CONTEXT, 112)
 OTHER(GENERIC_PARAM_LIST, 120)
 OTHER(GENERIC_SIGNATURE, 121)
 OTHER(REQUIREMENT_SIGNATURE, 122)
-// 123 is unused; was LAYOUT_REQUIREMENT
+OTHER(GENERIC_ENVIRONMENT, 123)
 OTHER(BUILTIN_PROTOCOL_CONFORMANCE, 124)
 OTHER(SIL_GENERIC_SIGNATURE, 125)
 OTHER(SUBSTITUTION_MAP, 126)