Merge pull request #41868 from slavapestov/rqm-opaque-archetypes

slavapestov · web-flow · commit e6d1ef9f6d8f · 2022-03-17T23:06:24.000-04:00
RequirementMachine: Opaque archetype support (sort of)
diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h
@@ -549,6 +549,11 @@ namespace swift {
     /// if you have a testcase which requires this, please submit a bug report.
     bool EnableRequirementMachineLoopNormalization = false;
 
+    /// Enable experimental, more correct support for opaque result types as
+    /// concrete types. This will sometimes fail to produce a convergent
+    /// rewrite system.
+    bool EnableRequirementMachineOpaqueArchetypes = false;
+
     /// Enables dumping type witness systems from associated type inference.
     bool DumpTypeWitnessSystems = false;
 
diff --git a/include/swift/Option/FrontendOptions.td b/include/swift/Option/FrontendOptions.td
@@ -362,6 +362,9 @@ def disable_requirement_machine_concrete_contraction : Flag<["-"], "disable-requ
 def enable_requirement_machine_loop_normalization : Flag<["-"], "enable-requirement-machine-loop-normalization">,
   HelpText<"Enable stronger minimization algorithm, for debugging only">;
 
+def enable_requirement_machine_opaque_archetypes : Flag<["-"], "enable-requirement-machine-opaque-archetypes">,
+  HelpText<"Enable more correct opaque archetype support, which is off by default because it might fail to produce a convergent rewrite system">;
+
 def dump_type_witness_systems : Flag<["-"], "dump-type-witness-systems">,
   HelpText<"Enables dumping type witness systems from associated type inference">;
 
diff --git a/lib/AST/RequirementMachine/ConcreteContraction.cpp b/lib/AST/RequirementMachine/ConcreteContraction.cpp
@@ -235,19 +235,10 @@ Optional<Type> ConcreteContraction::substTypeParameter(
                     ->substBaseType(module, *substBaseType);
   }
 
-  auto *decl = (*substBaseType)->getAnyNominal();
-  if (decl == nullptr) {
-    if (Debug) {
-      llvm::dbgs() << "@@@ Not a nominal type: " << *substBaseType << "\n";
-    }
-
-    return None;
-  }
-
   // An unresolved DependentMemberType stores an identifier. Handle this
   // by performing a name lookup into the base type.
   SmallVector<TypeDecl *> concreteDecls;
-  lookupConcreteNestedType(decl, memberType->getName(), concreteDecls);
+  lookupConcreteNestedType(*substBaseType, memberType->getName(), concreteDecls);
 
   auto *typeDecl = findBestConcreteNestedType(concreteDecls);
   if (typeDecl == nullptr) {
@@ -261,8 +252,9 @@ Optional<Type> ConcreteContraction::substTypeParameter(
   }
 
   // Substitute the base type into the member type.
+  auto *dc = typeDecl->getDeclContext();
   auto subMap = (*substBaseType)->getContextSubstitutionMap(
-      decl->getParentModule(), typeDecl->getDeclContext());
+      dc->getParentModule(), dc);
   return typeDecl->getDeclaredInterfaceType().subst(subMap);
 }
 
