Merge pull request #41524 from slavapestov/rqm-tiny-cleanup-and-comments

slavapestov · web-flow · commit efa055c3230b · 2022-02-23T14:37:18.000-05:00
RequirementMachine: Small cleanup and some comments
diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h
@@ -696,7 +696,7 @@ class alignas(1 << TypeAlignInBits) TypeBase
   ///
   /// "Unresolved" dependent member types have no known associated type,
   /// and are only used transiently in the type checker.
-  const DependentMemberType *findUnresolvedDependentMemberType();
+  DependentMemberType *findUnresolvedDependentMemberType();
 
   /// Return the root generic parameter of this type parameter type.
   GenericTypeParamType *getRootGenericParam();
diff --git a/lib/AST/RequirementMachine/Debug.h b/lib/AST/RequirementMachine/Debug.h
@@ -26,45 +26,42 @@ enum class DebugFlags : unsigned {
   /// Print debug output when adding rules.
   Add = (1<<1),
 
-  /// Print debug output when merging associated types.
-  Merge = (1<<2),
-
   /// Print debug output from the Knuth-Bendix algorithm.
-  Completion = (1<<3),
+  Completion = (1<<2),
 
   /// Print debug output from property map construction.
-  PropertyMap = (1<<4),
+  PropertyMap = (1<<3),
 
   /// Print debug output when unifying concrete types in the property map.
-  ConcreteUnification = (1<<5),
+  ConcreteUnification = (1<<4),
 
   /// Print debug output when concretizing nested types in the property map.
-  ConcretizeNestedTypes = (1<<6),
+  ConcretizeNestedTypes = (1<<5),
 
   /// Print debug output when inferring conditional requirements in the
   /// property map.
-  ConditionalRequirements = (1<<7),
+  ConditionalRequirements = (1<<6),
 
   /// Print debug output from the homotopy reduction algorithm.
-  HomotopyReduction = (1<<8),
+  HomotopyReduction = (1<<7),
 
   /// Print more detailed debug output from the homotopy reduction algorithm.
-  HomotopyReductionDetail = (1<<9),
+  HomotopyReductionDetail = (1<<8),
 
   /// Print debug output from the minimal conformances algorithm.
-  MinimalConformances = (1<<10),
+  MinimalConformances = (1<<9),
 
   /// Print debug output from the protocol dependency graph.
-  ProtocolDependencies = (1<<11),
+  ProtocolDependencies = (1<<10),
 
   /// Print debug output from generic signature minimization.
-  Minimization = (1<<12),
+  Minimization = (1<<11),
 
   /// Print redundant rules and their replacement paths.
-  RedundantRules = (1<<13),
+  RedundantRules = (1<<12),
 
   /// Print more detail about redundant rules.
-  RedundantRulesDetail = (1<<14)
+  RedundantRulesDetail = (1<<13)
 };
 
 using DebugOptions = OptionSet<DebugFlags>;
diff --git a/lib/AST/RequirementMachine/RequirementMachine.cpp b/lib/AST/RequirementMachine/RequirementMachine.cpp
@@ -36,38 +36,46 @@ RequirementMachine::RequirementMachine(RewriteContext &ctx)
 
 RequirementMachine::~RequirementMachine() {}
 
