Merge pull request swiftlang#24875 from jckarter/opaque-type-subst-assertion

AST: Cut a different corner when building GenericEnvironments for OpaqueTypeArchetypeTypes.

Author: jckarter

lib/AST/ASTContext.cpp

@@ -3440,50 +3440,13 @@ DependentMemberType *DependentMemberType::get(Type base,
 
 OpaqueTypeArchetypeType *
 OpaqueTypeArchetypeType::get(OpaqueTypeDecl *Decl,
-                             SubstitutionMap Substitutions) {
+                             SubstitutionMap Substitutions)
+{
   // TODO: We could attempt to preserve type sugar in the substitution map.
+  // Currently archetypes are assumed to be always canonical in many places,
+  // though, so doing so would require fixing those places.
   Substitutions = Substitutions.getCanonical();
-  
-  // TODO: Eventually an opaque archetype ought to be arbitrarily substitutable
-  // into any generic environment. However, there isn't currently a good way to
-  // do this with GenericSignatureBuilder; in a situation like this:
-  //
-  // __opaque_type Foo<t_0_0: P>: Q // internal signature <t_0_0: P, t_1_0: Q>
-  //
-  // func bar<t_0_0, t_0_1, t_0_2: P>() -> Foo<t_0_2>
-  //
-  // we'd want to feed the GSB constraints to form:
-  //
-  // <t_0_0: P, t_1_0: Q where t_0_0 == t_0_2>
-  //
-  // even though t_0_2 isn't *in* this generic signature; it represents a type
-  // bound elsewhere from some other generic context. If we knew the generic
-  // environment `t_0_2` came from, then maybe we could map it into that context,
-  // but currently we have no way to know that with certainty.
-  //
-  // For now, opaque types cannot propagate across decls; every declaration
-  // with an opaque type has a unique opaque decl. Therefore, the only
-  // expressions involving opaque types ought to be contextualized inside
-  // function bodies, and the only time we need an opaque interface type should
-  // be for the opaque decl itself and the originating decl with the opaque
-  // result type, in which case the interface type mapping is identity and
-  // this problem can be temporarily avoided.
-#ifndef NDEBUG
-  for (unsigned i : indices(Substitutions.getReplacementTypes())) {
-    auto replacement = Substitutions.getReplacementTypes()[i];
-    
-    if (!replacement->hasTypeParameter())
-      continue;
-    
-    auto replacementParam = replacement->getAs<GenericTypeParamType>();
-    if (!replacementParam)
-      llvm_unreachable("opaque types cannot currently be parameterized by non-identity type parameter mappings");
-    
-    assert(i == Decl->getGenericSignature()->getGenericParamOrdinal(replacementParam)
-           && "opaque types cannot currently be parameterized by non-identity type parameter mappings");
-  }
-#endif
-  
+
   llvm::FoldingSetNodeID id;
   Profile(id, Decl, Substitutions);
 
@@ -3517,7 +3480,34 @@ OpaqueTypeArchetypeType::get(OpaqueTypeDecl *Decl,
   // decl have all been same-type-bound to the arguments from our substitution
   // map.
   GenericSignatureBuilder builder(ctx);
+
   builder.addGenericSignature(Decl->getOpaqueInterfaceGenericSignature());
+  // TODO: The proper thing to do to build the environment in which the opaque
+  // type's archetype exists would be to take the generic signature of the
+  // decl, feed it into a GenericSignatureBuilder, then add same-type
+  // constraints into the builder to bind the outer generic parameters
+  // to their substituted types provided by \c Substitutions. However,
+  // this is problematic for interface types. In a situation like this:
+  //
+  // __opaque_type Foo<t_0_0: P>: Q // internal signature <t_0_0: P, t_1_0: Q>
+  //
+  // func bar<t_0_0, t_0_1, t_0_2: P>() -> Foo<t_0_2>
+  //
+  // we'd want to feed the GSB constraints to form:
+  //
+  // <t_0_0: P, t_1_0: Q where t_0_0 == t_0_2>
+  //
+  // even though t_0_2 isn't *in* the generic signature being built; it
+  // represents a type
+  // bound elsewhere from some other generic context. If we knew the generic
+  // environment `t_0_2` came from, then maybe we could map it into that context,
+  // but currently we have no way to know that with certainty.
+  //
+  // Because opaque types are currently limited so that they only have immediate
+  // protocol constraints, and therefore don't interact with the outer generic
+  // parameters at all, we can get away without adding these constraints for now.
+  // Adding where clauses would break this hack.
+#if DO_IT_CORRECTLY
   // Same-type-constrain the arguments in the outer signature to their
   // replacements in the substitution map.
   if (auto outerSig = Decl->getGenericSignature()) {
@@ -3529,7 +3519,24 @@ OpaqueTypeArchetypeType::get(OpaqueTypeDecl *Decl,
          [](Type, Type) { llvm_unreachable("error?"); });
     }
   }
-  
+#else
+  // Assert that there are no same type constraints on the underlying type.
+  // or its associated types.
+  //
+  // This should not be possible until we add where clause support.
+# ifndef NDEBUG
+  for (auto reqt :
+                Decl->getOpaqueInterfaceGenericSignature()->getRequirements()) {
+    auto reqtBase = reqt.getFirstType()->getRootGenericParam();
+    if (reqtBase->isEqual(Decl->getUnderlyingInterfaceType())) {
+      assert(reqt.getKind() != RequirementKind::SameType
+             && "supporting where clauses on opaque types requires correctly "
+                "setting up the generic environment for "
+                "OpaqueTypeArchetypeTypes; see comment above");
+    }
+  }
+# endif
+#endif
   auto signature = std::move(builder)
     .computeGenericSignature(SourceLoc());
 

test/type/opaque.swift

@@ -349,3 +349,19 @@ func closure() -> some P {
   }
   return 42
 }
+
+protocol HasAssocType {
+  associatedtype Assoc
+
+  func assoc() -> Assoc
+}
+
+struct GenericWithOpaqueAssoc<T>: HasAssocType {
+  func assoc() -> some Any { return 0 }
+}
+
+struct OtherGeneric<X, Y, Z> {
+  var x: GenericWithOpaqueAssoc<X>.Assoc
+  var y: GenericWithOpaqueAssoc<Y>.Assoc
+  var z: GenericWithOpaqueAssoc<Z>.Assoc
+}