diff --git a/lib/AST/RequirementMachine/ConcreteTypeWitness.cpp b/lib/AST/RequirementMachine/ConcreteTypeWitness.cpp
@@ -132,9 +132,7 @@ void PropertyMap::concretizeNestedTypesFromConcreteParent(
     //
     // This occurs when a pair of rules are inherited from the property map
     // entry for this key's suffix.
-    auto pair = std::make_pair(concreteRuleID, conformanceRuleID);
-    auto found = ConcreteConformances.find(pair);
-    if (found != ConcreteConformances.end())
+    if (!checkRulePairOnce(concreteRuleID, conformanceRuleID))
       continue;
 
     // FIXME: Either remove the ModuleDecl entirely from conformance lookup,
@@ -164,40 +162,44 @@ void PropertyMap::concretizeNestedTypesFromConcreteParent(
       continue;
     }
 
-    // FIXME: Maybe this can happen if the concrete type is an
-    // opaque result type?
-    assert(!conformance.isAbstract());
-
-    // Save this conformance for later.
-    auto *concrete = conformance.getConcrete();
-    auto inserted = ConcreteConformances.insert(
-        std::make_pair(pair, concrete));
-    assert(inserted.second);
-    (void) inserted;
-
     auto concreteConformanceSymbol = Symbol::forConcreteConformance(
         concreteType, substitutions, proto, Context);
 
     recordConcreteConformanceRule(concreteRuleID, conformanceRuleID,
                                   requirementKind, concreteConformanceSymbol);
 
+    // This is disabled by default because we fail to produce a convergent
+    // rewrite system if the opaque archetype has infinitely-recursive
+    // nested types. Fixing this requires a better representation for
+    // concrete conformances in the rewrite system.
+    if (conformance.isAbstract() &&
+        !Context.getASTContext().LangOpts.EnableRequirementMachineOpaqueArchetypes) {
+      if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) {
+        llvm::dbgs() << "^^ " << "Skipping abstract conformance of "
+                     << concreteType << " to " << proto->getName() << "\n";
+      }
+
+      continue;
+    }
+
     for (auto *assocType : proto->getAssociatedTypeMembers()) {
       concretizeTypeWitnessInConformance(key, requirementKind,
                                          concreteConformanceSymbol,
-                                         concrete, assocType);
+                                         conformance, assocType);
     }
 
     // We only infer conditional requirements in top-level generic signatures,
     // not in protocol requirement signatures.