-static void checkCompletionResult(const RequirementMachine &machine,
-                                  CompletionResult result) {
+/// Checks the result of a completion in a context where we can't diagnose
+/// failure, either when building a rewrite system from an existing
+/// minimal signature (which should have been checked when it was
+/// minimized) or from AbstractGenericSignatureRequest (where failure
+/// is fatal).
+void RequirementMachine::checkCompletionResult(CompletionResult result) const {
   switch (result) {
   case CompletionResult::Success:
     break;
 
   case CompletionResult::MaxRuleCount:
     llvm::errs() << "Rewrite system exceeded maximum rule count\n";
-    machine.dump(llvm::errs());
+    dump(llvm::errs());
     abort();
 
   case CompletionResult::MaxRuleLength:
     llvm::errs() << "Rewrite system exceeded rule length limit\n";
-    machine.dump(llvm::errs());
+    dump(llvm::errs());
     abort();
 
   case CompletionResult::MaxConcreteNesting:
     llvm::errs() << "Rewrite system exceeded concrete type nesting depth limit\n";
-    machine.dump(llvm::errs());
+    dump(llvm::errs());
     abort();
   }
 }
 
 /// Build a requirement machine for the requirements of a generic signature.
 ///
+/// In this mode, minimization is not going to be performed, so rewrite loops
+/// are not recorded.
+///
 /// This must only be called exactly once, before any other operations are
 /// performed on this requirement machine.
 ///
 /// Used by ASTContext::getOrCreateRequirementMachine().
 ///
-/// Asserts if completion fails within the configured number of steps.
-void RequirementMachine::initWithGenericSignature(CanGenericSignature sig) {
+/// Returns failure if completion fails within the configured number of steps.
+std::pair<CompletionResult, unsigned>
+RequirementMachine::initWithGenericSignature(CanGenericSignature sig) {
   Sig = sig;
   Params.append(sig.getGenericParams().begin(),
                 sig.getGenericParams().end());
@@ -92,17 +100,21 @@ void RequirementMachine::initWithGenericSignature(CanGenericSignature sig) {
                     std::move(builder.RequirementRules));
 
   auto result = computeCompletion(RewriteSystem::DisallowInvalidRequirements);
-  checkCompletionResult(*this, result.first);
 
   if (Dump) {
     llvm::dbgs() << "}\n";
   }
+
+  return result;
 }
 
 /// Build a requirement machine for the structural requirements of a set
 /// of protocols, which are understood to form a strongly-connected component
 /// (SCC) of the protocol dependency graph.
 ///
+/// In this mode, minimization will be performed, so rewrite loops are recorded
+/// during completion.
+///
 /// This must only be called exactly once, before any other operations are
 /// performed on this requirement machine.
 ///
@@ -138,55 +150,16 @@ RequirementMachine::initWithProtocols(ArrayRef<const ProtocolDecl *> protos) {
   return result;
 }
 
-/// Build a requirement machine from a set of generic parameters and
-/// (possibly non-canonical or non-minimal) abstract requirements.
-///
-/// This must only be called exactly once, before any other operations are
-/// performed on this requirement machine.
-///
-/// Used by AbstractGenericSignatureRequest.
-///
-/// Asserts if completion fails within the configured number of steps.
-void RequirementMachine::initWithAbstractRequirements(
-    ArrayRef<GenericTypeParamType *> genericParams,
-    ArrayRef<Requirement> requirements) {
-  Params.append(genericParams.begin(), genericParams.end());
-
-  FrontendStatsTracer tracer(Stats, "build-rewrite-system");
-
-  if (Dump) {
-    llvm::dbgs() << "Adding generic parameters:";
-    for (auto *paramTy : genericParams)
-      llvm::dbgs() << " " << Type(paramTy);
-    llvm::dbgs() << "\n";
-  }
-
-  // Collect the top-level requirements, and all transtively-referenced
-  // protocol requirement signatures.
-  RuleBuilder builder(Context, System.getProtocolMap());
-  builder.addRequirements(requirements);
-
-  // Add the initial set of rewrite rules to the rewrite system.
-  System.initialize(/*recordLoops=*/true,
-                    /*protos=*/ArrayRef<const ProtocolDecl *>(),
-                    std::move(builder.PermanentRules),
-                    std::move(builder.RequirementRules));
-
-  auto result = computeCompletion(RewriteSystem::AllowInvalidRequirements);
-  checkCompletionResult(*this, result.first);
-
-  if (Dump) {
-    llvm::dbgs() << "}\n";
-  }
-}
-
 /// Build a requirement machine from a set of generic parameters and
 /// structural requirements.
 ///
+/// In this mode, minimization will be performed, so rewrite loops are recorded
+/// during completion.
+///
 /// This must only be called exactly once, before any other operations are
 /// performed on this requirement machine.
 ///
-/// Used by InferredGenericSignatureRequest.
+/// Used by AbstractGenericSignatureRequest and InferredGenericSignatureRequest.
 ///
 /// Returns failure if completion fails within the configured number of steps.
 std::pair<CompletionResult, unsigned>
diff --git a/lib/AST/RequirementMachine/RequirementMachine.h b/lib/AST/RequirementMachine/RequirementMachine.h
@@ -88,12 +88,14 @@ class RequirementMachine final {
   RequirementMachine &operator=(const RequirementMachine &) = delete;
   RequirementMachine &operator=(RequirementMachine &&) = delete;
 
-  void initWithGenericSignature(CanGenericSignature sig);
+  void checkCompletionResult(CompletionResult result) const;
+
+  std::pair<CompletionResult, unsigned>
+  initWithGenericSignature(CanGenericSignature sig);
+
   std::pair<CompletionResult, unsigned>
   initWithProtocols(ArrayRef<const ProtocolDecl *> protos);
-  void initWithAbstractRequirements(
-      ArrayRef<GenericTypeParamType *> genericParams,
-      ArrayRef<Requirement> requirements);
+
   std::pair<CompletionResult, unsigned>
   initWithWrittenRequirements(
       ArrayRef<GenericTypeParamType *> genericParams,
diff --git a/lib/AST/RequirementMachine/RequirementMachineRequests.cpp b/lib/AST/RequirementMachine/RequirementMachineRequests.cpp
@@ -473,9 +473,9 @@ AbstractGenericSignatureRequestRQM::evaluate(
     return GenericSignatureWithError(result, /*hadError=*/false);
   }
 
-  SmallVector<Requirement, 4> requirements(
-      baseSignature.getRequirements().begin(),
-      baseSignature.getRequirements().end());
+  SmallVector<StructuralRequirement, 4> requirements;
+  for (auto req : baseSignature.getRequirements())
+    requirements.push_back({req, SourceLoc(), /*wasInferred=*/false});
 
   // We need to create this errors vector to pass to
   // desugarRequirement, but this request should never
@@ -492,14 +492,20 @@ AbstractGenericSignatureRequestRQM::evaluate(
   // Desugaring converts these kinds of requirements into "proper"
   // requirements where the subject type is always a type parameter,
   // which is what the RuleBuilder expects.
-  for (auto req : addedRequirements)
-    desugarRequirement(req, requirements, errors);
+  for (auto req : addedRequirements) {
+    SmallVector<Requirement, 2> reqs;
+    desugarRequirement(req, reqs, errors);
+    for (auto req : reqs)
+      requirements.push_back({req, SourceLoc(), /*wasInferred=*/false});
+  }
 
   // Heap-allocate the requirement machine to save stack space.
   std::unique_ptr<RequirementMachine> machine(new RequirementMachine(
       ctx.getRewriteContext()));
 
-  machine->initWithAbstractRequirements(genericParams, requirements);
+  auto status =
+      machine->initWithWrittenRequirements(genericParams, requirements);
+  machine->checkCompletionResult(status.first);
 
   auto minimalRequirements =
     machine->computeMinimalGenericSignatureRequirements();
@@ -611,7 +617,8 @@ InferredGenericSignatureRequestRQM::evaluate(
   std::unique_ptr<RequirementMachine> machine(new RequirementMachine(
       ctx.getRewriteContext()));
 
-  auto status = machine->initWithWrittenRequirements(genericParams, requirements);
+  auto status =
+      machine->initWithWrittenRequirements(genericParams, requirements);
   if (status.first != CompletionResult::Success) {
     ctx.Diags.diagnose(loc,
                        diag::requirement_machine_completion_failed,
diff --git a/lib/AST/RequirementMachine/RewriteContext.cpp b/lib/AST/RequirementMachine/RewriteContext.cpp
@@ -29,7 +29,6 @@ static DebugOptions parseDebugFlags(StringRef debugFlags) {
     auto flag = llvm::StringSwitch<Optional<DebugFlags>>(flagStr)
       .Case("simplify", DebugFlags::Simplify)
       .Case("add", DebugFlags::Add)
-      .Case("merge", DebugFlags::Merge)
       .Case("completion", DebugFlags::Completion)
       .Case("property-map", DebugFlags::PropertyMap)
       .Case("concrete-unification", DebugFlags::ConcreteUnification)
@@ -134,7 +133,9 @@ RequirementMachine *RewriteContext::getRequirementMachine(
 
   // This might re-entrantly invalidate 'machine', which is a reference
   // into Protos.
-  newMachine->initWithGenericSignature(sig);
+  auto status = newMachine->initWithGenericSignature(sig);
+  newMachine->checkCompletionResult(status.first);
+
   return newMachine;
 }
 
diff --git a/lib/AST/RequirementMachine/SimplifySubstitutions.cpp b/lib/AST/RequirementMachine/SimplifySubstitutions.cpp
@@ -1,4 +1,4 @@
-//===--- PropertyUnification.cpp - Rules added w/ building property map ---===//
+//===--- SimplifySubstitutions.cpp - Simplify concrete type rules ---------===//
 //
 // This source file is part of the Swift.org open source project
 //
@@ -9,6 +9,57 @@
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 //===----------------------------------------------------------------------===//
+//
+// Implements a pass for simplifying substitutions in concrete type symbols.
+// Substitutions can be simplifed in one of two ways; either a substitution
+// term can be replaced by a more canonical term, or it can be replaced by a
+// concrete type.
+//
+// For example, given pair of rewrite rules:
+//
+//    T.[concrete: G<Y>] => T
+//    Y => X
+//
+// We can apply (Y => X) to the term appearing in the concrete type symbol
+// [concrete: G<Y>] to obtain the rule:
+//
+//    T.[concrete: G<X>] => T
+//
+// Similarly, if we have a pair of rewrite rules:
+//
+//    T.[concrete: G<Y>] => T
+//    Y.[concrete: Int] => Y
+//
+// We can obtain the new rule:
+//
+//    T.[concrete: G<Int>] => T
+//
+// Substitution simplification occurs during the Knuth-Bendix completion
+// procedure, and after property map construction.
+//
+// In the first case, no property map is available yet, so substitution terms
+// are simplified to other terms, but concrete type replacement is not
+// performed. In the second case, the property map is consulted to perform
+// concrete type replacement where appropriate.
+//
+// Either the new rule or the old rule can become redundant; they are related
+// by rewrite loops. Additionally, rewrite loops are introduced for each
+// transformation applied to the substitutions to relate them to the concrete
+// type rules via "projections".
+//
+// These rewrite loops are in a sense dual to the property map's concrete type
+// unification, and share a lot of the code; whereas the property map will
+// relate two rules (T.[concrete: G<X>] => T) with (T.[concrete: G<Y>] => T)
+// and add the induced rule (Y => X), substitution simplification will use
+// (Y => X) to transform (T.[concrete: G<Y>] => T) into
+// (T.[concrete: G<X>] => T).
+//
+// This logic (and concrete type unification) heavily relies on the "type
+// difference" abstraction implemented in TypeDifference.cpp. Technical details
+// about the various rewrite loops introduced here can be found in comments at
+// the top of various functions below.
+//
+//===----------------------------------------------------------------------===//
 
 #include "PropertyMap.h"
 #include "RewriteSystem.h"
diff --git a/lib/AST/RequirementMachine/TypeDifference.cpp b/lib/AST/RequirementMachine/TypeDifference.cpp
@@ -9,6 +9,16 @@
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 //===----------------------------------------------------------------------===//
+//
+// A mechanism for working with types that are related via the transformation
+// of replacing a type parameter term with another type parameter term or
+// concrete type.
+//
+// Used by concrete type unification (in PropertyUnification.cpp) and for
+// substitution simplification (SimplifySubstitutions.cpp) to define rewrite
+// loops relating various rules for rewrite system minimization.
+//
+//===----------------------------------------------------------------------===//
 
 #include "TypeDifference.h"
 #include "swift/AST/Types.h"
diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp
@@ -4540,10 +4540,10 @@ Type Type::subst(TypeSubstitutionFn substitutions,
   return substType(*this, substitutions, conformances, options);
 }
 
-const DependentMemberType *TypeBase::findUnresolvedDependentMemberType() {
+DependentMemberType *TypeBase::findUnresolvedDependentMemberType() {
   if (!hasTypeParameter()) return nullptr;
 
-  const DependentMemberType *unresolvedDepMemTy = nullptr;
+  DependentMemberType *unresolvedDepMemTy = nullptr;
   Type(this).findIf([&](Type type) -> bool {
     if (auto depMemTy = type->getAs<DependentMemberType>()) {
       if (depMemTy->getAssocType() == nullptr) {