-    if (key.getRootProtocol() == nullptr)
-      inferConditionalRequirements(concrete, substitutions);
+    if (conformance.isConcrete() &&
+        key.getRootProtocol() == nullptr)
+      inferConditionalRequirements(conformance.getConcrete(), substitutions);
   }
 }
 
 void PropertyMap::concretizeTypeWitnessInConformance(
     Term key, RequirementKind requirementKind,
     Symbol concreteConformanceSymbol,
-    ProtocolConformance *concrete,
+    ProtocolConformanceRef conformance,
     AssociatedTypeDecl *assocType) const {
   auto concreteType = concreteConformanceSymbol.getConcreteType();
   auto substitutions = concreteConformanceSymbol.getSubstitutions();
@@ -211,17 +213,35 @@ void PropertyMap::concretizeTypeWitnessInConformance(
                  << " on " << concreteType << "\n";
   }
 
-  auto t = concrete->getTypeWitness(assocType);
-  if (!t) {
-    if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) {
-      llvm::dbgs() << "^^ " << "Type witness for " << assocType->getName()
-                   << " of " << concreteType << " could not be inferred\n";
+  CanType typeWitness;
+  if (conformance.isConcrete()) {
+    auto t = conformance.getConcrete()->getTypeWitness(assocType);
+
+    if (!t) {
+      if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) {
+        llvm::dbgs() << "^^ " << "Type witness for " << assocType->getName()
+                     << " of " << concreteType << " could not be inferred\n";
+      }
+
+      t = ErrorType::get(concreteType);
     }
 
-    t = ErrorType::get(concreteType);
-  }
+    typeWitness = t->getCanonicalType();
+  } else if (conformance.isAbstract()) {
+    auto archetype = concreteType->getAs<OpaqueTypeArchetypeType>();
+    if (archetype == nullptr) {
+      llvm::errs() << "Should only have an abstract conformance with an "
+                   << "opaque archetype type\n";
+      llvm::errs() << "Symbol: " << concreteConformanceSymbol << "\n";
+      llvm::errs() << "Term: " << key << "\n";
+      dump(llvm::errs());
+      abort();
+    }
 
-  auto typeWitness = t->getCanonicalType();
+    typeWitness = archetype->getNestedType(assocType)->getCanonicalType();
+  } else if (conformance.isInvalid()) {
+    typeWitness = CanType(ErrorType::get(Context.getASTContext()));
+  }
 
   if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) {
     llvm::dbgs() << "^^ " << "Type witness for " << assocType->getName()
diff --git a/lib/AST/RequirementMachine/GenericSignatureQueries.cpp b/lib/AST/RequirementMachine/GenericSignatureQueries.cpp
@@ -17,6 +17,7 @@
 
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/Decl.h"
+#include "swift/AST/GenericEnvironment.h"
 #include "swift/AST/GenericSignature.h"
 #include "swift/AST/Module.h"
 #include <vector>
@@ -659,8 +660,7 @@ RequirementMachine::lookupNestedType(Type depType, Identifier name) const {
       typeToSearch = props->getSuperclassBound();
 
     if (typeToSearch)
-      if (auto *decl = typeToSearch->getAnyNominal())
-        lookupConcreteNestedType(decl, name, concreteDecls);
+      lookupConcreteNestedType(typeToSearch, name, concreteDecls);
   }
 
   if (bestAssocType) {
diff --git a/lib/AST/RequirementMachine/NameLookup.cpp b/lib/AST/RequirementMachine/NameLookup.cpp
@@ -12,13 +12,33 @@
 
 #include "NameLookup.h"
 #include "swift/AST/Decl.h"
+#include "swift/AST/GenericEnvironment.h"
 #include "swift/AST/Module.h"
+#include "swift/AST/Types.h"
 #include "llvm/ADT/SmallVector.h"
 #include <algorithm>
 
 using namespace swift;
 using namespace rewriting;
 
+void
+swift::rewriting::lookupConcreteNestedType(
+    Type baseType,
+    Identifier name,
+    SmallVectorImpl<TypeDecl *> &concreteDecls) {
+  if (auto *decl = baseType->getAnyNominal())
+    lookupConcreteNestedType(decl, name, concreteDecls);
+  else if (auto *archetype = baseType->getAs<OpaqueTypeArchetypeType>()) {
+    // If our concrete type is an opaque result archetype, look into its
+    // generic environment recursively.
+    auto *genericEnv = archetype->getGenericEnvironment();
+    auto genericSig = genericEnv->getGenericSignature();
+
+    concreteDecls.push_back(
+        genericSig->lookupNestedType(archetype->getInterfaceType(), name));
+  }
+}
+
 void
 swift::rewriting::lookupConcreteNestedType(
     NominalTypeDecl *decl,
diff --git a/lib/AST/RequirementMachine/NameLookup.h b/lib/AST/RequirementMachine/NameLookup.h
@@ -19,10 +19,16 @@ namespace swift {
 
 class Identifier;
 class NominalTypeDecl;
+class Type;
 class TypeDecl;
 
 namespace rewriting {
 
+void lookupConcreteNestedType(
+    Type baseType,
+    Identifier name,
+    llvm::SmallVectorImpl<TypeDecl *> &concreteDecls);
+
 void lookupConcreteNestedType(
     NominalTypeDecl *decl,
     Identifier name,
diff --git a/lib/AST/RequirementMachine/PropertyMap.h b/lib/AST/RequirementMachine/PropertyMap.h
@@ -174,12 +174,6 @@ class PropertyMap {
   // To avoid wasted work from re-introducing the same induced rules,
   // we track the rules we've seen already on previous builds.
 
-  /// Maps a pair of rules where the first is a conformance rule and the
-  /// second is a superclass or concrete type rule, to a concrete
-  /// conformance.
-  llvm::DenseMap<std::pair<unsigned, unsigned>, ProtocolConformance *>
-      ConcreteConformances;
-
   /// Superclass requirements always imply a layout requirement, and
   /// concrete type requirements where the type is a class imply a
   /// superclass requirement.
@@ -291,7 +285,7 @@ class PropertyMap {
   void concretizeTypeWitnessInConformance(
                    Term key, RequirementKind requirementKind,
                    Symbol concreteConformanceSymbol,
-                   ProtocolConformance *concrete,
+                   ProtocolConformanceRef conformance,
                    AssociatedTypeDecl *assocType) const;
 
   void inferConditionalRequirements(
diff --git a/lib/AST/RequirementMachine/RequirementLowering.cpp b/lib/AST/RequirementMachine/RequirementLowering.cpp
@@ -168,13 +168,13 @@ static void desugarConformanceRequirement(Type subjectType, Type constraintType,
       errors.push_back(RequirementError::forRedundantRequirement(
           {RequirementKind::Conformance, subjectType, constraintType}, loc));
 
-      assert(conformance.isConcrete());
-      auto *concrete = conformance.getConcrete();
-
-      // Introduce conditional requirements if the subject type is concrete.
-      for (auto req : concrete->getConditionalRequirements()) {
-        desugarRequirement(req, result, errors);
+      if (conformance.isConcrete()) {
+        // Introduce conditional requirements if the conformance is concrete.
+        for (auto req : conformance.getConcrete()->getConditionalRequirements()) {
+          desugarRequirement(req, result, errors);
+        }
       }
+
       return;
     }
 
diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp
@@ -1007,6 +1007,9 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
   if (Args.hasArg(OPT_enable_requirement_machine_loop_normalization))
     Opts.EnableRequirementMachineLoopNormalization = true;
 
+  if (Args.hasArg(OPT_enable_requirement_machine_opaque_archetypes))
+    Opts.EnableRequirementMachineOpaqueArchetypes = true;
+
   Opts.DumpTypeWitnessSystems = Args.hasArg(OPT_dump_type_witness_systems);
 
   return HadError || UnsupportedOS || UnsupportedArch;
diff --git a/test/Generics/opaque_archetype_concrete_requirement.swift b/test/Generics/opaque_archetype_concrete_requirement.swift
@@ -0,0 +1,81 @@
+// RUN: %target-swift-frontend -typecheck -verify %s -disable-availability-checking -debug-generic-signatures -requirement-machine-inferred-signatures=on -enable-requirement-machine-opaque-archetypes 2>&1 | %FileCheck %s
+
+protocol P1 {
+  associatedtype T : P2
+  associatedtype U
+}
+
+struct S_P1 : P1 {
+  typealias T = S_P2
+  typealias U = Int
+}
+
+protocol P2 {}
+
+struct S_P2 : P2 {}
+
+protocol P {
+  associatedtype T
+
+  var t: T { get }
+}
+
+struct DefinesOpaqueP1 : P {
+  var t: some P1 {
+    return S_P1()
+  }
+}
+
+struct ConcreteHasP<T : P1, TT : P2, TU> {}
+
+// CHECK-LABEL: ExtensionDecl line={{.*}} base=ConcreteHasP
+// CHECK-NEXT: Generic signature: <T, TT, TU where T == some P1, TT == (some P1).T, TU == (some P1).U>
+extension ConcreteHasP where T == DefinesOpaqueP1.T, TT == T.T, TU == T.U {
+  func checkSameType1(_ t: TT) -> DefinesOpaqueP1.T.T { return t }
+  func checkSameType2(_ u: TU) -> DefinesOpaqueP1.T.U { return u }
+
+  func checkSameType3(_ t: T.T) -> DefinesOpaqueP1.T.T { return t }
+  func checkSameType4(_ u: T.U) -> DefinesOpaqueP1.T.U { return u }
+}
+
+struct G<T> {}
+
+protocol HasP {
+  associatedtype T : P1
+  associatedtype U
+}
+
+// CHECK-LABEL: ExtensionDecl line={{.*}} base=HasP
+// CHECK-NEXT: Generic signature: <Self where Self : HasP, Self.[HasP]T == some P1, Self.[HasP]U == G<(some P1).T>>
+extension HasP where T == DefinesOpaqueP1.T, U == G<T.T> {
+  func checkSameType1(_ t: T.T) -> DefinesOpaqueP1.T.T { return t }
+  func checkSameType2(_ u: T.U) -> DefinesOpaqueP1.T.U { return u }
+}
+
+// FIXME: This does not work with -enable-requirement-machine-opaque-archetypes.
+// See opaque_archetype_concrete_requirement_recursive.swift for a demonstration
+// that it works without the flag (but more involved examples like the above
+// won't work).
+
+protocol RecursiveP {
+  associatedtype T : RecursiveP
+}
+
+struct S_RecursiveP : RecursiveP {
+  typealias T = S_RecursiveP
+}
+
+struct DefinesRecursiveP : P {
+  var t: some RecursiveP {
+    return S_RecursiveP()
+  }
+}
+
+protocol HasRecursiveP {
+  associatedtype T : RecursiveP
+}
+
+extension HasRecursiveP where T == DefinesRecursiveP.T {}
+// expected-error@-1 {{cannot build rewrite system for generic signature; rule length limit exceeded}}
+// expected-note@-2 {{failed rewrite rule is τ_0_0.[HasRecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[concrete: ((((((((((some RecursiveP).T).T).T).T).T).T).T).T).T).T] => τ_0_0.[HasRecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T]}}
+
diff --git a/test/Generics/opaque_archetype_concrete_requirement_recursive.swift b/test/Generics/opaque_archetype_concrete_requirement_recursive.swift
diff --git a/validation-test/compiler_crashers_2_fixed/unsupported_recursive_opaque_conformance.swift b/validation-test/compiler_crashers_2_fixed/unsupported_recursive_opaque_conformance.swift
