From 7ea975015fae6d0f5291c0c16b690594d0d645b8 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 1 Oct 2024 20:01:19 -0400 Subject: [PATCH 01/26] Sema: Record ASTNode types in the trail --- include/swift/Sema/CSTrail.h | 12 +++++++- include/swift/Sema/ConstraintSystem.h | 40 +++++++++++++++++---------- lib/Sema/CSSolver.cpp | 12 -------- lib/Sema/CSTrail.cpp | 25 +++++++++++++++++ 4 files changed, 62 insertions(+), 27 deletions(-) diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index 93ab0c93f3d7c..be15fbeb781b3 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -76,8 +76,10 @@ class SolverTrail { /// Recorded the mapping from a pack element expression to its parent /// pack expansion expression. RecordedPackEnvironment, - /// Record a defaulted constraint at a locator. + /// Recorded a defaulted constraint at a locator. RecordedDefaultedConstraint, + /// Recorded an assignment of a type to an AST node. + RecordedNodeType, }; /// A change made to the constraint system. @@ -142,6 +144,11 @@ class SolverTrail { Type ReqTy; } FixedRequirement; + struct { + ASTNode Node; + Type OldType; + } Node; + ConstraintLocator *Locator; PackExpansionType *ExpansionTy; PackElementExpr *ElementExpr; @@ -221,6 +228,9 @@ class SolverTrail { /// Create a change that recorded a defaulted constraint at a locator. static Change recordedDefaultedConstraint(ConstraintLocator *locator); + /// Create a change that recorded an assignment of a type to an AST node. + static Change recordedNodeType(ASTNode node, Type oldType); + /// Undo this change, reverting the constraint graph to the state it /// had prior to this change. /// diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 0dc75fbf43742..9ace730820691 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2256,11 +2256,7 @@ class ConstraintSystem { /// nodes themselves. This allows us to typecheck and /// run through various diagnostics passes without actually mutating /// the types on the nodes. - llvm::MapVector NodeTypes; - - /// The nodes for which we have produced types, along with the prior type - /// each node had before introducing this type. - llvm::SmallVector, 8> addedNodeTypes; + llvm::DenseMap NodeTypes; /// Maps components in a key path expression to their type. Needed because /// KeyPathExpr + Index isn't an \c ASTNode and thus can't be stored in \c @@ -2894,8 +2890,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - unsigned numAddedNodeTypes; - unsigned numAddedKeyPathComponentTypes; unsigned numDisabledConstraints; @@ -3205,21 +3199,39 @@ class ConstraintSystem { this->FavoredTypes[E] = T; } - /// Set the type in our type map for the given node. + /// Set the type in our type map for the given node, and record the change + /// in the trail. /// - /// The side tables are used through the expression type checker to avoid mutating nodes until - /// we know we have successfully type-checked them. + /// The side tables are used through the expression type checker to avoid + /// mutating nodes until we know we have successfully type-checked them. void setType(ASTNode node, Type type) { - assert(!node.isNull() && "Cannot set type information on null node"); - assert(type && "Expected non-null type"); + ASSERT(!node.isNull() && "Cannot set type information on null node"); + ASSERT(type && "Expected non-null type"); // Record the type. Type &entry = NodeTypes[node]; Type oldType = entry; entry = type; - // Record the fact that we ascribed a type to this node. - addedNodeTypes.push_back({node, oldType}); + if (oldType.getPointer() != type.getPointer()) { + // Record the fact that we assigned a type to this node. + if (isRecordingChanges()) + recordChange(SolverTrail::Change::recordedNodeType(node, oldType)); + } + } + + /// Undo the above change. + void restoreType(ASTNode node, Type oldType) { + ASSERT(!node.isNull() && "Cannot set type information on null node"); + + if (oldType) { + auto found = NodeTypes.find(node); + ASSERT(found != NodeTypes.end()); + found->second = oldType; + } else { + bool erased = NodeTypes.erase(node); + ASSERT(erased); + } } /// Set the type in our type map for a given expression. The side diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 2a7c2a8d3d2ab..9b442315c6fbd 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -663,7 +663,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); - numAddedNodeTypes = cs.addedNodeTypes.size(); numAddedKeyPathComponentTypes = cs.addedKeyPathComponentTypes.size(); numKeyPaths = cs.KeyPaths.size(); numDisabledConstraints = cs.solverState->getNumDisabledConstraints(); @@ -718,17 +717,6 @@ ConstraintSystem::SolverScope::~SolverScope() { // constraints introduced by the current scope. cs.solverState->rollback(this); - // Remove any node types we registered. - for (unsigned i : - reverse(range(numAddedNodeTypes, cs.addedNodeTypes.size()))) { - auto node = cs.addedNodeTypes[i].first; - if (Type oldType = cs.addedNodeTypes[i].second) - cs.NodeTypes[node] = oldType; - else - cs.NodeTypes.erase(node); - } - truncate(cs.addedNodeTypes, numAddedNodeTypes); - // Remove any node types we registered. for (unsigned i : reverse(range(numAddedKeyPathComponentTypes, cs.addedKeyPathComponentTypes.size()))) { diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index 8f34347e78691..c46c0c0e284e3 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -225,6 +225,15 @@ SolverTrail::Change::recordedDefaultedConstraint(ConstraintLocator *locator) { return result; } +SolverTrail::Change +SolverTrail::Change::recordedNodeType(ASTNode node, Type oldType) { + Change result; + result.Kind = ChangeKind::RecordedNodeType; + result.Node.Node = node; + result.Node.OldType = oldType; + return result; +} + void SolverTrail::Change::undo(ConstraintSystem &cs) const { auto &cg = cs.getConstraintGraph(); @@ -313,6 +322,10 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { case ChangeKind::RecordedDefaultedConstraint: cs.removeDefaultedConstraint(Locator); break; + + case ChangeKind::RecordedNodeType: + cs.restoreType(Node.Node, Node.OldType); + break; } } @@ -468,6 +481,7 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, break; case ChangeKind::RecordedPackEnvironment: + // FIXME: Print short form of PackExpansionExpr out << "(recorded pack environment)\n"; break; @@ -476,6 +490,17 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, Locator->dump(&cs.getASTContext().SourceMgr, out); out << ")\n"; break; + + case ChangeKind::RecordedNodeType: + out << "(recorded node type at "; + Node.Node.getStartLoc().print(out, cs.getASTContext().SourceMgr); + out << " previous "; + if (Node.OldType) + Node.OldType->print(out, PO); + else + out << "null"; + out << ")\n"; + break; } } From 411c590bc8b4a7ba3240f4d3d47b4d64b7141fbd Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Wed, 2 Oct 2024 14:58:21 -0400 Subject: [PATCH 02/26] Sema: Record key path component types in the trail --- include/swift/Sema/CSTrail.h | 12 ++++++++ include/swift/Sema/ConstraintSystem.h | 40 +++++++++++++++++---------- lib/Sema/CSSolver.cpp | 14 ---------- lib/Sema/CSTrail.cpp | 31 ++++++++++++++++++++- 4 files changed, 68 insertions(+), 29 deletions(-) diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index be15fbeb781b3..dc516d7a64de5 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -80,6 +80,8 @@ class SolverTrail { RecordedDefaultedConstraint, /// Recorded an assignment of a type to an AST node. RecordedNodeType, + /// Recorded an assignment of a type to a keypath component. + RecordedKeyPathComponentType, }; /// A change made to the constraint system. @@ -149,6 +151,11 @@ class SolverTrail { Type OldType; } Node; + struct { + const KeyPathExpr *Expr; + Type OldType; + } KeyPath; + ConstraintLocator *Locator; PackExpansionType *ExpansionTy; PackElementExpr *ElementExpr; @@ -231,6 +238,11 @@ class SolverTrail { /// Create a change that recorded an assignment of a type to an AST node. static Change recordedNodeType(ASTNode node, Type oldType); + /// Create a change that recorded an assignment of a type to an AST node. + static Change recordedKeyPathComponentType(const KeyPathExpr *expr, + unsigned component, + Type oldType); + /// Undo this change, reverting the constraint graph to the state it /// had prior to this change. /// diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 9ace730820691..4e026cc559e30 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2265,11 +2265,6 @@ class ConstraintSystem { Type> KeyPathComponentTypes; - /// Same as \c addedNodeTypes for \c KeyPathComponentTypes. - llvm::SmallVector< - std::tuple> - addedKeyPathComponentTypes; - /// Maps a key path root, value, and decl context to the key path expression. llvm::MapVector 0; + } + /// Set the type in our type map for a given expression. The side /// map is used throughout the expression type checker in order to /// avoid mutating expressions until we know we have successfully /// type-checked them. void setType(const KeyPathExpr *KP, unsigned I, Type T) { - assert(KP && "Expected non-null key path parameter!"); - assert(T && "Expected non-null type!"); + ASSERT(KP && "Expected non-null key path parameter!"); + ASSERT(T && "Expected non-null type!"); Type &entry = KeyPathComponentTypes[{KP, I}]; Type oldType = entry; entry = T; - addedKeyPathComponentTypes.push_back(std::make_tuple(KP, I, oldType)); + if (oldType.getPointer() != T.getPointer()) { + if (isRecordingChanges()) { + recordChange( + SolverTrail::Change::recordedKeyPathComponentType( + KP, I, oldType)); + } + } } - /// Check to see if we have a type for a node. - bool hasType(ASTNode node) const { - assert(!node.isNull() && "Expected non-null node"); - return NodeTypes.count(node) > 0; + void restoreType(const KeyPathExpr *KP, unsigned I, Type T) { + ASSERT(KP && "Expected non-null key path parameter!"); + + if (T) { + auto found = KeyPathComponentTypes.find({KP, I}); + ASSERT(found != KeyPathComponentTypes.end()); + found->second = T; + } else { + bool erased = KeyPathComponentTypes.erase({KP, I}); + ASSERT(erased); + } } bool hasType(const KeyPathExpr *KP, unsigned I) const { diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 9b442315c6fbd..382acad509f35 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -663,7 +663,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); - numAddedKeyPathComponentTypes = cs.addedKeyPathComponentTypes.size(); numKeyPaths = cs.KeyPaths.size(); numDisabledConstraints = cs.solverState->getNumDisabledConstraints(); numFavoredConstraints = cs.solverState->getNumFavoredConstraints(); @@ -717,19 +716,6 @@ ConstraintSystem::SolverScope::~SolverScope() { // constraints introduced by the current scope. cs.solverState->rollback(this); - // Remove any node types we registered. - for (unsigned i : reverse(range(numAddedKeyPathComponentTypes, - cs.addedKeyPathComponentTypes.size()))) { - auto KeyPath = std::get<0>(cs.addedKeyPathComponentTypes[i]); - auto KeyPathIndex = std::get<1>(cs.addedKeyPathComponentTypes[i]); - if (Type oldType = std::get<2>(cs.addedKeyPathComponentTypes[i])) { - cs.KeyPathComponentTypes[{KeyPath, KeyPathIndex}] = oldType; - } else { - cs.KeyPathComponentTypes.erase({KeyPath, KeyPathIndex}); - } - } - truncate(cs.addedKeyPathComponentTypes, numAddedKeyPathComponentTypes); - /// Remove any key path expressions. truncate(cs.KeyPaths, numKeyPaths); diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index c46c0c0e284e3..b69433bca2634 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -234,6 +234,18 @@ SolverTrail::Change::recordedNodeType(ASTNode node, Type oldType) { return result; } +SolverTrail::Change +SolverTrail::Change::recordedKeyPathComponentType(const KeyPathExpr *expr, + unsigned component, + Type oldType) { + Change result; + result.Kind = ChangeKind::RecordedKeyPathComponentType; + result.Options = component; + result.KeyPath.Expr = expr; + result.KeyPath.OldType = oldType; + return result; +} + void SolverTrail::Change::undo(ConstraintSystem &cs) const { auto &cg = cs.getConstraintGraph(); @@ -326,6 +338,10 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { case ChangeKind::RecordedNodeType: cs.restoreType(Node.Node, Node.OldType); break; + + case ChangeKind::RecordedKeyPathComponentType: + cs.restoreType(KeyPath.Expr, Options, KeyPath.OldType); + break; } } @@ -482,7 +498,9 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, case ChangeKind::RecordedPackEnvironment: // FIXME: Print short form of PackExpansionExpr - out << "(recorded pack environment)\n"; + out << "(recorded pack environment"; + simple_display(out, ElementExpr); + out << "\n"; break; case ChangeKind::RecordedDefaultedConstraint: @@ -501,6 +519,17 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, out << "null"; out << ")\n"; break; + + case ChangeKind::RecordedKeyPathComponentType: + out << "(recorded key path "; + simple_display(out, KeyPath.Expr); + out << " with component type "; + if (Node.OldType) + Node.OldType->print(out, PO); + else + out << "null"; + out << " for component " << Options << ")\n"; + break; } } From b2adf51d7805eadf82235fc09f0ae88b939dcb51 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Wed, 2 Oct 2024 15:17:24 -0400 Subject: [PATCH 03/26] Sema: Record disabled constraints in the trail --- include/swift/Sema/CSTrail.h | 5 +++++ include/swift/Sema/ConstraintSystem.h | 18 +----------------- lib/Sema/CSSolver.cpp | 11 ++++++----- lib/Sema/CSTrail.cpp | 20 ++++++++++++++++++++ 4 files changed, 32 insertions(+), 22 deletions(-) diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index dc516d7a64de5..fc5657f65feff 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -82,6 +82,8 @@ class SolverTrail { RecordedNodeType, /// Recorded an assignment of a type to a keypath component. RecordedKeyPathComponentType, + /// Disabled a constraint. + DisabledConstraint, }; /// A change made to the constraint system. @@ -243,6 +245,9 @@ class SolverTrail { unsigned component, Type oldType); + /// Create a change that disabled a constraint. + static Change disabledConstraint(Constraint *constraint); + /// Undo this change, reverting the constraint graph to the state it /// had prior to this change. /// diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 4e026cc559e30..4cd179ec1835e 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2632,15 +2632,6 @@ class ConstraintSystem { generatedConstraints.erase(genStart, genEnd); - for (unsigned constraintIdx : - range(scope->numDisabledConstraints, disabledConstraints.size())) { - if (disabledConstraints[constraintIdx]->isDisabled()) - disabledConstraints[constraintIdx]->setEnabled(); - } - disabledConstraints.erase( - disabledConstraints.begin() + scope->numDisabledConstraints, - disabledConstraints.end()); - for (unsigned constraintIdx : range(scope->numFavoredConstraints, favoredConstraints.size())) { if (favoredConstraints[constraintIdx]->isFavored()) @@ -2657,15 +2648,11 @@ class ConstraintSystem { return AllowFreeTypeVariables != FreeTypeVariableBinding::Disallow; } - unsigned getNumDisabledConstraints() const { - return disabledConstraints.size(); - } - /// Disable the given constraint; this change will be rolled back /// when we exit the current solver scope. void disableConstraint(Constraint *constraint) { constraint->setDisabled(); - disabledConstraints.push_back(constraint); + Trail.recordChange(SolverTrail::Change::disabledConstraint(constraint)); } unsigned getNumFavoredConstraints() const { @@ -2702,7 +2689,6 @@ class ConstraintSystem { llvm::SmallVector< std::tuple, 4> scopes; - SmallVector disabledConstraints; SmallVector favoredConstraints; /// Depth of the solution stack. @@ -2885,8 +2871,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - unsigned numDisabledConstraints; - unsigned numFavoredConstraints; unsigned numResultBuilderTransformed; diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 382acad509f35..ef96b38887ccd 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -664,7 +664,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); numKeyPaths = cs.KeyPaths.size(); - numDisabledConstraints = cs.solverState->getNumDisabledConstraints(); numFavoredConstraints = cs.solverState->getNumFavoredConstraints(); numResultBuilderTransformed = cs.resultBuilderTransformed.size(); numAppliedPropertyWrappers = cs.appliedPropertyWrappers.size(); @@ -1771,10 +1770,12 @@ ConstraintSystem::filterDisjunction( if (restoreOnFail) constraintsToRestoreOnFail.push_back(constraint); - if (solverState) - solverState->disableConstraint(constraint); - else - constraint->setDisabled(); + if (!constraint->isDisabled()) { + if (solverState) + solverState->disableConstraint(constraint); + else + constraint->setDisabled(); + } } switch (numEnabledTerms) { diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index b69433bca2634..6208d187ff750 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -246,6 +246,14 @@ SolverTrail::Change::recordedKeyPathComponentType(const KeyPathExpr *expr, return result; } +SolverTrail::Change +SolverTrail::Change::disabledConstraint(Constraint *constraint) { + Change result; + result.Kind = ChangeKind::DisabledConstraint; + result.TheConstraint.Constraint = constraint; + return result; +} + void SolverTrail::Change::undo(ConstraintSystem &cs) const { auto &cg = cs.getConstraintGraph(); @@ -342,6 +350,11 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { case ChangeKind::RecordedKeyPathComponentType: cs.restoreType(KeyPath.Expr, Options, KeyPath.OldType); break; + + case ChangeKind::DisabledConstraint: + if (TheConstraint.Constraint->isDisabled()) + TheConstraint.Constraint->setEnabled(); + break; } } @@ -530,6 +543,13 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, out << "null"; out << " for component " << Options << ")\n"; break; + + case ChangeKind::DisabledConstraint: + out << "(disabled constraint "; + TheConstraint.Constraint->print(out, &cs.getASTContext().SourceMgr, + indent + 2); + out << ")\n"; + break; } } From 9115a46736d64391e88971d01e20db68a5f9939e Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Wed, 2 Oct 2024 15:24:06 -0400 Subject: [PATCH 04/26] Sema: Record favored constraints in the trail --- include/swift/Sema/CSTrail.h | 5 +++++ include/swift/Sema/ConstraintSystem.h | 21 +++------------------ lib/Sema/CSSolver.cpp | 1 - lib/Sema/CSTrail.cpp | 20 ++++++++++++++++++++ 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index fc5657f65feff..ebfb7974b606e 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -84,6 +84,8 @@ class SolverTrail { RecordedKeyPathComponentType, /// Disabled a constraint. DisabledConstraint, + /// Favored a constraint. + FavoredConstraint, }; /// A change made to the constraint system. @@ -248,6 +250,9 @@ class SolverTrail { /// Create a change that disabled a constraint. static Change disabledConstraint(Constraint *constraint); + /// Create a change that favored a constraint. + static Change favoredConstraint(Constraint *constraint); + /// Undo this change, reverting the constraint graph to the state it /// had prior to this change. /// diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 4cd179ec1835e..4a8c563ebfbe6 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2631,15 +2631,6 @@ class ConstraintSystem { } generatedConstraints.erase(genStart, genEnd); - - for (unsigned constraintIdx : - range(scope->numFavoredConstraints, favoredConstraints.size())) { - if (favoredConstraints[constraintIdx]->isFavored()) - favoredConstraints[constraintIdx]->setFavored(false); - } - favoredConstraints.erase( - favoredConstraints.begin() + scope->numFavoredConstraints, - favoredConstraints.end()); } /// Check whether constraint system is allowed to form solutions @@ -2651,21 +2642,18 @@ class ConstraintSystem { /// Disable the given constraint; this change will be rolled back /// when we exit the current solver scope. void disableConstraint(Constraint *constraint) { + ASSERT(!constraint->isDisabled()); constraint->setDisabled(); Trail.recordChange(SolverTrail::Change::disabledConstraint(constraint)); } - unsigned getNumFavoredConstraints() const { - return favoredConstraints.size(); - } - /// Favor the given constraint; this change will be rolled back /// when we exit the current solver scope. void favorConstraint(Constraint *constraint) { assert(!constraint->isFavored()); constraint->setFavored(); - favoredConstraints.push_back(constraint); + Trail.recordChange(SolverTrail::Change::favoredConstraint(constraint)); } private: @@ -2689,8 +2677,7 @@ class ConstraintSystem { llvm::SmallVector< std::tuple, 4> scopes; - SmallVector favoredConstraints; - + /// Depth of the solution stack. unsigned depth = 0; }; @@ -2871,8 +2858,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - unsigned numFavoredConstraints; - unsigned numResultBuilderTransformed; /// The length of \c appliedPropertyWrappers diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index ef96b38887ccd..221facde210d7 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -664,7 +664,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); numKeyPaths = cs.KeyPaths.size(); - numFavoredConstraints = cs.solverState->getNumFavoredConstraints(); numResultBuilderTransformed = cs.resultBuilderTransformed.size(); numAppliedPropertyWrappers = cs.appliedPropertyWrappers.size(); numResolvedOverloads = cs.ResolvedOverloads.size(); diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index 6208d187ff750..5152b6bd5251d 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -254,6 +254,14 @@ SolverTrail::Change::disabledConstraint(Constraint *constraint) { return result; } +SolverTrail::Change +SolverTrail::Change::favoredConstraint(Constraint *constraint) { + Change result; + result.Kind = ChangeKind::FavoredConstraint; + result.TheConstraint.Constraint = constraint; + return result; +} + void SolverTrail::Change::undo(ConstraintSystem &cs) const { auto &cg = cs.getConstraintGraph(); @@ -355,6 +363,11 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { if (TheConstraint.Constraint->isDisabled()) TheConstraint.Constraint->setEnabled(); break; + + case ChangeKind::FavoredConstraint: + if (TheConstraint.Constraint->isFavored()) + TheConstraint.Constraint->setFavored(false); + break; } } @@ -550,6 +563,13 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, indent + 2); out << ")\n"; break; + + case ChangeKind::FavoredConstraint: + out << "(favored constraint "; + TheConstraint.Constraint->print(out, &cs.getASTContext().SourceMgr, + indent + 2); + out << ")\n"; + break; } } From 61575d991362f48bda2ce505f44de59435549851 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Wed, 2 Oct 2024 20:14:51 -0400 Subject: [PATCH 05/26] Sema: Remove a few isRecordingChanges() checks to tighten invariants In these places we now assert if we attempt to record a change during an active undo. --- include/swift/Sema/ConstraintSystem.h | 7 +++---- lib/Sema/ConstraintSystem.cpp | 20 ++++++++++---------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 4a8c563ebfbe6..82441a8c6e76a 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2412,9 +2412,8 @@ class ConstraintSystem { void recordDefaultedConstraint(ConstraintLocator *locator) { bool inserted = DefaultedConstraints.insert(locator).second; if (inserted) { - if (isRecordingChanges()) { + if (solverState) recordChange(SolverTrail::Change::recordedDefaultedConstraint(locator)); - } } } @@ -3177,7 +3176,7 @@ class ConstraintSystem { if (oldType.getPointer() != type.getPointer()) { // Record the fact that we assigned a type to this node. - if (isRecordingChanges()) + if (solverState) recordChange(SolverTrail::Change::recordedNodeType(node, oldType)); } } @@ -3215,7 +3214,7 @@ class ConstraintSystem { entry = T; if (oldType.getPointer() != T.getPointer()) { - if (isRecordingChanges()) { + if (solverState) { recordChange( SolverTrail::Change::recordedKeyPathComponentType( KP, I, oldType)); diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 3d14ac3a5e5c4..130928dec0a51 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -266,7 +266,7 @@ void ConstraintSystem::addConversionRestriction( if (!inserted) return; - if (isRecordingChanges()) { + if (solverState) { recordChange(SolverTrail::Change::addedConversionRestriction( srcType, dstType)); } @@ -284,7 +284,7 @@ void ConstraintSystem::addFix(ConstraintFix *fix) { if (!inserted) return; - if (isRecordingChanges()) + if (solverState) recordChange(SolverTrail::Change::addedFix(fix)); } @@ -304,7 +304,7 @@ void ConstraintSystem::recordDisjunctionChoice( return; } - if (isRecordingChanges()) { + if (solverState) { recordChange(SolverTrail::Change::recordedDisjunctionChoice( locator, index)); } @@ -316,7 +316,7 @@ void ConstraintSystem::recordAppliedDisjunction( auto inserted = AppliedDisjunctions.insert( std::make_pair(locator, fnType)); if (inserted.second) { - if (isRecordingChanges()) { + if (solverState) { recordChange(SolverTrail::Change::recordedAppliedDisjunction(locator)); } } @@ -854,7 +854,7 @@ void ConstraintSystem::recordOpenedExistentialType( ConstraintLocator *locator, OpenedArchetypeType *opened) { bool inserted = OpenedExistentialTypes.insert({locator, opened}).second; if (inserted) { - if (isRecordingChanges()) + if (solverState) recordChange(SolverTrail::Change::recordedOpenedExistentialType(locator)); } } @@ -895,7 +895,7 @@ void ConstraintSystem::recordPackExpansionEnvironment( ConstraintLocator *locator, std::pair uuidAndShape) { bool inserted = PackExpansionEnvironments.insert({locator, uuidAndShape}).second; if (inserted) { - if (isRecordingChanges()) { + if (solverState) { recordChange( SolverTrail::Change::recordedPackExpansionEnvironment(locator)); } @@ -913,7 +913,7 @@ void ConstraintSystem::addPackEnvironment(PackElementExpr *packElement, bool inserted = PackEnvironments.insert({packElement, packExpansion}).second; if (inserted) { - if (isRecordingChanges()) + if (solverState) recordChange(SolverTrail::Change::recordedPackEnvironment(packElement)); } } @@ -1279,7 +1279,7 @@ void ConstraintSystem::recordOpenedPackExpansionType(PackExpansionType *expansio TypeVariableType *expansionVar) { bool inserted = OpenedPackExpansionTypes.insert({expansion, expansionVar}).second; if (inserted) { - if (isRecordingChanges()) + if (solverState) recordChange(SolverTrail::Change::recordedOpenedPackExpansionType(expansion)); } } @@ -1688,7 +1688,7 @@ void ConstraintSystem::recordOpenedType( ConstraintLocator *locator, ArrayRef openedTypes) { bool inserted = OpenedTypes.insert({locator, openedTypes}).second; if (inserted) { - if (isRecordingChanges()) + if (solverState) recordChange(SolverTrail::Change::recordedOpenedTypes(locator)); } } @@ -7443,7 +7443,7 @@ void ConstraintSystem::recordFixedRequirement(GenericTypeParamType *GP, bool inserted = FixedRequirements.insert( std::make_tuple(GP, reqKind, requirementTy.getPointer())).second; if (inserted) { - if (isRecordingChanges()) { + if (solverState) { recordChange(SolverTrail::Change::addedFixedRequirement( GP, reqKind, requirementTy)); } From 64293ece51ea482b10b3ba050783c8ff1e3318d3 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Wed, 2 Oct 2024 22:31:28 -0400 Subject: [PATCH 06/26] Sema: Push reconciliation down into applySolution() to strengthen invariants Now, we assert if you try to record the same change twice in any other code path. --- include/swift/Sema/ConstraintSystem.h | 3 +- lib/Sema/CSSimplify.cpp | 8 +-- lib/Sema/CSSolver.cpp | 42 +++++++++---- lib/Sema/ConstraintSystem.cpp | 90 ++++++++++++--------------- 4 files changed, 76 insertions(+), 67 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 82441a8c6e76a..c657aa7fe7941 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -4422,7 +4422,8 @@ class ConstraintSystem { /// Record the set of opened types for the given locator. void recordOpenedTypes( ConstraintLocatorBuilder locator, - const OpenedTypeMap &replacements); + const OpenedTypeMap &replacements, + bool fixmeAllowDuplicates=false); /// Check whether the given type conforms to the given protocol and if /// so return a valid conformance reference. diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 9b36ba1a58621..a41d1273df465 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -14927,10 +14927,10 @@ void ConstraintSystem::recordMatchCallArgumentResult( ConstraintLocator *locator, MatchCallArgumentResult result) { assert(locator->isLastElement()); bool inserted = argumentMatchingChoices.insert({locator, result}).second; - if (inserted) { - if (isRecordingChanges()) - recordChange(SolverTrail::Change::recordedMatchCallArgumentResult(locator)); - } + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::recordedMatchCallArgumentResult(locator)); } void ConstraintSystem::recordCallAsFunction(UnresolvedDotExpr *root, diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 221facde210d7..1ebea097e8fda 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -304,17 +304,20 @@ void ConstraintSystem::applySolution(const Solution &solution) { // Register the solution's disjunction choices. for (auto &choice : solution.DisjunctionChoices) { - recordDisjunctionChoice(choice.first, choice.second); + if (DisjunctionChoices.count(choice.first) == 0) + recordDisjunctionChoice(choice.first, choice.second); } // Register the solution's applied disjunctions. for (auto &choice : solution.AppliedDisjunctions) { - recordAppliedDisjunction(choice.first, choice.second); + if (AppliedDisjunctions.count(choice.first) == 0) + recordAppliedDisjunction(choice.first, choice.second); } // Remember all of the argument/parameter matching choices we made. for (auto &argumentMatch : solution.argumentMatchingChoices) { - recordMatchCallArgumentResult(argumentMatch.first, argumentMatch.second); + if (argumentMatchingChoices.count(argumentMatch.first) == 0) + recordMatchCallArgumentResult(argumentMatch.first, argumentMatch.second); } // Remember implied results. @@ -323,32 +326,40 @@ void ConstraintSystem::applySolution(const Solution &solution) { // Register the solution's opened types. for (const auto &opened : solution.OpenedTypes) { - recordOpenedType(opened.first, opened.second); + if (OpenedTypes.count(opened.first) == 0) + recordOpenedType(opened.first, opened.second); } // Register the solution's opened existential types. for (const auto &openedExistential : solution.OpenedExistentialTypes) { - recordOpenedExistentialType(openedExistential.first, openedExistential.second); + if (OpenedExistentialTypes.count(openedExistential.first) == 0) { + recordOpenedExistentialType(openedExistential.first, + openedExistential.second); + } } // Register the solution's opened pack expansion types. for (const auto &expansion : solution.OpenedPackExpansionTypes) { - recordOpenedPackExpansionType(expansion.first, expansion.second); + if (OpenedPackExpansionTypes.count(expansion.first) == 0) + recordOpenedPackExpansionType(expansion.first, expansion.second); } // Register the solutions's pack expansion environments. for (const auto &expansion : solution.PackExpansionEnvironments) { - recordPackExpansionEnvironment(expansion.first, expansion.second); + if (PackExpansionEnvironments.count(expansion.first) == 0) + recordPackExpansionEnvironment(expansion.first, expansion.second); } // Register the solutions's pack environments. for (auto &packEnvironment : solution.PackEnvironments) { - addPackEnvironment(packEnvironment.first, packEnvironment.second); + if (PackEnvironments.count(packEnvironment.first) == 0) + addPackEnvironment(packEnvironment.first, packEnvironment.second); } // Register the defaulted type variables. - for (auto *locator : solution.DefaultedConstraints) + for (auto *locator : solution.DefaultedConstraints) { recordDefaultedConstraint(locator); + } // Add the node types back. for (auto &nodeType : solution.nodeTypes) { @@ -424,12 +435,17 @@ void ConstraintSystem::applySolution(const Solution &solution) { } // Register any fixes produced along this path. - for (auto *fix : solution.Fixes) - addFix(fix); + for (auto *fix : solution.Fixes) { + if (Fixes.count(fix) == 0) + addFix(fix); + } // Register fixed requirements. - for (auto fix : solution.FixedRequirements) - recordFixedRequirement(std::get<0>(fix), std::get<1>(fix), std::get<2>(fix)); + for (auto fix : solution.FixedRequirements) { + recordFixedRequirement(std::get<0>(fix), + std::get<1>(fix), + std::get<2>(fix)); + } } bool ConstraintSystem::simplify() { // While we have a constraint in the worklist, process it. diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 130928dec0a51..5406471e24b45 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -281,8 +281,7 @@ void ConstraintSystem::removeConversionRestriction( void ConstraintSystem::addFix(ConstraintFix *fix) { bool inserted = Fixes.insert(fix); - if (!inserted) - return; + ASSERT(inserted); if (solverState) recordChange(SolverTrail::Change::addedFix(fix)); @@ -294,32 +293,23 @@ void ConstraintSystem::removeFix(ConstraintFix *fix) { } void ConstraintSystem::recordDisjunctionChoice( - ConstraintLocator *locator, - unsigned index) { - // We shouldn't ever register disjunction choices multiple times. - auto inserted = DisjunctionChoices.insert( - std::make_pair(locator, index)); - if (!inserted.second) { - ASSERT(inserted.first->second == index); - return; - } + ConstraintLocator *locator, unsigned index) { + bool inserted = DisjunctionChoices.insert({locator, index}).second; + ASSERT(inserted); - if (solverState) { - recordChange(SolverTrail::Change::recordedDisjunctionChoice( - locator, index)); - } + if (solverState) + recordChange(SolverTrail::Change::recordedDisjunctionChoice(locator, index)); } void ConstraintSystem::recordAppliedDisjunction( ConstraintLocator *locator, FunctionType *fnType) { // We shouldn't ever register disjunction choices multiple times. - auto inserted = AppliedDisjunctions.insert( - std::make_pair(locator, fnType)); - if (inserted.second) { - if (solverState) { - recordChange(SolverTrail::Change::recordedAppliedDisjunction(locator)); - } - } + bool inserted = AppliedDisjunctions.insert( + std::make_pair(locator, fnType)).second; + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::recordedAppliedDisjunction(locator)); } /// Retrieve a dynamic result signature for the given declaration. @@ -853,10 +843,10 @@ std::pair ConstraintSystem::openExistentialType( void ConstraintSystem::recordOpenedExistentialType( ConstraintLocator *locator, OpenedArchetypeType *opened) { bool inserted = OpenedExistentialTypes.insert({locator, opened}).second; - if (inserted) { - if (solverState) - recordChange(SolverTrail::Change::recordedOpenedExistentialType(locator)); - } + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::recordedOpenedExistentialType(locator)); } GenericEnvironment * @@ -894,12 +884,10 @@ ConstraintSystem::getPackElementEnvironment(ConstraintLocator *locator, void ConstraintSystem::recordPackExpansionEnvironment( ConstraintLocator *locator, std::pair uuidAndShape) { bool inserted = PackExpansionEnvironments.insert({locator, uuidAndShape}).second; - if (inserted) { - if (solverState) { - recordChange( - SolverTrail::Change::recordedPackExpansionEnvironment(locator)); - } - } + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::recordedPackExpansionEnvironment(locator)); } PackExpansionExpr * @@ -910,12 +898,11 @@ ConstraintSystem::getPackEnvironment(PackElementExpr *packElement) const { void ConstraintSystem::addPackEnvironment(PackElementExpr *packElement, PackExpansionExpr *packExpansion) { - bool inserted = - PackEnvironments.insert({packElement, packExpansion}).second; - if (inserted) { - if (solverState) - recordChange(SolverTrail::Change::recordedPackEnvironment(packElement)); - } + bool inserted = PackEnvironments.insert({packElement, packExpansion}).second; + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::recordedPackEnvironment(packElement)); } /// Extend the given depth map by adding depths for all of the subexpressions @@ -1028,7 +1015,8 @@ Type ConstraintSystem::openUnboundGenericType(GenericTypeDecl *decl, openGeneric(decl->getDeclContext(), decl->getGenericSignature(), locator, replacements); - recordOpenedTypes(locator, replacements); + // FIXME: Get rid of fixmeAllowDuplicates. + recordOpenedTypes(locator, replacements, /*fixmeAllowDuplicates=*/true); if (parentTy) { const auto parentTyInContext = @@ -1278,10 +1266,10 @@ Type ConstraintSystem::openPackExpansionType(PackExpansionType *expansion, void ConstraintSystem::recordOpenedPackExpansionType(PackExpansionType *expansion, TypeVariableType *expansionVar) { bool inserted = OpenedPackExpansionTypes.insert({expansion, expansionVar}).second; - if (inserted) { - if (solverState) - recordChange(SolverTrail::Change::recordedOpenedPackExpansionType(expansion)); - } + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::recordedOpenedPackExpansionType(expansion)); } Type ConstraintSystem::openOpaqueType(OpaqueTypeArchetypeType *opaque, @@ -1687,10 +1675,10 @@ Type ConstraintSystem::getUnopenedTypeOfReference( void ConstraintSystem::recordOpenedType( ConstraintLocator *locator, ArrayRef openedTypes) { bool inserted = OpenedTypes.insert({locator, openedTypes}).second; - if (inserted) { - if (solverState) - recordChange(SolverTrail::Change::recordedOpenedTypes(locator)); - } + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::recordedOpenedTypes(locator)); } void ConstraintSystem::removeOpenedType(ConstraintLocator *locator) { @@ -1700,7 +1688,8 @@ void ConstraintSystem::removeOpenedType(ConstraintLocator *locator) { void ConstraintSystem::recordOpenedTypes( ConstraintLocatorBuilder locator, - const OpenedTypeMap &replacements) { + const OpenedTypeMap &replacements, + bool fixmeAllowDuplicates) { if (replacements.empty()) return; @@ -1721,7 +1710,10 @@ void ConstraintSystem::recordOpenedTypes( OpenedType* openedTypes = Allocator.Allocate(replacements.size()); std::copy(replacements.begin(), replacements.end(), openedTypes); - recordOpenedType( + + // FIXME: Get rid of fixmeAllowDuplicates. + if (!fixmeAllowDuplicates || OpenedTypes.count(locatorPtr) == 0) + recordOpenedType( locatorPtr, llvm::ArrayRef(openedTypes, replacements.size())); } From c7edc3494e4d827191b0e7d297fdc87080172a20 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Wed, 2 Oct 2024 22:43:42 -0400 Subject: [PATCH 07/26] Sema: Record result builder transforms in the trail --- include/swift/Sema/CSTrail.h | 9 +++++++++ include/swift/Sema/ConstraintSystem.h | 10 ++++++++-- lib/Sema/BuilderTransform.cpp | 26 +++++++++++++++++--------- lib/Sema/CSSolver.cpp | 7 ++----- lib/Sema/CSTrail.cpp | 18 ++++++++++++++++++ 5 files changed, 54 insertions(+), 16 deletions(-) diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index ebfb7974b606e..4f715a929a847 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -17,6 +17,9 @@ #ifndef SWIFT_SEMA_CSTRAIL_H #define SWIFT_SEMA_CSTRAIL_H +#include "swift/AST/AnyFunctionRef.h" +#include "swift/AST/Type.h" +#include "swift/AST/Types.h" #include namespace llvm { @@ -86,6 +89,8 @@ class SolverTrail { DisabledConstraint, /// Favored a constraint. FavoredConstraint, + /// Recorded a result builder transform. + RecordedResultBuilderTransform, }; /// A change made to the constraint system. @@ -163,6 +168,7 @@ class SolverTrail { ConstraintLocator *Locator; PackExpansionType *ExpansionTy; PackElementExpr *ElementExpr; + AnyFunctionRef AFR; }; Change() : Kind(ChangeKind::AddedTypeVariable), TypeVar(nullptr) { } @@ -253,6 +259,9 @@ class SolverTrail { /// Create a change that favored a constraint. static Change favoredConstraint(Constraint *constraint); + /// Create a change that recorded a result builder transform. + static Change recordedResultBuilderTransform(AnyFunctionRef fn); + /// Undo this change, reverting the constraint graph to the state it /// had prior to this change. /// diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index c657aa7fe7941..65bae1e320fee 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2857,8 +2857,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - unsigned numResultBuilderTransformed; - /// The length of \c appliedPropertyWrappers unsigned numAppliedPropertyWrappers; @@ -5317,6 +5315,14 @@ class ConstraintSystem { ConstraintKind bodyResultConstraintKind, Type contextualType, ConstraintLocatorBuilder locator); + /// Used by matchResultBuilder() to update resultBuilderTransformed and record + /// a change in the trail. + void recordResultBuilderTransform(AnyFunctionRef fn, + AppliedBuilderTransform transformInfo); + + /// Undo the above change. + void removeResultBuilderTransform(AnyFunctionRef fn); + /// Matches a wrapped or projected value parameter type to its backing /// property wrapper type by applying the property wrapper. TypeMatchResult applyPropertyWrapperToParameter( diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp index 9793d5c1c2dff..c4efc435456d7 100644 --- a/lib/Sema/BuilderTransform.cpp +++ b/lib/Sema/BuilderTransform.cpp @@ -1211,15 +1211,7 @@ ConstraintSystem::matchResultBuilder(AnyFunctionRef fn, Type builderType, transformInfo.transformedBody = transformedBody->second; // Record the transformation. - assert( - std::find_if( - resultBuilderTransformed.begin(), resultBuilderTransformed.end(), - [&](const std::pair &elt) { - return elt.first == fn; - }) == resultBuilderTransformed.end() && - "already transformed this body along this path!?!"); - resultBuilderTransformed.insert( - std::make_pair(fn, std::move(transformInfo))); + recordResultBuilderTransform(fn, std::move(transformInfo)); if (generateConstraints(fn, transformInfo.transformedBody)) return getTypeMatchFailure(locator); @@ -1227,6 +1219,22 @@ ConstraintSystem::matchResultBuilder(AnyFunctionRef fn, Type builderType, return getTypeMatchSuccess(); } +void ConstraintSystem::recordResultBuilderTransform(AnyFunctionRef fn, + AppliedBuilderTransform transformInfo) { + bool inserted = resultBuilderTransformed.insert( + std::make_pair(fn, std::move(transformInfo))).second; + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::recordedResultBuilderTransform(fn)); +} + +/// Undo the above change. +void ConstraintSystem::removeResultBuilderTransform(AnyFunctionRef fn) { + bool erased = resultBuilderTransformed.erase(fn); + ASSERT(erased); +} + namespace { class ReturnStmtFinder : public ASTWalker { std::vector ReturnStmts; diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 1ebea097e8fda..3335c168c8603 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -410,7 +410,8 @@ void ConstraintSystem::applySolution(const Solution &solution) { } for (const auto &transformed : solution.resultBuilderTransformed) { - resultBuilderTransformed.insert(transformed); + if (resultBuilderTransformed.count(transformed.first) == 0) + recordResultBuilderTransform(transformed.first, transformed.second); } for (const auto &appliedWrapper : solution.appliedPropertyWrappers) { @@ -680,7 +681,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); numKeyPaths = cs.KeyPaths.size(); - numResultBuilderTransformed = cs.resultBuilderTransformed.size(); numAppliedPropertyWrappers = cs.appliedPropertyWrappers.size(); numResolvedOverloads = cs.ResolvedOverloads.size(); numInferredClosureTypes = cs.ClosureTypes.size(); @@ -733,9 +733,6 @@ ConstraintSystem::SolverScope::~SolverScope() { /// Remove any key path expressions. truncate(cs.KeyPaths, numKeyPaths); - /// Remove any builder transformed closures. - truncate(cs.resultBuilderTransformed, numResultBuilderTransformed); - // Remove any applied property wrappers. truncate(cs.appliedPropertyWrappers, numAppliedPropertyWrappers); diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index 5152b6bd5251d..1a8131ea4a96c 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -262,6 +262,14 @@ SolverTrail::Change::favoredConstraint(Constraint *constraint) { return result; } +SolverTrail::Change +SolverTrail::Change::recordedResultBuilderTransform(AnyFunctionRef fn) { + Change result; + result.Kind = ChangeKind::RecordedResultBuilderTransform; + result.AFR = fn; + return result; +} + void SolverTrail::Change::undo(ConstraintSystem &cs) const { auto &cg = cs.getConstraintGraph(); @@ -368,6 +376,10 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { if (TheConstraint.Constraint->isFavored()) TheConstraint.Constraint->setFavored(false); break; + + case ChangeKind::RecordedResultBuilderTransform: + cs.removeResultBuilderTransform(AFR); + break; } } @@ -570,6 +582,12 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, indent + 2); out << ")\n"; break; + + case ChangeKind::RecordedResultBuilderTransform: + out << "(recorded result builder transform "; + simple_display(out, AFR); + out << ")\n"; + break; } } From 6862955244cfbecea4a97d751505deadb110b170 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Wed, 2 Oct 2024 23:02:40 -0400 Subject: [PATCH 08/26] Sema: Record applied property wrappers in the trail --- include/swift/Sema/CSTrail.h | 6 ++++++ include/swift/Sema/ConstraintSystem.h | 14 +++++++++----- lib/Sema/CSGen.cpp | 20 ++++++++++++++++++-- lib/Sema/CSSolver.cpp | 13 ++++++++----- lib/Sema/CSTrail.cpp | 18 ++++++++++++++++++ 5 files changed, 59 insertions(+), 12 deletions(-) diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index 4f715a929a847..1ca6b02199174 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -91,6 +91,8 @@ class SolverTrail { FavoredConstraint, /// Recorded a result builder transform. RecordedResultBuilderTransform, + /// Applied a property wrapper. + AppliedPropertyWrapper, }; /// A change made to the constraint system. @@ -168,6 +170,7 @@ class SolverTrail { ConstraintLocator *Locator; PackExpansionType *ExpansionTy; PackElementExpr *ElementExpr; + Expr *TheExpr; AnyFunctionRef AFR; }; @@ -262,6 +265,9 @@ class SolverTrail { /// Create a change that recorded a result builder transform. static Change recordedResultBuilderTransform(AnyFunctionRef fn); + /// Create a change that recorded a result builder transform. + static Change appliedPropertyWrapper(Expr *anchor); + /// Undo this change, reverting the constraint graph to the state it /// had prior to this change. /// diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 65bae1e320fee..642e7c68c0e37 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -1604,7 +1604,7 @@ class Solution { resultBuilderTransformed; /// A map from argument expressions to their applied property wrapper expressions. - llvm::MapVector> appliedPropertyWrappers; + llvm::DenseMap> appliedPropertyWrappers; /// A mapping from the constraint locators for references to various /// names (e.g., member references, normal name references, possible @@ -2403,7 +2403,7 @@ class ConstraintSystem { public: /// A map from argument expressions to their applied property wrapper expressions. - llvm::SmallMapVector, 4> + llvm::SmallDenseMap, 4> appliedPropertyWrappers; /// The locators of \c Defaultable constraints whose defaults were used. @@ -2857,9 +2857,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - /// The length of \c appliedPropertyWrappers - unsigned numAppliedPropertyWrappers; - /// The length of \c ResolvedOverloads. unsigned numResolvedOverloads; @@ -5329,6 +5326,13 @@ class ConstraintSystem { Type wrapperType, Type paramType, ParamDecl *param, Identifier argLabel, ConstraintKind matchKind, ConstraintLocatorBuilder locator); + /// Used by applyPropertyWrapperToParameter() to update appliedPropertyWrappers + /// and record a change in the trail. + void applyPropertyWrapper(Expr *anchor, AppliedPropertyWrapper applied); + + /// Undo the above change. + void removePropertyWrapper(Expr *anchor); + /// Determine whether given type variable with its set of bindings is viable /// to be attempted on the next step of the solver. std::optional determineBestBindings( diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index ddad265d565c2..a44072809aa9a 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -5046,6 +5046,22 @@ bool ConstraintSystem::generateConstraints(StmtCondition condition, return false; } +void ConstraintSystem::applyPropertyWrapper( + Expr *anchor, AppliedPropertyWrapper applied) { + appliedPropertyWrappers[anchor].push_back(applied); + + if (solverState) + recordChange(SolverTrail::Change::appliedPropertyWrapper(anchor)); +} + +void ConstraintSystem::removePropertyWrapper(Expr *anchor) { + auto found = appliedPropertyWrappers.find(anchor); + ASSERT(found != appliedPropertyWrappers.end()); + auto &wrappers = found->second; + ASSERT(!wrappers.empty()); + wrappers.pop_back(); +} + ConstraintSystem::TypeMatchResult ConstraintSystem::applyPropertyWrapperToParameter( Type wrapperType, Type paramType, ParamDecl *param, Identifier argLabel, @@ -5079,13 +5095,13 @@ ConstraintSystem::applyPropertyWrapperToParameter( setType(param->getPropertyWrapperProjectionVar(), projectionType); } - appliedPropertyWrappers[anchor].push_back({ wrapperType, PropertyWrapperInitKind::ProjectedValue }); + applyPropertyWrapper(anchor, { wrapperType, PropertyWrapperInitKind::ProjectedValue }); } else if (param->hasExternalPropertyWrapper()) { Type wrappedValueType = computeWrappedValueType(param, wrapperType); addConstraint(matchKind, paramType, wrappedValueType, locator); setType(param->getPropertyWrapperWrappedValueVar(), wrappedValueType); - appliedPropertyWrappers[anchor].push_back({ wrapperType, PropertyWrapperInitKind::WrappedValue }); + applyPropertyWrapper(anchor, { wrapperType, PropertyWrapperInitKind::WrappedValue }); } else { return getTypeMatchFailure(locator); } diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 3335c168c8603..083525f91ba1a 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -415,7 +415,14 @@ void ConstraintSystem::applySolution(const Solution &solution) { } for (const auto &appliedWrapper : solution.appliedPropertyWrappers) { - appliedPropertyWrappers.insert(appliedWrapper); + auto found = appliedPropertyWrappers.find(appliedWrapper.first); + if (found == appliedPropertyWrappers.end()) { + appliedPropertyWrappers.insert(appliedWrapper); + } else { + auto &existing = found->second; + ASSERT(existing.size() <= appliedWrapper.second.size()); + existing = appliedWrapper.second; + } } for (auto &valueConversion : solution.ImplicitValueConversions) { @@ -681,7 +688,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); numKeyPaths = cs.KeyPaths.size(); - numAppliedPropertyWrappers = cs.appliedPropertyWrappers.size(); numResolvedOverloads = cs.ResolvedOverloads.size(); numInferredClosureTypes = cs.ClosureTypes.size(); numImpliedResults = cs.ImpliedResults.size(); @@ -733,9 +739,6 @@ ConstraintSystem::SolverScope::~SolverScope() { /// Remove any key path expressions. truncate(cs.KeyPaths, numKeyPaths); - // Remove any applied property wrappers. - truncate(cs.appliedPropertyWrappers, numAppliedPropertyWrappers); - // Remove any inferred closure types (e.g. used in result builder body). truncate(cs.ClosureTypes, numInferredClosureTypes); diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index 1a8131ea4a96c..7b2a7f06bf521 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -270,6 +270,14 @@ SolverTrail::Change::recordedResultBuilderTransform(AnyFunctionRef fn) { return result; } +SolverTrail::Change +SolverTrail::Change::appliedPropertyWrapper(Expr *expr) { + Change result; + result.Kind = ChangeKind::AppliedPropertyWrapper; + result.TheExpr = expr; + return result; +} + void SolverTrail::Change::undo(ConstraintSystem &cs) const { auto &cg = cs.getConstraintGraph(); @@ -380,6 +388,10 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { case ChangeKind::RecordedResultBuilderTransform: cs.removeResultBuilderTransform(AFR); break; + + case ChangeKind::AppliedPropertyWrapper: + cs.removePropertyWrapper(TheExpr); + break; } } @@ -588,6 +600,12 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, simple_display(out, AFR); out << ")\n"; break; + + case ChangeKind::AppliedPropertyWrapper: + out << "(applied property wrapper to "; + simple_display(out, TheExpr); + out << ")\n"; + break; } } From 434b02166e8195e5180f62b4223743482ec424d2 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 3 Oct 2024 14:54:58 -0400 Subject: [PATCH 09/26] Sema: LLVM_DEBUG for CSBindings.cpp --- lib/Sema/CSBindings.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 2f9a971ab60ec..a1fa5b84108ba 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -21,9 +21,12 @@ #include "swift/Sema/ConstraintGraph.h" #include "swift/Sema/ConstraintSystem.h" #include "llvm/ADT/SetVector.h" +#include "llvm/Support/Debug.h" #include "llvm/Support/raw_ostream.h" #include +#define DEBUG_TYPE "PotentialBindings" + using namespace swift; using namespace constraints; using namespace inference; @@ -1951,6 +1954,13 @@ void PotentialBindings::retract(Constraint *constraint) { if (CS.isRecordingChanges()) CS.recordChange(SolverTrail::Change::retractedBindings(TypeVar, constraint)); + LLVM_DEBUG( + llvm::dbgs() << Constraints.size() << " " << Bindings.size() << " " + << Protocols.size() << " " << Literals.size() << " " + << AdjacentVars.size() << " " << DelayedBy.size() << " " + << SubtypeOf.size() << " " << SupertypeOf.size() << " " + << EquivalentTo.size() << "\n"); + Bindings.erase( llvm::remove_if(Bindings, [&constraint](const PotentialBinding &binding) { From 9201a375191629d140d4c3264a8d7f62f90e94cc Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 3 Oct 2024 14:55:30 -0400 Subject: [PATCH 10/26] Sema: Strenghten invariants in SolverTrail::Change::undo() --- lib/Sema/CSSolver.cpp | 26 ++++++++++++++------------ lib/Sema/CSTrail.cpp | 7 +++---- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 083525f91ba1a..1db75ecaf8df5 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1782,25 +1782,29 @@ ConstraintSystem::filterDisjunction( llvm::errs().indent(indent) << ")\n"; } - if (restoreOnFail) - constraintsToRestoreOnFail.push_back(constraint); - if (!constraint->isDisabled()) { - if (solverState) + if (restoreOnFail) + constraintsToRestoreOnFail.push_back(constraint); + else if (solverState) solverState->disableConstraint(constraint); else constraint->setDisabled(); } } - switch (numEnabledTerms) { - case 0: + if (numEnabledTerms == 0) + return SolutionKind::Error; + + if (restoreOnFail) { for (auto constraint : constraintsToRestoreOnFail) { - constraint->setEnabled(); + if (solverState) + solverState->disableConstraint(constraint); + else + constraint->setDisabled(); } - return SolutionKind::Error; + } - case 1: { + if (numEnabledTerms == 1) { // Only a single constraint remains. Retire the disjunction and make // the remaining constraint active. auto choice = disjunction->getNestedConstraints()[choiceIdx]; @@ -1849,9 +1853,7 @@ ConstraintSystem::filterDisjunction( return failedConstraint ? SolutionKind::Unsolved : SolutionKind::Solved; } - default: - return SolutionKind::Unsolved; - } + return SolutionKind::Unsolved; } // Attempt to find a disjunction of bind constraints where all options diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index 7b2a7f06bf521..dc31e6c7e0ae3 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -376,13 +376,12 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { break; case ChangeKind::DisabledConstraint: - if (TheConstraint.Constraint->isDisabled()) - TheConstraint.Constraint->setEnabled(); + TheConstraint.Constraint->setEnabled(); break; case ChangeKind::FavoredConstraint: - if (TheConstraint.Constraint->isFavored()) - TheConstraint.Constraint->setFavored(false); + ASSERT(TheConstraint.Constraint->isFavored()); + TheConstraint.Constraint->setFavored(false); break; case ChangeKind::RecordedResultBuilderTransform: From 892e79cd70cb2474b72617c91e4d6f442acf0b87 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 3 Oct 2024 21:26:50 -0400 Subject: [PATCH 11/26] Sema: Use an xmacro to clean up some duplication in CSTrail.cpp --- include/swift/Sema/CSTrail.def | 61 ++++++++ include/swift/Sema/CSTrail.h | 123 ++++------------ include/swift/Sema/ConstraintSystem.h | 12 +- lib/Sema/BuilderTransform.cpp | 2 +- lib/Sema/CSBindings.cpp | 4 +- lib/Sema/CSGen.cpp | 2 +- lib/Sema/CSSimplify.cpp | 2 +- lib/Sema/CSTrail.cpp | 203 ++++++++++---------------- lib/Sema/ConstraintGraph.cpp | 14 +- lib/Sema/ConstraintSystem.cpp | 20 +-- 10 files changed, 198 insertions(+), 245 deletions(-) create mode 100644 include/swift/Sema/CSTrail.def diff --git a/include/swift/Sema/CSTrail.def b/include/swift/Sema/CSTrail.def new file mode 100644 index 0000000000000..ad7c612eded3b --- /dev/null +++ b/include/swift/Sema/CSTrail.def @@ -0,0 +1,61 @@ +//===--- CSTrail.def - Trail Change Kinds ---------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// +/// +/// This file enumerates the kinds of SolverTrail::Change. +/// +//===----------------------------------------------------------------------===// + +#ifndef CHANGE +#define CHANGE(Name) +#endif + +#ifndef LOCATOR_CHANGE +#define LOCATOR_CHANGE(Name) CHANGE(Name) +#endif + +#ifndef LAST_CHANGE +#define LAST_CHANGE(Name) +#endif + +LOCATOR_CHANGE(RecordedAppliedDisjunction) +LOCATOR_CHANGE(RecordedMatchCallArgumentResult) +LOCATOR_CHANGE(RecordedOpenedTypes) +LOCATOR_CHANGE(RecordedOpenedExistentialType) +LOCATOR_CHANGE(RecordedPackExpansionEnvironment) +LOCATOR_CHANGE(RecordedDefaultedConstraint) + +CHANGE(AddedTypeVariable) +CHANGE(AddedConstraint) +CHANGE(RemovedConstraint) +CHANGE(ExtendedEquivalenceClass) +CHANGE(RelatedTypeVariables) +CHANGE(InferredBindings) +CHANGE(RetractedBindings) +CHANGE(UpdatedTypeVariable) +CHANGE(AddedConversionRestriction) +CHANGE(AddedFix) +CHANGE(AddedFixedRequirement) +CHANGE(RecordedDisjunctionChoice) +CHANGE(RecordedOpenedPackExpansionType) +CHANGE(RecordedPackEnvironment) +CHANGE(RecordedNodeType) +CHANGE(RecordedKeyPathComponentType) +CHANGE(DisabledConstraint) +CHANGE(FavoredConstraint) +CHANGE(RecordedResultBuilderTransform) +CHANGE(AppliedPropertyWrapper) + +LAST_CHANGE(AppliedPropertyWrapper) + +#undef LOCATOR_CHANGE +#undef LAST_CHANGE +#undef CHANGE \ No newline at end of file diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index 1ca6b02199174..d345cf8ca2c8a 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -40,59 +40,11 @@ class SolverTrail { /// The kind of change made to the graph. enum class ChangeKind: unsigned { - /// Added a new vertex to the constraint graph. - AddedTypeVariable, - /// Added a new constraint to the constraint graph. - AddedConstraint, - /// Removed an existing constraint from the constraint graph. - RemovedConstraint, - /// Extended the equivalence class of a type variable in the constraint graph. - ExtendedEquivalenceClass, - /// Added a new edge in the constraint graph. - RelatedTypeVariables, - /// Inferred potential bindings from a constraint. - InferredBindings, - /// Retracted potential bindings from a constraint. - RetractedBindings, - /// Set the fixed type or parent and flags for a type variable. - UpdatedTypeVariable, - /// Recorded a conversion restriction kind. - AddedConversionRestriction, - /// Recorded a fix. - AddedFix, - /// Recorded a fixed requirement. - AddedFixedRequirement, - /// Recorded a disjunction choice. - RecordedDisjunctionChoice, - /// Recorded an applied disjunction. - RecordedAppliedDisjunction, - /// Recorded an argument matching choice. - RecordedMatchCallArgumentResult, - /// Recorded a list of opened types at a locator. - RecordedOpenedTypes, - /// Recorded the opening of an existential type at a locator. - RecordedOpenedExistentialType, - /// Recorded the opening of a pack existential type. - RecordedOpenedPackExpansionType, - /// Recorded the creation of a generic environment for a pack expansion expression. - RecordedPackExpansionEnvironment, - /// Recorded the mapping from a pack element expression to its parent - /// pack expansion expression. - RecordedPackEnvironment, - /// Recorded a defaulted constraint at a locator. - RecordedDefaultedConstraint, - /// Recorded an assignment of a type to an AST node. - RecordedNodeType, - /// Recorded an assignment of a type to a keypath component. - RecordedKeyPathComponentType, - /// Disabled a constraint. - DisabledConstraint, - /// Favored a constraint. - FavoredConstraint, - /// Recorded a result builder transform. - RecordedResultBuilderTransform, - /// Applied a property wrapper. - AppliedPropertyWrapper, +#define CHANGE(Name) Name, +#define LAST_CHANGE(Name) Last = Name +#include "CSTrail.def" +#undef CHANGE +#undef LAST_CHANGE }; /// A change made to the constraint system. @@ -176,97 +128,82 @@ class SolverTrail { Change() : Kind(ChangeKind::AddedTypeVariable), TypeVar(nullptr) { } +#define LOCATOR_CHANGE(Name) static Change Name(ConstraintLocator *locator); +#include "swift/Sema/CSTrail.def" + /// Create a change that added a type variable. - static Change addedTypeVariable(TypeVariableType *typeVar); + static Change AddedTypeVariable(TypeVariableType *typeVar); /// Create a change that added a constraint. - static Change addedConstraint(TypeVariableType *typeVar, Constraint *constraint); + static Change AddedConstraint(TypeVariableType *typeVar, Constraint *constraint); /// Create a change that removed a constraint. - static Change removedConstraint(TypeVariableType *typeVar, Constraint *constraint); + static Change RemovedConstraint(TypeVariableType *typeVar, Constraint *constraint); /// Create a change that extended an equivalence class. - static Change extendedEquivalenceClass(TypeVariableType *typeVar, + static Change ExtendedEquivalenceClass(TypeVariableType *typeVar, unsigned prevSize); /// Create a change that updated the references/referenced by sets of /// a type variable pair. - static Change relatedTypeVariables(TypeVariableType *typeVar, + static Change RelatedTypeVariables(TypeVariableType *typeVar, TypeVariableType *otherTypeVar); /// Create a change that inferred bindings from a constraint. - static Change inferredBindings(TypeVariableType *typeVar, + static Change InferredBindings(TypeVariableType *typeVar, Constraint *constraint); /// Create a change that retracted bindings from a constraint. - static Change retractedBindings(TypeVariableType *typeVar, + static Change RetractedBindings(TypeVariableType *typeVar, Constraint *constraint); /// Create a change that updated a type variable. - static Change updatedTypeVariable( + static Change UpdatedTypeVariable( TypeVariableType *typeVar, llvm::PointerUnion parentOrFixed, unsigned options); /// Create a change that recorded a restriction. - static Change addedConversionRestriction(Type srcType, Type dstType); + static Change AddedConversionRestriction(Type srcType, Type dstType); /// Create a change that recorded a fix. - static Change addedFix(ConstraintFix *fix); + static Change AddedFix(ConstraintFix *fix); /// Create a change that recorded a fixed requirement. - static Change addedFixedRequirement(GenericTypeParamType *GP, + static Change AddedFixedRequirement(GenericTypeParamType *GP, unsigned reqKind, Type requirementTy); /// Create a change that recorded a disjunction choice. - static Change recordedDisjunctionChoice(ConstraintLocator *locator, + static Change RecordedDisjunctionChoice(ConstraintLocator *locator, unsigned index); - /// Create a change that recorded an applied disjunction. - static Change recordedAppliedDisjunction(ConstraintLocator *locator); - - /// Create a change that recorded an applied disjunction. - static Change recordedMatchCallArgumentResult(ConstraintLocator *locator); - - /// Create a change that recorded a list of opened types. - static Change recordedOpenedTypes(ConstraintLocator *locator); - - /// Create a change that recorded the opening of an existential type. - static Change recordedOpenedExistentialType(ConstraintLocator *locator); - /// Create a change that recorded the opening of a pack expansion type. - static Change recordedOpenedPackExpansionType(PackExpansionType *expansion); - - /// Create a change that recorded the opening of a pack expansion type. - static Change recordedPackExpansionEnvironment(ConstraintLocator *locator); + static Change RecordedOpenedPackExpansionType(PackExpansionType *expansion); /// Create a change that recorded a mapping from a pack element expression /// to its parent expansion expression. - static Change recordedPackEnvironment(PackElementExpr *packElement); - - /// Create a change that recorded a defaulted constraint at a locator. - static Change recordedDefaultedConstraint(ConstraintLocator *locator); + static Change RecordedPackEnvironment(PackElementExpr *packElement); /// Create a change that recorded an assignment of a type to an AST node. - static Change recordedNodeType(ASTNode node, Type oldType); + static Change RecordedNodeType(ASTNode node, Type oldType); /// Create a change that recorded an assignment of a type to an AST node. - static Change recordedKeyPathComponentType(const KeyPathExpr *expr, + static Change RecordedKeyPathComponentType(const KeyPathExpr *expr, unsigned component, Type oldType); /// Create a change that disabled a constraint. - static Change disabledConstraint(Constraint *constraint); + static Change DisabledConstraint(Constraint *constraint); /// Create a change that favored a constraint. - static Change favoredConstraint(Constraint *constraint); + static Change FavoredConstraint(Constraint *constraint); /// Create a change that recorded a result builder transform. - static Change recordedResultBuilderTransform(AnyFunctionRef fn); + static Change RecordedResultBuilderTransform(AnyFunctionRef fn); /// Create a change that recorded a result builder transform. - static Change appliedPropertyWrapper(Expr *anchor); + static Change AppliedPropertyWrapper(Expr *anchor); /// Undo this change, reverting the constraint graph to the state it /// had prior to this change. @@ -278,7 +215,7 @@ class SolverTrail { unsigned indent = 0) const; }; - SolverTrail(ConstraintSystem &cs) : CS(cs) {} + SolverTrail(ConstraintSystem &cs); ~SolverTrail(); @@ -305,6 +242,8 @@ class SolverTrail { /// The list of changes made to this constraint system. std::vector Changes; + uint64_t Profile[unsigned(ChangeKind::Last) + 1]; + bool UndoActive = false; unsigned Total = 0; unsigned Max = 0; diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 642e7c68c0e37..d039d29e57068 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -435,7 +435,7 @@ class TypeVariableType::Implementation { /// Record the current type-variable binding. void recordBinding(constraints::SolverTrail &trail) { - trail.recordChange(constraints::SolverTrail::Change::updatedTypeVariable( + trail.recordChange(constraints::SolverTrail::Change::UpdatedTypeVariable( getTypeVariable(), ParentOrFixed, getRawOptions())); } @@ -2413,7 +2413,7 @@ class ConstraintSystem { bool inserted = DefaultedConstraints.insert(locator).second; if (inserted) { if (solverState) - recordChange(SolverTrail::Change::recordedDefaultedConstraint(locator)); + recordChange(SolverTrail::Change::RecordedDefaultedConstraint(locator)); } } @@ -2643,7 +2643,7 @@ class ConstraintSystem { void disableConstraint(Constraint *constraint) { ASSERT(!constraint->isDisabled()); constraint->setDisabled(); - Trail.recordChange(SolverTrail::Change::disabledConstraint(constraint)); + Trail.recordChange(SolverTrail::Change::DisabledConstraint(constraint)); } /// Favor the given constraint; this change will be rolled back @@ -2652,7 +2652,7 @@ class ConstraintSystem { assert(!constraint->isFavored()); constraint->setFavored(); - Trail.recordChange(SolverTrail::Change::favoredConstraint(constraint)); + Trail.recordChange(SolverTrail::Change::FavoredConstraint(constraint)); } private: @@ -3172,7 +3172,7 @@ class ConstraintSystem { if (oldType.getPointer() != type.getPointer()) { // Record the fact that we assigned a type to this node. if (solverState) - recordChange(SolverTrail::Change::recordedNodeType(node, oldType)); + recordChange(SolverTrail::Change::RecordedNodeType(node, oldType)); } } @@ -3211,7 +3211,7 @@ class ConstraintSystem { if (oldType.getPointer() != T.getPointer()) { if (solverState) { recordChange( - SolverTrail::Change::recordedKeyPathComponentType( + SolverTrail::Change::RecordedKeyPathComponentType( KP, I, oldType)); } } diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp index c4efc435456d7..658602b4610f4 100644 --- a/lib/Sema/BuilderTransform.cpp +++ b/lib/Sema/BuilderTransform.cpp @@ -1226,7 +1226,7 @@ void ConstraintSystem::recordResultBuilderTransform(AnyFunctionRef fn, ASSERT(inserted); if (solverState) - recordChange(SolverTrail::Change::recordedResultBuilderTransform(fn)); + recordChange(SolverTrail::Change::RecordedResultBuilderTransform(fn)); } /// Undo the above change. diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index a1fa5b84108ba..97075fd87dc20 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -1777,7 +1777,7 @@ void PotentialBindings::infer(Constraint *constraint) { // Record the change, if there are active scopes. if (CS.isRecordingChanges()) - CS.recordChange(SolverTrail::Change::inferredBindings(TypeVar, constraint)); + CS.recordChange(SolverTrail::Change::InferredBindings(TypeVar, constraint)); switch (constraint->getKind()) { case ConstraintKind::Bind: @@ -1952,7 +1952,7 @@ void PotentialBindings::retract(Constraint *constraint) { // Record the change, if there are active scopes. if (CS.isRecordingChanges()) - CS.recordChange(SolverTrail::Change::retractedBindings(TypeVar, constraint)); + CS.recordChange(SolverTrail::Change::RetractedBindings(TypeVar, constraint)); LLVM_DEBUG( llvm::dbgs() << Constraints.size() << " " << Bindings.size() << " " diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index a44072809aa9a..da6c9f1171845 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -5051,7 +5051,7 @@ void ConstraintSystem::applyPropertyWrapper( appliedPropertyWrappers[anchor].push_back(applied); if (solverState) - recordChange(SolverTrail::Change::appliedPropertyWrapper(anchor)); + recordChange(SolverTrail::Change::AppliedPropertyWrapper(anchor)); } void ConstraintSystem::removePropertyWrapper(Expr *anchor) { diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index a41d1273df465..e3aaed5b409b5 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -14930,7 +14930,7 @@ void ConstraintSystem::recordMatchCallArgumentResult( ASSERT(inserted); if (solverState) - recordChange(SolverTrail::Change::recordedMatchCallArgumentResult(locator)); + recordChange(SolverTrail::Change::RecordedMatchCallArgumentResult(locator)); } void ConstraintSystem::recordCallAsFunction(UnresolvedDotExpr *root, diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index dc31e6c7e0ae3..8943b7f965fd4 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -33,6 +33,13 @@ using namespace constraints; #define DEBUG_TYPE "SolverTrail" +SolverTrail::SolverTrail(ConstraintSystem &cs) + : CS(cs) { + for (unsigned i = 0; i <= unsigned(ChangeKind::Last); ++i) { + Profile[i] = 0; + } +} + SolverTrail::~SolverTrail() { // If constraint system is in an invalid state, it's // possible that constraint graph is corrupted as well @@ -41,8 +48,18 @@ SolverTrail::~SolverTrail() { ASSERT(Changes.empty() && "Trail corrupted"); } +#define LOCATOR_CHANGE(Name) \ + SolverTrail::Change \ + SolverTrail::Change::Name(ConstraintLocator *locator) { \ + Change result; \ + result.Kind = ChangeKind::Name; \ + result.Locator = locator; \ + return result; \ + } +#include "swift/Sema/CSTrail.def" + SolverTrail::Change -SolverTrail::Change::addedTypeVariable(TypeVariableType *typeVar) { +SolverTrail::Change::AddedTypeVariable(TypeVariableType *typeVar) { Change result; result.Kind = ChangeKind::AddedTypeVariable; result.TypeVar = typeVar; @@ -50,7 +67,7 @@ SolverTrail::Change::addedTypeVariable(TypeVariableType *typeVar) { } SolverTrail::Change -SolverTrail::Change::addedConstraint(TypeVariableType *typeVar, +SolverTrail::Change::AddedConstraint(TypeVariableType *typeVar, Constraint *constraint) { Change result; result.Kind = ChangeKind::AddedConstraint; @@ -60,7 +77,7 @@ SolverTrail::Change::addedConstraint(TypeVariableType *typeVar, } SolverTrail::Change -SolverTrail::Change::removedConstraint(TypeVariableType *typeVar, +SolverTrail::Change::RemovedConstraint(TypeVariableType *typeVar, Constraint *constraint) { Change result; result.Kind = ChangeKind::RemovedConstraint; @@ -70,7 +87,7 @@ SolverTrail::Change::removedConstraint(TypeVariableType *typeVar, } SolverTrail::Change -SolverTrail::Change::extendedEquivalenceClass(TypeVariableType *typeVar, +SolverTrail::Change::ExtendedEquivalenceClass(TypeVariableType *typeVar, unsigned prevSize) { Change result; result.Kind = ChangeKind::ExtendedEquivalenceClass; @@ -80,7 +97,7 @@ SolverTrail::Change::extendedEquivalenceClass(TypeVariableType *typeVar, } SolverTrail::Change -SolverTrail::Change::relatedTypeVariables(TypeVariableType *typeVar, +SolverTrail::Change::RelatedTypeVariables(TypeVariableType *typeVar, TypeVariableType *otherTypeVar) { Change result; result.Kind = ChangeKind::RelatedTypeVariables; @@ -90,7 +107,7 @@ SolverTrail::Change::relatedTypeVariables(TypeVariableType *typeVar, } SolverTrail::Change -SolverTrail::Change::inferredBindings(TypeVariableType *typeVar, +SolverTrail::Change::InferredBindings(TypeVariableType *typeVar, Constraint *constraint) { Change result; result.Kind = ChangeKind::InferredBindings; @@ -100,7 +117,7 @@ SolverTrail::Change::inferredBindings(TypeVariableType *typeVar, } SolverTrail::Change -SolverTrail::Change::retractedBindings(TypeVariableType *typeVar, +SolverTrail::Change::RetractedBindings(TypeVariableType *typeVar, Constraint *constraint) { Change result; result.Kind = ChangeKind::RetractedBindings; @@ -110,7 +127,7 @@ SolverTrail::Change::retractedBindings(TypeVariableType *typeVar, } SolverTrail::Change -SolverTrail::Change::updatedTypeVariable( +SolverTrail::Change::UpdatedTypeVariable( TypeVariableType *typeVar, llvm::PointerUnion parentOrFixed, unsigned options) { @@ -123,7 +140,7 @@ SolverTrail::Change::updatedTypeVariable( } SolverTrail::Change -SolverTrail::Change::addedConversionRestriction(Type srcType, Type dstType) { +SolverTrail::Change::AddedConversionRestriction(Type srcType, Type dstType) { Change result; result.Kind = ChangeKind::AddedConversionRestriction; result.Restriction.SrcType = srcType; @@ -132,7 +149,7 @@ SolverTrail::Change::addedConversionRestriction(Type srcType, Type dstType) { } SolverTrail::Change -SolverTrail::Change::addedFix(ConstraintFix *fix) { +SolverTrail::Change::AddedFix(ConstraintFix *fix) { Change result; result.Kind = ChangeKind::AddedFix; result.Fix = fix; @@ -140,7 +157,7 @@ SolverTrail::Change::addedFix(ConstraintFix *fix) { } SolverTrail::Change -SolverTrail::Change::addedFixedRequirement(GenericTypeParamType *GP, +SolverTrail::Change::AddedFixedRequirement(GenericTypeParamType *GP, unsigned reqKind, Type reqTy) { Change result; @@ -152,7 +169,7 @@ SolverTrail::Change::addedFixedRequirement(GenericTypeParamType *GP, } SolverTrail::Change -SolverTrail::Change::recordedDisjunctionChoice(ConstraintLocator *locator, +SolverTrail::Change::RecordedDisjunctionChoice(ConstraintLocator *locator, unsigned index) { Change result; result.Kind = ChangeKind::RecordedDisjunctionChoice; @@ -162,39 +179,7 @@ SolverTrail::Change::recordedDisjunctionChoice(ConstraintLocator *locator, } SolverTrail::Change -SolverTrail::Change::recordedAppliedDisjunction(ConstraintLocator *locator) { - Change result; - result.Kind = ChangeKind::RecordedAppliedDisjunction; - result.Locator = locator; - return result; -} - -SolverTrail::Change -SolverTrail::Change::recordedMatchCallArgumentResult(ConstraintLocator *locator) { - Change result; - result.Kind = ChangeKind::RecordedMatchCallArgumentResult; - result.Locator = locator; - return result; -} - -SolverTrail::Change -SolverTrail::Change::recordedOpenedTypes(ConstraintLocator *locator) { - Change result; - result.Kind = ChangeKind::RecordedOpenedTypes; - result.Locator = locator; - return result; -} - -SolverTrail::Change -SolverTrail::Change::recordedOpenedExistentialType(ConstraintLocator *locator) { - Change result; - result.Kind = ChangeKind::RecordedOpenedExistentialType; - result.Locator = locator; - return result; -} - -SolverTrail::Change -SolverTrail::Change::recordedOpenedPackExpansionType(PackExpansionType *expansionTy) { +SolverTrail::Change::RecordedOpenedPackExpansionType(PackExpansionType *expansionTy) { Change result; result.Kind = ChangeKind::RecordedOpenedPackExpansionType; result.ExpansionTy = expansionTy; @@ -202,15 +187,7 @@ SolverTrail::Change::recordedOpenedPackExpansionType(PackExpansionType *expansio } SolverTrail::Change -SolverTrail::Change::recordedPackExpansionEnvironment(ConstraintLocator *locator) { - Change result; - result.Kind = ChangeKind::RecordedPackExpansionEnvironment; - result.Locator = locator; - return result; -} - -SolverTrail::Change -SolverTrail::Change::recordedPackEnvironment(PackElementExpr *packElement) { +SolverTrail::Change::RecordedPackEnvironment(PackElementExpr *packElement) { Change result; result.Kind = ChangeKind::RecordedPackEnvironment; result.ElementExpr = packElement; @@ -218,15 +195,7 @@ SolverTrail::Change::recordedPackEnvironment(PackElementExpr *packElement) { } SolverTrail::Change -SolverTrail::Change::recordedDefaultedConstraint(ConstraintLocator *locator) { - Change result; - result.Kind = ChangeKind::RecordedDefaultedConstraint; - result.Locator = locator; - return result; -} - -SolverTrail::Change -SolverTrail::Change::recordedNodeType(ASTNode node, Type oldType) { +SolverTrail::Change::RecordedNodeType(ASTNode node, Type oldType) { Change result; result.Kind = ChangeKind::RecordedNodeType; result.Node.Node = node; @@ -235,7 +204,7 @@ SolverTrail::Change::recordedNodeType(ASTNode node, Type oldType) { } SolverTrail::Change -SolverTrail::Change::recordedKeyPathComponentType(const KeyPathExpr *expr, +SolverTrail::Change::RecordedKeyPathComponentType(const KeyPathExpr *expr, unsigned component, Type oldType) { Change result; @@ -247,7 +216,7 @@ SolverTrail::Change::recordedKeyPathComponentType(const KeyPathExpr *expr, } SolverTrail::Change -SolverTrail::Change::disabledConstraint(Constraint *constraint) { +SolverTrail::Change::DisabledConstraint(Constraint *constraint) { Change result; result.Kind = ChangeKind::DisabledConstraint; result.TheConstraint.Constraint = constraint; @@ -255,7 +224,7 @@ SolverTrail::Change::disabledConstraint(Constraint *constraint) { } SolverTrail::Change -SolverTrail::Change::favoredConstraint(Constraint *constraint) { +SolverTrail::Change::FavoredConstraint(Constraint *constraint) { Change result; result.Kind = ChangeKind::FavoredConstraint; result.TheConstraint.Constraint = constraint; @@ -263,7 +232,7 @@ SolverTrail::Change::favoredConstraint(Constraint *constraint) { } SolverTrail::Change -SolverTrail::Change::recordedResultBuilderTransform(AnyFunctionRef fn) { +SolverTrail::Change::RecordedResultBuilderTransform(AnyFunctionRef fn) { Change result; result.Kind = ChangeKind::RecordedResultBuilderTransform; result.AFR = fn; @@ -271,7 +240,7 @@ SolverTrail::Change::recordedResultBuilderTransform(AnyFunctionRef fn) { } SolverTrail::Change -SolverTrail::Change::appliedPropertyWrapper(Expr *expr) { +SolverTrail::Change::AppliedPropertyWrapper(Expr *expr) { Change result; result.Kind = ChangeKind::AppliedPropertyWrapper; result.TheExpr = expr; @@ -403,14 +372,23 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, out.indent(indent); switch (Kind) { + +#define LOCATOR_CHANGE(Name) \ + case ChangeKind::Name: \ + out << "(" << #Name << " at "; \ + Locator->dump(&cs.getASTContext().SourceMgr, out); \ + out << ")\n"; \ + break; +#include "swift/Sema/CSTrail.def" + case ChangeKind::AddedTypeVariable: - out << "(added type variable "; + out << "(AddedTypeVariable "; TypeVar->print(out, PO); out << ")\n"; break; case ChangeKind::AddedConstraint: - out << "(added constraint "; + out << "(AddedConstraint "; TheConstraint.Constraint->print(out, &cs.getASTContext().SourceMgr, indent + 2); out << " to type variable "; @@ -419,7 +397,7 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, break; case ChangeKind::RemovedConstraint: - out << "(removed constraint "; + out << "(RemovedConstraint "; TheConstraint.Constraint->print(out, &cs.getASTContext().SourceMgr, indent + 2); out << " from type variable "; @@ -428,14 +406,14 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, break; case ChangeKind::ExtendedEquivalenceClass: { - out << "(equivalence "; + out << "(ExtendedEquivalenceClass "; EquivClass.TypeVar->print(out, PO); out << " " << EquivClass.PrevSize << ")\n"; break; } case ChangeKind::RelatedTypeVariables: - out << "(related type variable "; + out << "(RelatedTypeVariables "; Relation.TypeVar->print(out, PO); out << " with "; Relation.OtherTypeVar->print(out, PO); @@ -443,7 +421,7 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, break; case ChangeKind::InferredBindings: - out << "(inferred bindings from "; + out << "(InferredBindings from "; TheConstraint.Constraint->print(out, &cs.getASTContext().SourceMgr, indent + 2); out << " for type variable "; @@ -452,7 +430,7 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, break; case ChangeKind::RetractedBindings: - out << "(retracted bindings from "; + out << "(RetractedBindings from "; TheConstraint.Constraint->print(out, &cs.getASTContext().SourceMgr, indent + 2); out << " for type variable "; @@ -461,7 +439,7 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, break; case ChangeKind::UpdatedTypeVariable: { - out << "(updated type variable "; + out << "(UpdatedTypeVariable "; Update.TypeVar->print(out, PO); auto parentOrFixed = Update.TypeVar->getImpl().ParentOrFixed; @@ -480,7 +458,7 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, } case ChangeKind::AddedConversionRestriction: - out << "(added restriction with source "; + out << "(AddedConversionRestriction with source "; Restriction.SrcType->print(out, PO); out << " and destination "; Restriction.DstType->print(out, PO); @@ -488,13 +466,13 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, break; case ChangeKind::AddedFix: - out << "(added a fix "; + out << "(AddedFix "; Fix->print(out); out << ")\n"; break; case ChangeKind::AddedFixedRequirement: - out << "(added a fixed requirement "; + out << "(AddedFixedRequirement "; FixedRequirement.GP->print(out, PO); out << " kind "; out << Options << " "; @@ -503,63 +481,27 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, break; case ChangeKind::RecordedDisjunctionChoice: - out << "(recorded disjunction choice at "; + out << "(RecordedDisjunctionChoice at "; Locator->dump(&cs.getASTContext().SourceMgr, out); out << " index "; out << Options << ")\n"; break; - case ChangeKind::RecordedAppliedDisjunction: - out << "(recorded applied disjunction at "; - Locator->dump(&cs.getASTContext().SourceMgr, out); - out << ")\n"; - break; - - case ChangeKind::RecordedMatchCallArgumentResult: - out << "(recorded argument matching choice at "; - Locator->dump(&cs.getASTContext().SourceMgr, out); - out << ")\n"; - break; - - case ChangeKind::RecordedOpenedTypes: - out << "(recorded list of opened types at "; - Locator->dump(&cs.getASTContext().SourceMgr, out); - out << ")\n"; - break; - - case ChangeKind::RecordedOpenedExistentialType: - out << "(recorded opened existential type at "; - Locator->dump(&cs.getASTContext().SourceMgr, out); - out << ")\n"; - break; - case ChangeKind::RecordedOpenedPackExpansionType: - out << "(recorded opened pack expansion type for "; + out << "(RecordedOpenedPackExpansionType for "; ExpansionTy->print(out, PO); out << ")\n"; break; - case ChangeKind::RecordedPackExpansionEnvironment: - out << "(recorded pack expansion environment at "; - Locator->dump(&cs.getASTContext().SourceMgr, out); - out << ")\n"; - break; - case ChangeKind::RecordedPackEnvironment: // FIXME: Print short form of PackExpansionExpr - out << "(recorded pack environment"; + out << "(RecordedPackEnvironment "; simple_display(out, ElementExpr); out << "\n"; break; - case ChangeKind::RecordedDefaultedConstraint: - out << "(recorded defaulted constraint at "; - Locator->dump(&cs.getASTContext().SourceMgr, out); - out << ")\n"; - break; - case ChangeKind::RecordedNodeType: - out << "(recorded node type at "; + out << "(RecordedNodeType at "; Node.Node.getStartLoc().print(out, cs.getASTContext().SourceMgr); out << " previous "; if (Node.OldType) @@ -570,7 +512,7 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, break; case ChangeKind::RecordedKeyPathComponentType: - out << "(recorded key path "; + out << "(RecordedKeyPathComponentType "; simple_display(out, KeyPath.Expr); out << " with component type "; if (Node.OldType) @@ -581,27 +523,27 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, break; case ChangeKind::DisabledConstraint: - out << "(disabled constraint "; + out << "(DisabledConstraint "; TheConstraint.Constraint->print(out, &cs.getASTContext().SourceMgr, indent + 2); out << ")\n"; break; case ChangeKind::FavoredConstraint: - out << "(favored constraint "; + out << "(FavoredConstraint "; TheConstraint.Constraint->print(out, &cs.getASTContext().SourceMgr, indent + 2); out << ")\n"; break; case ChangeKind::RecordedResultBuilderTransform: - out << "(recorded result builder transform "; + out << "(RecordedResultBuilderTransform "; simple_display(out, AFR); out << ")\n"; break; case ChangeKind::AppliedPropertyWrapper: - out << "(applied property wrapper to "; + out << "(AppliedPropertyWrapper "; simple_display(out, TheExpr); out << ")\n"; break; @@ -614,6 +556,7 @@ void SolverTrail::recordChange(Change change) { Changes.push_back(change); + ++Profile[unsigned(change.Kind)]; ++Total; if (Changes.size() > Max) Max = Changes.size(); @@ -625,9 +568,19 @@ void SolverTrail::undo(unsigned toIndex) { if (CS.inInvalidState()) return; + auto dumpHistogram = [&]() { +#define CHANGE(Name) \ + if (auto count = Profile[unsigned(ChangeKind::Name)]) \ + llvm::dbgs() << "* " << #Name << ": " << count << "\n"; +#include "swift/Sema/CSTrail.def" + }; + LLVM_DEBUG(llvm::dbgs() << "decisions " << Changes.size() << " max " << Max - << " total " << Total << "\n"); + << " total " << Total << "\n"; + dumpHistogram(); + llvm::dbgs() << "\n"); + ASSERT(Changes.size() >= toIndex && "Trail corrupted"); ASSERT(!UndoActive); UndoActive = true; diff --git a/lib/Sema/ConstraintGraph.cpp b/lib/Sema/ConstraintGraph.cpp index f6d3f14d28ad3..096a99541515e 100644 --- a/lib/Sema/ConstraintGraph.cpp +++ b/lib/Sema/ConstraintGraph.cpp @@ -84,7 +84,7 @@ ConstraintGraph::lookupNode(TypeVariableType *typeVar) { // recordChange() to assert if there's an active undo. It is not valid to // create new nodes during an undo. if (CS.solverState) - CS.recordChange(SolverTrail::Change::addedTypeVariable(typeVar)); + CS.recordChange(SolverTrail::Change::AddedTypeVariable(typeVar)); // If this type variable is not the representative of its equivalence class, // add it to its representative's set of equivalences. @@ -400,7 +400,7 @@ void ConstraintGraph::addConstraint(Constraint *constraint) { for (auto typeVar : referencedTypeVars) { // Record the change, if there are active scopes. if (CS.isRecordingChanges()) - CS.recordChange(SolverTrail::Change::addedConstraint(typeVar, constraint)); + CS.recordChange(SolverTrail::Change::AddedConstraint(typeVar, constraint)); addConstraint(typeVar, constraint); @@ -420,7 +420,7 @@ void ConstraintGraph::addConstraint(Constraint *constraint) { if (referencedTypeVars.empty()) { // Record the change, if there are active scopes. if (CS.isRecordingChanges()) - CS.recordChange(SolverTrail::Change::addedConstraint(nullptr, constraint)); + CS.recordChange(SolverTrail::Change::AddedConstraint(nullptr, constraint)); addConstraint(nullptr, constraint); } @@ -455,7 +455,7 @@ void ConstraintGraph::removeConstraint(Constraint *constraint) { // Record the change, if there are active scopes. if (CS.isRecordingChanges()) - CS.recordChange(SolverTrail::Change::removedConstraint(typeVar, constraint)); + CS.recordChange(SolverTrail::Change::RemovedConstraint(typeVar, constraint)); removeConstraint(typeVar, constraint); } @@ -464,7 +464,7 @@ void ConstraintGraph::removeConstraint(Constraint *constraint) { if (referencedTypeVars.empty()) { // Record the change, if there are active scopes. if (CS.isRecordingChanges()) - CS.recordChange(SolverTrail::Change::removedConstraint(nullptr, constraint)); + CS.recordChange(SolverTrail::Change::RemovedConstraint(nullptr, constraint)); removeConstraint(nullptr, constraint); } @@ -503,7 +503,7 @@ void ConstraintGraph::mergeNodes(TypeVariableType *typeVar1, // Record the change, if there are active scopes. if (CS.isRecordingChanges()) { CS.recordChange( - SolverTrail::Change::extendedEquivalenceClass( + SolverTrail::Change::ExtendedEquivalenceClass( typeVarRep, repNode.getEquivalenceClass().size())); } @@ -533,7 +533,7 @@ void ConstraintGraph::bindTypeVariable(TypeVariableType *typeVar, Type fixed) { // Record the change, if there are active scopes. if (CS.isRecordingChanges()) - CS.recordChange(SolverTrail::Change::relatedTypeVariables(typeVar, otherTypeVar)); + CS.recordChange(SolverTrail::Change::RelatedTypeVariables(typeVar, otherTypeVar)); } } diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 5406471e24b45..7a605cd586380 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -267,7 +267,7 @@ void ConstraintSystem::addConversionRestriction( return; if (solverState) { - recordChange(SolverTrail::Change::addedConversionRestriction( + recordChange(SolverTrail::Change::AddedConversionRestriction( srcType, dstType)); } } @@ -284,7 +284,7 @@ void ConstraintSystem::addFix(ConstraintFix *fix) { ASSERT(inserted); if (solverState) - recordChange(SolverTrail::Change::addedFix(fix)); + recordChange(SolverTrail::Change::AddedFix(fix)); } void ConstraintSystem::removeFix(ConstraintFix *fix) { @@ -298,7 +298,7 @@ void ConstraintSystem::recordDisjunctionChoice( ASSERT(inserted); if (solverState) - recordChange(SolverTrail::Change::recordedDisjunctionChoice(locator, index)); + recordChange(SolverTrail::Change::RecordedDisjunctionChoice(locator, index)); } void ConstraintSystem::recordAppliedDisjunction( @@ -309,7 +309,7 @@ void ConstraintSystem::recordAppliedDisjunction( ASSERT(inserted); if (solverState) - recordChange(SolverTrail::Change::recordedAppliedDisjunction(locator)); + recordChange(SolverTrail::Change::RecordedAppliedDisjunction(locator)); } /// Retrieve a dynamic result signature for the given declaration. @@ -846,7 +846,7 @@ void ConstraintSystem::recordOpenedExistentialType( ASSERT(inserted); if (solverState) - recordChange(SolverTrail::Change::recordedOpenedExistentialType(locator)); + recordChange(SolverTrail::Change::RecordedOpenedExistentialType(locator)); } GenericEnvironment * @@ -887,7 +887,7 @@ void ConstraintSystem::recordPackExpansionEnvironment( ASSERT(inserted); if (solverState) - recordChange(SolverTrail::Change::recordedPackExpansionEnvironment(locator)); + recordChange(SolverTrail::Change::RecordedPackExpansionEnvironment(locator)); } PackExpansionExpr * @@ -902,7 +902,7 @@ void ConstraintSystem::addPackEnvironment(PackElementExpr *packElement, ASSERT(inserted); if (solverState) - recordChange(SolverTrail::Change::recordedPackEnvironment(packElement)); + recordChange(SolverTrail::Change::RecordedPackEnvironment(packElement)); } /// Extend the given depth map by adding depths for all of the subexpressions @@ -1269,7 +1269,7 @@ void ConstraintSystem::recordOpenedPackExpansionType(PackExpansionType *expansio ASSERT(inserted); if (solverState) - recordChange(SolverTrail::Change::recordedOpenedPackExpansionType(expansion)); + recordChange(SolverTrail::Change::RecordedOpenedPackExpansionType(expansion)); } Type ConstraintSystem::openOpaqueType(OpaqueTypeArchetypeType *opaque, @@ -1678,7 +1678,7 @@ void ConstraintSystem::recordOpenedType( ASSERT(inserted); if (solverState) - recordChange(SolverTrail::Change::recordedOpenedTypes(locator)); + recordChange(SolverTrail::Change::RecordedOpenedTypes(locator)); } void ConstraintSystem::removeOpenedType(ConstraintLocator *locator) { @@ -7436,7 +7436,7 @@ void ConstraintSystem::recordFixedRequirement(GenericTypeParamType *GP, std::make_tuple(GP, reqKind, requirementTy.getPointer())).second; if (inserted) { if (solverState) { - recordChange(SolverTrail::Change::addedFixedRequirement( + recordChange(SolverTrail::Change::AddedFixedRequirement( GP, reqKind, requirementTy)); } } From 43a4ac9216419f33600f7cef7e69b13f03c9afc4 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 3 Oct 2024 22:11:16 -0400 Subject: [PATCH 12/26] Sema: Record resolved overloads in the trail --- include/swift/Sema/CSTrail.def | 1 + include/swift/Sema/ConstraintSystem.h | 12 +++++++----- lib/Sema/CSSolver.cpp | 9 ++++----- lib/Sema/CSTrail.cpp | 4 ++++ lib/Sema/ConstraintSystem.cpp | 18 +++++++++++++++--- 5 files changed, 31 insertions(+), 13 deletions(-) diff --git a/include/swift/Sema/CSTrail.def b/include/swift/Sema/CSTrail.def index ad7c612eded3b..9109f0e0c762b 100644 --- a/include/swift/Sema/CSTrail.def +++ b/include/swift/Sema/CSTrail.def @@ -32,6 +32,7 @@ LOCATOR_CHANGE(RecordedOpenedTypes) LOCATOR_CHANGE(RecordedOpenedExistentialType) LOCATOR_CHANGE(RecordedPackExpansionEnvironment) LOCATOR_CHANGE(RecordedDefaultedConstraint) +LOCATOR_CHANGE(ResolvedOverload) CHANGE(AddedTypeVariable) CHANGE(AddedConstraint) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index d039d29e57068..e05e03fdb0848 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2207,7 +2207,7 @@ class ConstraintSystem { llvm::FoldingSetVector ConstraintLocators; /// The overload sets that have been resolved along the current path. - llvm::MapVector ResolvedOverloads; + llvm::DenseMap ResolvedOverloads; /// The current fixed score for this constraint system and the (partial) /// solution it represents. @@ -2857,9 +2857,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - /// The length of \c ResolvedOverloads. - unsigned numResolvedOverloads; - /// The length of \c ClosureTypes. unsigned numInferredClosureTypes; @@ -4932,6 +4929,11 @@ class ConstraintSystem { buildDisjunctionForOptionalVsUnderlying(boundTy, type, dynamicLocator); } + void recordResolvedOverload(ConstraintLocator *locator, + SelectedOverload choice); + + void removeResolvedOverload(ConstraintLocator *locator); + /// Resolve the given overload set to the given choice. void resolveOverload(ConstraintLocator *locator, Type boundType, OverloadChoice choice, DeclContext *useDC); @@ -5689,7 +5691,7 @@ class ConstraintSystem { } /// The overload sets that have already been resolved along the current path. - const llvm::MapVector & + const llvm::DenseMap & getResolvedOverloads() const { return ResolvedOverloads; } diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 1db75ecaf8df5..4cdf8f6a0a5be 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -290,8 +290,10 @@ void ConstraintSystem::applySolution(const Solution &solution) { // Register overload choices. // FIXME: Copy these directly into some kind of partial solution? - for (auto overload : solution.overloadChoices) - ResolvedOverloads.insert(overload); + for (auto overload : solution.overloadChoices) { + if (!ResolvedOverloads.count(overload.first)) + recordResolvedOverload(overload.first, overload.second); + } // Register constraint restrictions. // FIXME: Copy these directly into some kind of partial solution? @@ -688,7 +690,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); numKeyPaths = cs.KeyPaths.size(); - numResolvedOverloads = cs.ResolvedOverloads.size(); numInferredClosureTypes = cs.ClosureTypes.size(); numImpliedResults = cs.ImpliedResults.size(); numContextualTypes = cs.contextualTypes.size(); @@ -717,8 +718,6 @@ ConstraintSystem::SolverScope::~SolverScope() { // Erase the end of various lists. truncate(cs.TypeVariables, numTypeVariables); - truncate(cs.ResolvedOverloads, numResolvedOverloads); - // Move any remaining active constraints into the inactive list. if (!cs.ActiveConstraints.empty()) { for (auto &constraint : cs.ActiveConstraints) { diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index 8943b7f965fd4..083d9e4409041 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -360,6 +360,10 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { case ChangeKind::AppliedPropertyWrapper: cs.removePropertyWrapper(TheExpr); break; + + case ChangeKind::ResolvedOverload: + cs.removeResolvedOverload(Locator); + break; } } diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 7a605cd586380..8d84c52172f78 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -3735,6 +3735,20 @@ void ConstraintSystem::bindOverloadType( llvm_unreachable("Unhandled OverloadChoiceKind in switch."); } +void ConstraintSystem::recordResolvedOverload(ConstraintLocator *locator, + SelectedOverload overload) { + bool inserted = ResolvedOverloads.insert({locator, overload}).second; + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::ResolvedOverload(locator)); +} + +void ConstraintSystem::removeResolvedOverload(ConstraintLocator *locator) { + bool erased = ResolvedOverloads.erase(locator); + ASSERT(erased); +} + void ConstraintSystem::resolveOverload(ConstraintLocator *locator, Type boundType, OverloadChoice choice, @@ -3990,9 +4004,7 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator, auto overload = SelectedOverload{ choice, openedType, adjustedOpenedType, refType, adjustedRefType, boundType}; - auto result = ResolvedOverloads.insert({locator, overload}); - assert(result.second && "Already resolved this overload?"); - (void)result; + recordResolvedOverload(locator, overload); // Add the constraints necessary to bind the overload type. bindOverloadType(overload, boundType, locator, useDC, From 1d18cd07cf34c5d5db63a88d604e7d2cc9ee59c0 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 3 Oct 2024 22:52:38 -0400 Subject: [PATCH 13/26] Sema: Record closure types in the trail --- include/swift/Sema/CSTrail.def | 3 ++- include/swift/Sema/CSTrail.h | 4 ++++ include/swift/Sema/ConstraintSystem.h | 22 +++++++++++++++------- lib/Sema/CSSolver.cpp | 4 ---- lib/Sema/CSTrail.cpp | 18 ++++++++++++++++++ 5 files changed, 39 insertions(+), 12 deletions(-) diff --git a/include/swift/Sema/CSTrail.def b/include/swift/Sema/CSTrail.def index 9109f0e0c762b..7b31559d21bbc 100644 --- a/include/swift/Sema/CSTrail.def +++ b/include/swift/Sema/CSTrail.def @@ -54,8 +54,9 @@ CHANGE(DisabledConstraint) CHANGE(FavoredConstraint) CHANGE(RecordedResultBuilderTransform) CHANGE(AppliedPropertyWrapper) +CHANGE(RecordedClosureType) -LAST_CHANGE(AppliedPropertyWrapper) +LAST_CHANGE(RecordedClosureType) #undef LOCATOR_CHANGE #undef LAST_CHANGE diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index d345cf8ca2c8a..0b8e1d17fdde6 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -124,6 +124,7 @@ class SolverTrail { PackElementExpr *ElementExpr; Expr *TheExpr; AnyFunctionRef AFR; + const ClosureExpr *Closure; }; Change() : Kind(ChangeKind::AddedTypeVariable), TypeVar(nullptr) { } @@ -205,6 +206,9 @@ class SolverTrail { /// Create a change that recorded a result builder transform. static Change AppliedPropertyWrapper(Expr *anchor); + /// Create a change that recorded a closure type. + static Change RecordedClosureType(const ClosureExpr *closure); + /// Undo this change, reverting the constraint graph to the state it /// had prior to this change. /// diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index e05e03fdb0848..1f026f98cd83b 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2221,6 +2221,9 @@ class ConstraintSystem { /// Maps discovered closures to their types inferred /// from declared parameters/result and body. + /// + /// This is a MapVector because contractEdges() iterates over it and + /// may depend on order. llvm::MapVector ClosureTypes; /// Maps closures and local functions to the pack expansion expressions they @@ -2857,9 +2860,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - /// The length of \c ClosureTypes. - unsigned numInferredClosureTypes; - /// The length of \c ImpliedResults. unsigned numImpliedResults; @@ -3083,10 +3083,18 @@ class ConstraintSystem { } void setClosureType(const ClosureExpr *closure, FunctionType *type) { - assert(closure); - assert(type && "Expected non-null type"); - assert(ClosureTypes.count(closure) == 0 && "Cannot reset closure type"); - ClosureTypes.insert({closure, type}); + ASSERT(closure); + ASSERT(type); + bool inserted = ClosureTypes.insert({closure, type}).second; + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::RecordedClosureType(closure)); + } + + void removeClosureType(const ClosureExpr *closure) { + bool erased = ClosureTypes.erase(closure); + ASSERT(erased); } FunctionType *getClosureType(const ClosureExpr *closure) const { diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 4cdf8f6a0a5be..57a19cc2012d8 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -690,7 +690,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); numKeyPaths = cs.KeyPaths.size(); - numInferredClosureTypes = cs.ClosureTypes.size(); numImpliedResults = cs.ImpliedResults.size(); numContextualTypes = cs.contextualTypes.size(); numTargets = cs.targets.size(); @@ -738,9 +737,6 @@ ConstraintSystem::SolverScope::~SolverScope() { /// Remove any key path expressions. truncate(cs.KeyPaths, numKeyPaths); - // Remove any inferred closure types (e.g. used in result builder body). - truncate(cs.ClosureTypes, numInferredClosureTypes); - // Remove any implied results. truncate(cs.ImpliedResults, numImpliedResults); diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index 083d9e4409041..9062912a9c04c 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -247,6 +247,14 @@ SolverTrail::Change::AppliedPropertyWrapper(Expr *expr) { return result; } +SolverTrail::Change +SolverTrail::Change::RecordedClosureType(const ClosureExpr *closure) { + Change result; + result.Kind = ChangeKind::RecordedClosureType; + result.Closure = closure; + return result; +} + void SolverTrail::Change::undo(ConstraintSystem &cs) const { auto &cg = cs.getConstraintGraph(); @@ -364,6 +372,10 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { case ChangeKind::ResolvedOverload: cs.removeResolvedOverload(Locator); break; + + case ChangeKind::RecordedClosureType: + cs.removeClosureType(Closure); + break; } } @@ -551,6 +563,12 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, simple_display(out, TheExpr); out << ")\n"; break; + + case ChangeKind::RecordedClosureType: + out << "(RecordedClosureType "; + simple_display(out, Closure); + out << ")\n"; + break; } } From 666361adf266a98fc6e3678a1b0cb97d54d50f17 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 3 Oct 2024 23:05:27 -0400 Subject: [PATCH 14/26] Sema: Record implied results in the trail --- include/swift/Sema/CSTrail.def | 9 +++++++- include/swift/Sema/CSTrail.h | 4 +--- include/swift/Sema/ConstraintSystem.h | 25 ++++++++++++--------- lib/Sema/CSSolver.cpp | 10 ++++----- lib/Sema/CSTrail.cpp | 32 +++++++++++++++------------ 5 files changed, 46 insertions(+), 34 deletions(-) diff --git a/include/swift/Sema/CSTrail.def b/include/swift/Sema/CSTrail.def index 7b31559d21bbc..a9f2a971bee67 100644 --- a/include/swift/Sema/CSTrail.def +++ b/include/swift/Sema/CSTrail.def @@ -22,6 +22,10 @@ #define LOCATOR_CHANGE(Name) CHANGE(Name) #endif +#ifndef EXPR_CHANGE +#define EXPR_CHANGE(Name) CHANGE(Name) +#endif + #ifndef LAST_CHANGE #define LAST_CHANGE(Name) #endif @@ -34,6 +38,9 @@ LOCATOR_CHANGE(RecordedPackExpansionEnvironment) LOCATOR_CHANGE(RecordedDefaultedConstraint) LOCATOR_CHANGE(ResolvedOverload) +EXPR_CHANGE(AppliedPropertyWrapper) +EXPR_CHANGE(RecordedImpliedResult) + CHANGE(AddedTypeVariable) CHANGE(AddedConstraint) CHANGE(RemovedConstraint) @@ -53,11 +60,11 @@ CHANGE(RecordedKeyPathComponentType) CHANGE(DisabledConstraint) CHANGE(FavoredConstraint) CHANGE(RecordedResultBuilderTransform) -CHANGE(AppliedPropertyWrapper) CHANGE(RecordedClosureType) LAST_CHANGE(RecordedClosureType) #undef LOCATOR_CHANGE +#undef EXPR_CHANGE #undef LAST_CHANGE #undef CHANGE \ No newline at end of file diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index 0b8e1d17fdde6..82b8b858863c2 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -130,6 +130,7 @@ class SolverTrail { Change() : Kind(ChangeKind::AddedTypeVariable), TypeVar(nullptr) { } #define LOCATOR_CHANGE(Name) static Change Name(ConstraintLocator *locator); +#define EXPR_CHANGE(Name) static Change Name(Expr *expr); #include "swift/Sema/CSTrail.def" /// Create a change that added a type variable. @@ -203,9 +204,6 @@ class SolverTrail { /// Create a change that recorded a result builder transform. static Change RecordedResultBuilderTransform(AnyFunctionRef fn); - /// Create a change that recorded a result builder transform. - static Change AppliedPropertyWrapper(Expr *anchor); - /// Create a change that recorded a closure type. static Change RecordedClosureType(const ClosureExpr *closure); diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 1f026f98cd83b..8019cf25b8fa1 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -1515,7 +1515,7 @@ class Solution { /// Maps expressions for implied results (e.g implicit 'then' statements, /// implicit 'return' statements in single expression body closures) to their /// result kind. - llvm::MapVector ImpliedResults; + llvm::DenseMap ImpliedResults; /// For locators associated with call expressions, the trailing closure /// matching rule and parameter bindings that were applied. @@ -2233,7 +2233,7 @@ class ConstraintSystem { /// Maps expressions for implied results (e.g implicit 'then' statements, /// implicit 'return' statements in single expression body closures) to their /// result kind. - llvm::MapVector ImpliedResults; + llvm::DenseMap ImpliedResults; /// This is a *global* list of all result builder bodies that have /// been determined to be incorrect by failing constraint generation. @@ -2860,9 +2860,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - /// The length of \c ImpliedResults. - unsigned numImpliedResults; - /// The length of \c contextualTypes. unsigned numContextualTypes; @@ -3065,16 +3062,24 @@ class ConstraintSystem { } /// Record an implied result for a ReturnStmt or ThenStmt. - void recordImpliedResult(const Expr *E, ImpliedResultKind kind) { - assert(E); + void recordImpliedResult(Expr *E, ImpliedResultKind kind) { + ASSERT(E); auto inserted = ImpliedResults.insert({E, kind}).second; - assert(inserted && "Duplicate implied result?"); - (void)inserted; + ASSERT(inserted && "Duplicate implied result?"); + + if (solverState) + recordChange(SolverTrail::Change::RecordedImpliedResult(E)); + } + + /// Undo the above change. + void removeImpliedResult(Expr *E) { + bool erased = ImpliedResults.erase(E); + ASSERT(erased); } /// Whether the given expression is the implied result for either a ReturnStmt /// or ThenStmt, and if so, the kind of implied result. - std::optional isImpliedResult(const Expr *E) const { + std::optional isImpliedResult(Expr *E) const { auto result = ImpliedResults.find(E); if (result == ImpliedResults.end()) return std::nullopt; diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 57a19cc2012d8..ae61f2af2f3b4 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -323,8 +323,10 @@ void ConstraintSystem::applySolution(const Solution &solution) { } // Remember implied results. - for (auto impliedResult : solution.ImpliedResults) - ImpliedResults.insert(impliedResult); + for (auto impliedResult : solution.ImpliedResults) { + if (ImpliedResults.count(impliedResult.first) == 0) + recordImpliedResult(impliedResult.first, impliedResult.second); + } // Register the solution's opened types. for (const auto &opened : solution.OpenedTypes) { @@ -690,7 +692,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); numKeyPaths = cs.KeyPaths.size(); - numImpliedResults = cs.ImpliedResults.size(); numContextualTypes = cs.contextualTypes.size(); numTargets = cs.targets.size(); numCaseLabelItems = cs.caseLabelItems.size(); @@ -737,9 +738,6 @@ ConstraintSystem::SolverScope::~SolverScope() { /// Remove any key path expressions. truncate(cs.KeyPaths, numKeyPaths); - // Remove any implied results. - truncate(cs.ImpliedResults, numImpliedResults); - // Remove any contextual types. truncate(cs.contextualTypes, numContextualTypes); diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index 9062912a9c04c..93488874e60df 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -56,6 +56,14 @@ SolverTrail::~SolverTrail() { result.Locator = locator; \ return result; \ } +#define EXPR_CHANGE(Name) \ + SolverTrail::Change \ + SolverTrail::Change::Name(Expr *expr) { \ + Change result; \ + result.Kind = ChangeKind::Name; \ + result.TheExpr = expr; \ + return result; \ + } #include "swift/Sema/CSTrail.def" SolverTrail::Change @@ -239,14 +247,6 @@ SolverTrail::Change::RecordedResultBuilderTransform(AnyFunctionRef fn) { return result; } -SolverTrail::Change -SolverTrail::Change::AppliedPropertyWrapper(Expr *expr) { - Change result; - result.Kind = ChangeKind::AppliedPropertyWrapper; - result.TheExpr = expr; - return result; -} - SolverTrail::Change SolverTrail::Change::RecordedClosureType(const ClosureExpr *closure) { Change result; @@ -376,6 +376,10 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { case ChangeKind::RecordedClosureType: cs.removeClosureType(Closure); break; + + case ChangeKind::RecordedImpliedResult: + cs.removeImpliedResult(TheExpr); + break; } } @@ -395,6 +399,12 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, Locator->dump(&cs.getASTContext().SourceMgr, out); \ out << ")\n"; \ break; +#define EXPR_CHANGE(Name) \ + case ChangeKind::Name: \ + out << "(" << #Name << " "; \ + simple_display(out, TheExpr); \ + out << ")\n"; \ + break; #include "swift/Sema/CSTrail.def" case ChangeKind::AddedTypeVariable: @@ -558,12 +568,6 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, out << ")\n"; break; - case ChangeKind::AppliedPropertyWrapper: - out << "(AppliedPropertyWrapper "; - simple_display(out, TheExpr); - out << ")\n"; - break; - case ChangeKind::RecordedClosureType: out << "(RecordedClosureType "; simple_display(out, Closure); From 516277f0fde49f7a574915389e4bbcc74b32bd71 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 3 Oct 2024 23:16:43 -0400 Subject: [PATCH 15/26] Sema: Record contextual types in the trail --- include/swift/Sema/CSTrail.def | 3 ++- include/swift/Sema/CSTrail.h | 3 +++ include/swift/Sema/ConstraintSystem.h | 20 ++++++++++++-------- lib/Sema/CSSolver.cpp | 4 ---- lib/Sema/CSTrail.cpp | 17 +++++++++++++++++ 5 files changed, 34 insertions(+), 13 deletions(-) diff --git a/include/swift/Sema/CSTrail.def b/include/swift/Sema/CSTrail.def index a9f2a971bee67..c1bcc91d1ed7d 100644 --- a/include/swift/Sema/CSTrail.def +++ b/include/swift/Sema/CSTrail.def @@ -61,8 +61,9 @@ CHANGE(DisabledConstraint) CHANGE(FavoredConstraint) CHANGE(RecordedResultBuilderTransform) CHANGE(RecordedClosureType) +CHANGE(RecordedContextualInfo) -LAST_CHANGE(RecordedClosureType) +LAST_CHANGE(RecordedContextualInfo) #undef LOCATOR_CHANGE #undef EXPR_CHANGE diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index 82b8b858863c2..adb699bb103ca 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -207,6 +207,9 @@ class SolverTrail { /// Create a change that recorded a closure type. static Change RecordedClosureType(const ClosureExpr *closure); + /// Create a change that recorded the contextual type of an AST node. + static Change RecordedContextualInfo(ASTNode node); + /// Undo this change, reverting the constraint graph to the state it /// had prior to this change. /// diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 8019cf25b8fa1..b19a1981ccd7d 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2281,7 +2281,7 @@ class ConstraintSystem { /// constraint system. The second type, if valid, contains the type as it /// should appear in actual constraint. This will have unbound generic types /// opened, placeholder types converted to type variables, etc. - llvm::MapVector> contextualTypes; + llvm::DenseMap> contextualTypes; /// Information about each case label item tracked by the constraint system. llvm::SmallMapVector @@ -2860,9 +2860,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - /// The length of \c contextualTypes. - unsigned numContextualTypes; - /// The length of \c targets. unsigned numTargets; @@ -3294,10 +3291,17 @@ class ConstraintSystem { } void setContextualInfo(ASTNode node, ContextualTypeInfo info) { - assert(bool(node) && "Expected non-null expression!"); - assert(contextualTypes.count(node) == 0 && - "Already set this contextual type"); - contextualTypes[node] = {info, Type()}; + ASSERT(bool(node) && "Expected non-null expression!"); + bool inserted = contextualTypes.insert({node, {info, Type()}}).second; + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::RecordedContextualInfo(node)); + } + + void removeContextualInfo(ASTNode node) { + bool erased = contextualTypes.erase(node); + ASSERT(erased); } std::optional getContextualTypeInfo(ASTNode node) const { diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index ae61f2af2f3b4..882c71a30854c 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -692,7 +692,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); numKeyPaths = cs.KeyPaths.size(); - numContextualTypes = cs.contextualTypes.size(); numTargets = cs.targets.size(); numCaseLabelItems = cs.caseLabelItems.size(); numPotentialThrowSites = cs.potentialThrowSites.size(); @@ -738,9 +737,6 @@ ConstraintSystem::SolverScope::~SolverScope() { /// Remove any key path expressions. truncate(cs.KeyPaths, numKeyPaths); - // Remove any contextual types. - truncate(cs.contextualTypes, numContextualTypes); - // Remove any targets. truncate(cs.targets, numTargets); diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index 93488874e60df..717c3f3f64ad3 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -255,6 +255,14 @@ SolverTrail::Change::RecordedClosureType(const ClosureExpr *closure) { return result; } +SolverTrail::Change +SolverTrail::Change::RecordedContextualInfo(ASTNode node) { + Change result; + result.Kind = ChangeKind::RecordedContextualInfo; + result.Node.Node = node; + return result; +} + void SolverTrail::Change::undo(ConstraintSystem &cs) const { auto &cg = cs.getConstraintGraph(); @@ -380,6 +388,10 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { case ChangeKind::RecordedImpliedResult: cs.removeImpliedResult(TheExpr); break; + + case ChangeKind::RecordedContextualInfo: + cs.removeContextualInfo(Node.Node); + break; } } @@ -573,6 +585,11 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, simple_display(out, Closure); out << ")\n"; break; + + case ChangeKind::RecordedContextualInfo: + // FIXME: Print short form of ASTNode + out << "(RecordedContextualInfo)\n"; + break; } } From 8c8a385a4a6e1cd5d0eac91843f46851fcccabae Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 4 Oct 2024 12:17:02 -0400 Subject: [PATCH 16/26] Sema: Record SyntacticElementTargetKeys in the trail --- include/swift/Sema/CSTrail.def | 3 +- include/swift/Sema/CSTrail.h | 27 ++++-- include/swift/Sema/ConstraintSystem.h | 74 ++++++-------- lib/Sema/CSGen.cpp | 2 +- lib/Sema/CSSolver.cpp | 7 +- lib/Sema/CSSyntacticElement.cpp | 30 ++++++ lib/Sema/CSTrail.cpp | 135 ++++++++++++++++++++------ 7 files changed, 194 insertions(+), 84 deletions(-) diff --git a/include/swift/Sema/CSTrail.def b/include/swift/Sema/CSTrail.def index c1bcc91d1ed7d..48f350e071d02 100644 --- a/include/swift/Sema/CSTrail.def +++ b/include/swift/Sema/CSTrail.def @@ -62,8 +62,9 @@ CHANGE(FavoredConstraint) CHANGE(RecordedResultBuilderTransform) CHANGE(RecordedClosureType) CHANGE(RecordedContextualInfo) +CHANGE(RecordedTarget) -LAST_CHANGE(RecordedContextualInfo) +LAST_CHANGE(RecordedTarget) #undef LOCATOR_CHANGE #undef EXPR_CHANGE diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index adb699bb103ca..e5c5121fd2ddc 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -34,6 +34,7 @@ class TypeVariableType; namespace constraints { class Constraint; +struct SyntacticElementTargetKey; class SolverTrail { public: @@ -102,7 +103,6 @@ class SolverTrail { Type DstType; } Restriction; - ConstraintFix *Fix; struct { GenericTypeParamType *GP; @@ -119,12 +119,19 @@ class SolverTrail { Type OldType; } KeyPath; - ConstraintLocator *Locator; - PackExpansionType *ExpansionTy; - PackElementExpr *ElementExpr; + ConstraintFix *TheFix; + ConstraintLocator *TheLocator; + PackExpansionType *TheExpansion; + PackElementExpr *TheElement; Expr *TheExpr; - AnyFunctionRef AFR; - const ClosureExpr *Closure; + Stmt *TheStmt; + StmtConditionElement *TheCondElt; + Pattern *ThePattern; + PatternBindingDecl *ThePatternBinding; + VarDecl *TheVar; + AnyFunctionRef TheRef; + ClosureExpr *TheClosure; + DeclContext *TheDeclContext; }; Change() : Kind(ChangeKind::AddedTypeVariable), TypeVar(nullptr) { } @@ -205,11 +212,14 @@ class SolverTrail { static Change RecordedResultBuilderTransform(AnyFunctionRef fn); /// Create a change that recorded a closure type. - static Change RecordedClosureType(const ClosureExpr *closure); + static Change RecordedClosureType(ClosureExpr *closure); /// Create a change that recorded the contextual type of an AST node. static Change RecordedContextualInfo(ASTNode node); + /// Create a change that recorded a SyntacticElementTarget. + static Change RecordedTarget(SyntacticElementTargetKey key); + /// Undo this change, reverting the constraint graph to the state it /// had prior to this change. /// @@ -218,6 +228,9 @@ class SolverTrail { void dump(llvm::raw_ostream &out, ConstraintSystem &cs, unsigned indent = 0) const; + + private: + SyntacticElementTargetKey getSyntacticElementTargetKey() const; }; SolverTrail(ConstraintSystem &cs); diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index b19a1981ccd7d..c0c1bd682b5c1 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -1203,8 +1203,7 @@ struct CaseLabelItemInfo { /// Key to the constraint solver's mapping from AST nodes to their corresponding /// target. -class SyntacticElementTargetKey { -public: +struct SyntacticElementTargetKey { enum class Kind { empty, tombstone, @@ -1218,72 +1217,75 @@ class SyntacticElementTargetKey { functionRef, }; -private: Kind kind; union { - const StmtConditionElement *stmtCondElement; + StmtConditionElement *stmtCondElement; - const Expr *expr; + Expr *expr; - const Stmt *stmt; + Stmt *stmt; - const Pattern *pattern; + Pattern *pattern; struct PatternBindingEntry { - const PatternBindingDecl *patternBinding; + PatternBindingDecl *patternBinding; unsigned index; } patternBindingEntry; - const VarDecl *varDecl; + VarDecl *varDecl; - const DeclContext *functionRef; + DeclContext *functionRef; } storage; -public: SyntacticElementTargetKey(Kind kind) { assert(kind == Kind::empty || kind == Kind::tombstone); this->kind = kind; } - SyntacticElementTargetKey(const StmtConditionElement *stmtCondElement) { + SyntacticElementTargetKey(StmtConditionElement *stmtCondElement) { kind = Kind::stmtCondElement; storage.stmtCondElement = stmtCondElement; } - SyntacticElementTargetKey(const Expr *expr) { + SyntacticElementTargetKey(Expr *expr) { kind = Kind::expr; storage.expr = expr; } - SyntacticElementTargetKey(const ClosureExpr *closure) { + SyntacticElementTargetKey(ClosureExpr *closure) { kind = Kind::closure; storage.expr = closure; } - SyntacticElementTargetKey(const Stmt *stmt) { + SyntacticElementTargetKey(Stmt *stmt) { kind = Kind::stmt; storage.stmt = stmt; } - SyntacticElementTargetKey(const Pattern *pattern) { + SyntacticElementTargetKey(Pattern *pattern) { kind = Kind::pattern; storage.pattern = pattern; } - SyntacticElementTargetKey(const PatternBindingDecl *patternBinding, + SyntacticElementTargetKey(PatternBindingDecl *patternBinding, unsigned index) { kind = Kind::patternBindingEntry; storage.patternBindingEntry.patternBinding = patternBinding; storage.patternBindingEntry.index = index; } - SyntacticElementTargetKey(const VarDecl *varDecl) { + SyntacticElementTargetKey(VarDecl *varDecl) { kind = Kind::varDecl; storage.varDecl = varDecl; } - SyntacticElementTargetKey(const AnyFunctionRef functionRef) { + SyntacticElementTargetKey(DeclContext *dc) { + kind = Kind::functionRef; + storage.functionRef = dc; + } + + SyntacticElementTargetKey(AnyFunctionRef functionRef) { kind = Kind::functionRef; storage.functionRef = functionRef.getAsDeclContext(); } @@ -1574,7 +1576,7 @@ class Solution { std::vector> contextualTypes; /// Maps AST nodes to their target. - llvm::MapVector targets; + llvm::DenseMap targets; /// Maps case label items to information tracked about them as they are /// being solved. @@ -1727,12 +1729,7 @@ class Solution { } std::optional - getTargetFor(SyntacticElementTargetKey key) const { - auto known = targets.find(key); - if (known == targets.end()) - return std::nullopt; - return known->second; - } + getTargetFor(SyntacticElementTargetKey key) const; ConstraintLocator *getCalleeLocator(ConstraintLocator *locator, bool lookThroughApply = true) const; @@ -2275,7 +2272,7 @@ class ConstraintSystem { KeyPaths; /// Maps AST entries to their targets. - llvm::MapVector targets; + llvm::DenseMap targets; /// Contextual type information for expressions that are part of this /// constraint system. The second type, if valid, contains the type as it @@ -2860,9 +2857,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - /// The length of \c targets. - unsigned numTargets; - /// The length of \c caseLabelItems. unsigned numCaseLabelItems; @@ -3090,8 +3084,10 @@ class ConstraintSystem { bool inserted = ClosureTypes.insert({closure, type}).second; ASSERT(inserted); - if (solverState) - recordChange(SolverTrail::Change::RecordedClosureType(closure)); + if (solverState) { + recordChange(SolverTrail::Change::RecordedClosureType( + const_cast(closure))); + } } void removeClosureType(const ClosureExpr *closure) { @@ -3359,18 +3355,12 @@ class ConstraintSystem { } void setTargetFor(SyntacticElementTargetKey key, - SyntacticElementTarget target) { - assert(targets.count(key) == 0 && "Already set this target"); - targets.insert({key, target}); - } + SyntacticElementTarget target); + + void removeTargetFor(SyntacticElementTargetKey key); std::optional - getTargetFor(SyntacticElementTargetKey key) const { - auto known = targets.find(key); - if (known == targets.end()) - return std::nullopt; - return known->second; - } + getTargetFor(SyntacticElementTargetKey key) const; std::optional getAppliedResultBuilderTransform(AnyFunctionRef fn) const { diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index da6c9f1171845..f7936a4049c61 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -4995,7 +4995,7 @@ bool ConstraintSystem::generateConstraints(StmtCondition condition, } Type boolTy = boolDecl->getDeclaredInterfaceType(); - for (const auto &condElement : condition) { + for (auto &condElement : condition) { switch (condElement.getKind()) { case StmtConditionElement::CK_Availability: // Nothing to do here. diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 882c71a30854c..88c6d3a6604b6 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -218,7 +218,8 @@ Solution ConstraintSystem::finalize() { solution.contextualTypes.push_back({entry.first, entry.second.first}); } - solution.targets = targets; + for (auto &target : targets) + solution.targets.insert(target); for (const auto &item : caseLabelItems) solution.caseLabelItems.insert(item); @@ -692,7 +693,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); numKeyPaths = cs.KeyPaths.size(); - numTargets = cs.targets.size(); numCaseLabelItems = cs.caseLabelItems.size(); numPotentialThrowSites = cs.potentialThrowSites.size(); numExprPatterns = cs.exprPatterns.size(); @@ -737,9 +737,6 @@ ConstraintSystem::SolverScope::~SolverScope() { /// Remove any key path expressions. truncate(cs.KeyPaths, numKeyPaths); - // Remove any targets. - truncate(cs.targets, numTargets); - // Remove any case label item infos. truncate(cs.caseLabelItems, numCaseLabelItems); diff --git a/lib/Sema/CSSyntacticElement.cpp b/lib/Sema/CSSyntacticElement.cpp index 850ff03e5a7ad..9f444422d8de5 100644 --- a/lib/Sema/CSSyntacticElement.cpp +++ b/lib/Sema/CSSyntacticElement.cpp @@ -26,6 +26,36 @@ using namespace swift; using namespace swift::constraints; +void ConstraintSystem::setTargetFor(SyntacticElementTargetKey key, + SyntacticElementTarget target) { + bool inserted = targets.insert({key, target}).second; + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::RecordedTarget(key)); +} + +void ConstraintSystem::removeTargetFor(SyntacticElementTargetKey key) { + bool erased = targets.erase(key); + ASSERT(erased); +} + +std::optional +ConstraintSystem::getTargetFor(SyntacticElementTargetKey key) const { + auto known = targets.find(key); + if (known == targets.end()) + return std::nullopt; + return known->second; +} + +std::optional +Solution::getTargetFor(SyntacticElementTargetKey key) const { + auto known = targets.find(key); + if (known == targets.end()) + return std::nullopt; + return known->second; +} + namespace { // Produce an implicit empty tuple expression. diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index 717c3f3f64ad3..968f783189dc3 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -53,7 +53,7 @@ SolverTrail::~SolverTrail() { SolverTrail::Change::Name(ConstraintLocator *locator) { \ Change result; \ result.Kind = ChangeKind::Name; \ - result.Locator = locator; \ + result.TheLocator = locator; \ return result; \ } #define EXPR_CHANGE(Name) \ @@ -160,7 +160,7 @@ SolverTrail::Change SolverTrail::Change::AddedFix(ConstraintFix *fix) { Change result; result.Kind = ChangeKind::AddedFix; - result.Fix = fix; + result.TheFix = fix; return result; } @@ -181,7 +181,7 @@ SolverTrail::Change::RecordedDisjunctionChoice(ConstraintLocator *locator, unsigned index) { Change result; result.Kind = ChangeKind::RecordedDisjunctionChoice; - result.Locator = locator; + result.TheLocator = locator; result.Options = index; return result; } @@ -190,7 +190,7 @@ SolverTrail::Change SolverTrail::Change::RecordedOpenedPackExpansionType(PackExpansionType *expansionTy) { Change result; result.Kind = ChangeKind::RecordedOpenedPackExpansionType; - result.ExpansionTy = expansionTy; + result.TheExpansion = expansionTy; return result; } @@ -198,7 +198,7 @@ SolverTrail::Change SolverTrail::Change::RecordedPackEnvironment(PackElementExpr *packElement) { Change result; result.Kind = ChangeKind::RecordedPackEnvironment; - result.ElementExpr = packElement; + result.TheElement = packElement; return result; } @@ -243,15 +243,15 @@ SolverTrail::Change SolverTrail::Change::RecordedResultBuilderTransform(AnyFunctionRef fn) { Change result; result.Kind = ChangeKind::RecordedResultBuilderTransform; - result.AFR = fn; + result.TheRef = fn; return result; } SolverTrail::Change -SolverTrail::Change::RecordedClosureType(const ClosureExpr *closure) { +SolverTrail::Change::RecordedClosureType(ClosureExpr *closure) { Change result; result.Kind = ChangeKind::RecordedClosureType; - result.Closure = closure; + result.TheClosure = closure; return result; } @@ -263,6 +263,75 @@ SolverTrail::Change::RecordedContextualInfo(ASTNode node) { return result; } +SolverTrail::Change +SolverTrail::Change::RecordedTarget(SyntacticElementTargetKey key) { + Change result; + result.Kind = ChangeKind::RecordedTarget; + result.Options = unsigned(key.kind); + + switch (key.kind) { + case SyntacticElementTargetKey::Kind::empty: + case SyntacticElementTargetKey::Kind::tombstone: + llvm_unreachable("Invalid SyntacticElementTargetKey::Kind"); + case SyntacticElementTargetKey::Kind::stmtCondElement: + result.TheCondElt = key.storage.stmtCondElement; + break; + case SyntacticElementTargetKey::Kind::expr: + result.TheExpr = key.storage.expr; + break; + case SyntacticElementTargetKey::Kind::closure: + result.TheClosure = cast(key.storage.expr); + break; + case SyntacticElementTargetKey::Kind::stmt: + result.TheStmt = key.storage.stmt; + break; + case SyntacticElementTargetKey::Kind::pattern: + result.ThePattern = key.storage.pattern; + break; + case SyntacticElementTargetKey::Kind::patternBindingEntry: + result.ThePatternBinding = key.storage.patternBindingEntry.patternBinding; + result.Options |= key.storage.patternBindingEntry.index << 8; + break; + case SyntacticElementTargetKey::Kind::varDecl: + result.TheVar = key.storage.varDecl; + break; + case SyntacticElementTargetKey::Kind::functionRef: + result.TheDeclContext = key.storage.functionRef; + break; + } + + return result; +} + +SyntacticElementTargetKey +SolverTrail::Change::getSyntacticElementTargetKey() const { + ASSERT(Kind == ChangeKind::RecordedTarget); + + auto kind = SyntacticElementTargetKey::Kind(Options & 0xff); + + switch (kind) { + case SyntacticElementTargetKey::Kind::empty: + case SyntacticElementTargetKey::Kind::tombstone: + llvm_unreachable("Invalid SyntacticElementTargetKey::Kind"); + case SyntacticElementTargetKey::Kind::stmtCondElement: + return SyntacticElementTargetKey(TheCondElt); + case SyntacticElementTargetKey::Kind::expr: + return SyntacticElementTargetKey(TheExpr); + case SyntacticElementTargetKey::Kind::closure: + return SyntacticElementTargetKey(TheClosure); + case SyntacticElementTargetKey::Kind::stmt: + return SyntacticElementTargetKey(TheStmt); + case SyntacticElementTargetKey::Kind::pattern: + return SyntacticElementTargetKey(ThePattern); + case SyntacticElementTargetKey::Kind::patternBindingEntry: + return SyntacticElementTargetKey(ThePatternBinding, Options >> 8); + case SyntacticElementTargetKey::Kind::varDecl: + return SyntacticElementTargetKey(TheVar); + case SyntacticElementTargetKey::Kind::functionRef: + return SyntacticElementTargetKey(TheDeclContext); + } +} + void SolverTrail::Change::undo(ConstraintSystem &cs) const { auto &cg = cs.getConstraintGraph(); @@ -308,7 +377,7 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { break; case ChangeKind::AddedFix: - cs.removeFix(Fix); + cs.removeFix(TheFix); break; case ChangeKind::AddedFixedRequirement: @@ -317,39 +386,39 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { break; case ChangeKind::RecordedDisjunctionChoice: - cs.removeDisjunctionChoice(Locator); + cs.removeDisjunctionChoice(TheLocator); break; case ChangeKind::RecordedAppliedDisjunction: - cs.removeAppliedDisjunction(Locator); + cs.removeAppliedDisjunction(TheLocator); break; case ChangeKind::RecordedMatchCallArgumentResult: - cs.removeMatchCallArgumentResult(Locator); + cs.removeMatchCallArgumentResult(TheLocator); break; case ChangeKind::RecordedOpenedTypes: - cs.removeOpenedType(Locator); + cs.removeOpenedType(TheLocator); break; case ChangeKind::RecordedOpenedExistentialType: - cs.removeOpenedExistentialType(Locator); + cs.removeOpenedExistentialType(TheLocator); break; case ChangeKind::RecordedOpenedPackExpansionType: - cs.removeOpenedPackExpansionType(ExpansionTy); + cs.removeOpenedPackExpansionType(TheExpansion); break; case ChangeKind::RecordedPackExpansionEnvironment: - cs.removePackExpansionEnvironment(Locator); + cs.removePackExpansionEnvironment(TheLocator); break; case ChangeKind::RecordedPackEnvironment: - cs.removePackEnvironment(ElementExpr); + cs.removePackEnvironment(TheElement); break; case ChangeKind::RecordedDefaultedConstraint: - cs.removeDefaultedConstraint(Locator); + cs.removeDefaultedConstraint(TheLocator); break; case ChangeKind::RecordedNodeType: @@ -370,7 +439,7 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { break; case ChangeKind::RecordedResultBuilderTransform: - cs.removeResultBuilderTransform(AFR); + cs.removeResultBuilderTransform(TheRef); break; case ChangeKind::AppliedPropertyWrapper: @@ -378,11 +447,11 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { break; case ChangeKind::ResolvedOverload: - cs.removeResolvedOverload(Locator); + cs.removeResolvedOverload(TheLocator); break; case ChangeKind::RecordedClosureType: - cs.removeClosureType(Closure); + cs.removeClosureType(TheClosure); break; case ChangeKind::RecordedImpliedResult: @@ -392,6 +461,10 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { case ChangeKind::RecordedContextualInfo: cs.removeContextualInfo(Node.Node); break; + + case ChangeKind::RecordedTarget: + cs.removeTargetFor(getSyntacticElementTargetKey()); + break; } } @@ -408,7 +481,7 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, #define LOCATOR_CHANGE(Name) \ case ChangeKind::Name: \ out << "(" << #Name << " at "; \ - Locator->dump(&cs.getASTContext().SourceMgr, out); \ + TheLocator->dump(&cs.getASTContext().SourceMgr, out); \ out << ")\n"; \ break; #define EXPR_CHANGE(Name) \ @@ -505,7 +578,7 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, case ChangeKind::AddedFix: out << "(AddedFix "; - Fix->print(out); + TheFix->print(out); out << ")\n"; break; @@ -520,21 +593,21 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, case ChangeKind::RecordedDisjunctionChoice: out << "(RecordedDisjunctionChoice at "; - Locator->dump(&cs.getASTContext().SourceMgr, out); + TheLocator->dump(&cs.getASTContext().SourceMgr, out); out << " index "; out << Options << ")\n"; break; case ChangeKind::RecordedOpenedPackExpansionType: out << "(RecordedOpenedPackExpansionType for "; - ExpansionTy->print(out, PO); + TheExpansion->print(out, PO); out << ")\n"; break; case ChangeKind::RecordedPackEnvironment: // FIXME: Print short form of PackExpansionExpr out << "(RecordedPackEnvironment "; - simple_display(out, ElementExpr); + simple_display(out, TheElement); out << "\n"; break; @@ -576,13 +649,13 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, case ChangeKind::RecordedResultBuilderTransform: out << "(RecordedResultBuilderTransform "; - simple_display(out, AFR); + simple_display(out, TheRef); out << ")\n"; break; case ChangeKind::RecordedClosureType: out << "(RecordedClosureType "; - simple_display(out, Closure); + simple_display(out, TheClosure); out << ")\n"; break; @@ -590,6 +663,12 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, // FIXME: Print short form of ASTNode out << "(RecordedContextualInfo)\n"; break; + + case ChangeKind::RecordedTarget: + out << "(RecordedTarget "; + getSyntacticElementTargetKey().dump(out); + out << ")\n"; + break; } } From 39d0eab2753e9012be4145dc49c6e63b60e622ee Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 4 Oct 2024 13:24:44 -0400 Subject: [PATCH 17/26] Sema: Record case label items in the trail --- include/swift/Sema/CSTrail.def | 3 ++- include/swift/Sema/CSTrail.h | 4 ++++ include/swift/Sema/ConstraintSystem.h | 25 +++++++++++++++---------- lib/Sema/CSSolver.cpp | 4 ---- lib/Sema/CSTrail.cpp | 17 +++++++++++++++++ 5 files changed, 38 insertions(+), 15 deletions(-) diff --git a/include/swift/Sema/CSTrail.def b/include/swift/Sema/CSTrail.def index 48f350e071d02..4bc0c4e28941f 100644 --- a/include/swift/Sema/CSTrail.def +++ b/include/swift/Sema/CSTrail.def @@ -63,8 +63,9 @@ CHANGE(RecordedResultBuilderTransform) CHANGE(RecordedClosureType) CHANGE(RecordedContextualInfo) CHANGE(RecordedTarget) +CHANGE(RecordedCaseLabelItemInfo) -LAST_CHANGE(RecordedTarget) +LAST_CHANGE(RecordedCaseLabelItemInfo) #undef LOCATOR_CHANGE #undef EXPR_CHANGE diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index e5c5121fd2ddc..5760482c6e9f6 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -132,6 +132,7 @@ class SolverTrail { AnyFunctionRef TheRef; ClosureExpr *TheClosure; DeclContext *TheDeclContext; + CaseLabelItem *TheItem; }; Change() : Kind(ChangeKind::AddedTypeVariable), TypeVar(nullptr) { } @@ -220,6 +221,9 @@ class SolverTrail { /// Create a change that recorded a SyntacticElementTarget. static Change RecordedTarget(SyntacticElementTargetKey key); + /// Create a change that recorded a SyntacticElementTarget. + static Change RecordedCaseLabelItemInfo(CaseLabelItem *item); + /// Undo this change, reverting the constraint graph to the state it /// had prior to this change. /// diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index c0c1bd682b5c1..a7bea7a333ffe 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -1580,8 +1580,7 @@ class Solution { /// Maps case label items to information tracked about them as they are /// being solved. - llvm::MapVector - caseLabelItems; + llvm::DenseMap caseLabelItems; /// Maps catch nodes to the set of potential throw sites that will be caught /// at that location. @@ -2281,8 +2280,7 @@ class ConstraintSystem { llvm::DenseMap> contextualTypes; /// Information about each case label item tracked by the constraint system. - llvm::SmallMapVector - caseLabelItems; + llvm::SmallDenseMap caseLabelItems; /// Keep track of all of the potential throw sites. /// FIXME: This data structure should be replaced with something that @@ -2857,9 +2855,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - /// The length of \c caseLabelItems. - unsigned numCaseLabelItems; - /// The length of \c potentialThrowSites. unsigned numPotentialThrowSites; @@ -3392,9 +3387,19 @@ class ConstraintSystem { } void setCaseLabelItemInfo(const CaseLabelItem *item, CaseLabelItemInfo info) { - assert(item != nullptr); - assert(caseLabelItems.count(item) == 0); - caseLabelItems[item] = info; + ASSERT(item); + bool inserted = caseLabelItems.insert({item, info}).second; + ASSERT(inserted); + + if (solverState) { + recordChange(SolverTrail::Change::RecordedCaseLabelItemInfo( + const_cast(item))); + } + } + + void removeCaseLabelItemInfo(const CaseLabelItem *item) { + bool erased = caseLabelItems.erase(item); + ASSERT(erased); } /// Record a given ExprPattern as the parent of its sub-expression. diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 88c6d3a6604b6..a4ca05a943ad1 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -693,7 +693,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); numKeyPaths = cs.KeyPaths.size(); - numCaseLabelItems = cs.caseLabelItems.size(); numPotentialThrowSites = cs.potentialThrowSites.size(); numExprPatterns = cs.exprPatterns.size(); numIsolatedParams = cs.isolatedParams.size(); @@ -737,9 +736,6 @@ ConstraintSystem::SolverScope::~SolverScope() { /// Remove any key path expressions. truncate(cs.KeyPaths, numKeyPaths); - // Remove any case label item infos. - truncate(cs.caseLabelItems, numCaseLabelItems); - // Remove any potential throw sites. truncate(cs.potentialThrowSites, numPotentialThrowSites); diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index 968f783189dc3..e46e9cd9ded70 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -303,6 +303,14 @@ SolverTrail::Change::RecordedTarget(SyntacticElementTargetKey key) { return result; } +SolverTrail::Change +SolverTrail::Change::RecordedCaseLabelItemInfo(CaseLabelItem *item) { + Change result; + result.Kind = ChangeKind::RecordedCaseLabelItemInfo; + result.TheItem = item; + return result; +} + SyntacticElementTargetKey SolverTrail::Change::getSyntacticElementTargetKey() const { ASSERT(Kind == ChangeKind::RecordedTarget); @@ -465,6 +473,10 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { case ChangeKind::RecordedTarget: cs.removeTargetFor(getSyntacticElementTargetKey()); break; + + case ChangeKind::RecordedCaseLabelItemInfo: + cs.removeCaseLabelItemInfo(TheItem); + break; } } @@ -669,6 +681,11 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, getSyntacticElementTargetKey().dump(out); out << ")\n"; break; + + case ChangeKind::RecordedCaseLabelItemInfo: + // FIXME: Print something here + out << "(RecordedCaseLabelItemInfo)\n"; + break; } } From 500acd122ac3b7d672b97d7428db629d74371590 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 4 Oct 2024 13:34:34 -0400 Subject: [PATCH 18/26] Sema: Record potential throw sites in the trail --- include/swift/Sema/CSTrail.def | 3 ++- include/swift/Sema/CSTrail.h | 4 ++++ include/swift/Sema/ConstraintSystem.h | 13 +++++++++---- lib/Sema/CSSolver.cpp | 12 +++++------- lib/Sema/CSTrail.cpp | 17 +++++++++++++++++ lib/Sema/ConstraintSystem.cpp | 22 ++++++++++++++++------ 6 files changed, 53 insertions(+), 18 deletions(-) diff --git a/include/swift/Sema/CSTrail.def b/include/swift/Sema/CSTrail.def index 4bc0c4e28941f..41e6fe3dfa14f 100644 --- a/include/swift/Sema/CSTrail.def +++ b/include/swift/Sema/CSTrail.def @@ -64,8 +64,9 @@ CHANGE(RecordedClosureType) CHANGE(RecordedContextualInfo) CHANGE(RecordedTarget) CHANGE(RecordedCaseLabelItemInfo) +CHANGE(RecordedPotentialThrowSite) -LAST_CHANGE(RecordedCaseLabelItemInfo) +LAST_CHANGE(RecordedPotentialThrowSite) #undef LOCATOR_CHANGE #undef EXPR_CHANGE diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index 5760482c6e9f6..196e755652bed 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -133,6 +133,7 @@ class SolverTrail { ClosureExpr *TheClosure; DeclContext *TheDeclContext; CaseLabelItem *TheItem; + CatchNode TheCatchNode; }; Change() : Kind(ChangeKind::AddedTypeVariable), TypeVar(nullptr) { } @@ -224,6 +225,9 @@ class SolverTrail { /// Create a change that recorded a SyntacticElementTarget. static Change RecordedCaseLabelItemInfo(CaseLabelItem *item); + /// Create a change that recorded a potential throw site. + static Change RecordedPotentialThrowSite(CatchNode catchNode); + /// Undo this change, reverting the constraint graph to the state it /// had prior to this change. /// diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index a7bea7a333ffe..f98b4e9008474 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -1585,7 +1585,7 @@ class Solution { /// Maps catch nodes to the set of potential throw sites that will be caught /// at that location. - /// The set of opened types for a given locator. + /// Keep track of all of the potential throw sites. std::vector> potentialThrowSites; @@ -2855,9 +2855,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - /// The length of \c potentialThrowSites. - unsigned numPotentialThrowSites; - /// The length of \c exprPatterns. unsigned numExprPatterns; @@ -3425,6 +3422,14 @@ class ConstraintSystem { PotentialThrowSite::Kind kind, Type type, ConstraintLocatorBuilder locator); + /// Used by the above to update potentialThrowSites and record a change + /// in the trail. + void recordPotentialThrowSite(CatchNode catchNode, + PotentialThrowSite site); + + /// Undo the above change. + void removePotentialThrowSite(CatchNode catchNode); + /// Determine the caught error type for the given catch node. Type getCaughtErrorType(CatchNode node); diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index a4ca05a943ad1..fe6d4fd579f1d 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -399,9 +399,11 @@ void ConstraintSystem::applySolution(const Solution &solution) { setCaseLabelItemInfo(info.first, info.second); } - potentialThrowSites.insert(potentialThrowSites.end(), - solution.potentialThrowSites.begin(), - solution.potentialThrowSites.end()); + auto sites = ArrayRef(solution.potentialThrowSites); + ASSERT(sites.size() >= potentialThrowSites.size()); + for (const auto &site : sites.slice(potentialThrowSites.size())) { + potentialThrowSites.push_back(site); + } for (auto param : solution.isolatedParams) { isolatedParams.insert(param); @@ -693,7 +695,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); numKeyPaths = cs.KeyPaths.size(); - numPotentialThrowSites = cs.potentialThrowSites.size(); numExprPatterns = cs.exprPatterns.size(); numIsolatedParams = cs.isolatedParams.size(); numPreconcurrencyClosures = cs.preconcurrencyClosures.size(); @@ -736,9 +737,6 @@ ConstraintSystem::SolverScope::~SolverScope() { /// Remove any key path expressions. truncate(cs.KeyPaths, numKeyPaths); - // Remove any potential throw sites. - truncate(cs.potentialThrowSites, numPotentialThrowSites); - // Remove any ExprPattern mappings. truncate(cs.exprPatterns, numExprPatterns); diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index e46e9cd9ded70..b2aebc2acd450 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -311,6 +311,14 @@ SolverTrail::Change::RecordedCaseLabelItemInfo(CaseLabelItem *item) { return result; } +SolverTrail::Change +SolverTrail::Change::RecordedPotentialThrowSite(CatchNode catchNode) { + Change result; + result.Kind = ChangeKind::RecordedPotentialThrowSite; + result.TheCatchNode = catchNode; + return result; +} + SyntacticElementTargetKey SolverTrail::Change::getSyntacticElementTargetKey() const { ASSERT(Kind == ChangeKind::RecordedTarget); @@ -477,6 +485,10 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { case ChangeKind::RecordedCaseLabelItemInfo: cs.removeCaseLabelItemInfo(TheItem); break; + + case ChangeKind::RecordedPotentialThrowSite: + cs.removePotentialThrowSite(TheCatchNode); + break; } } @@ -686,6 +698,11 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, // FIXME: Print something here out << "(RecordedCaseLabelItemInfo)\n"; break; + + case ChangeKind::RecordedPotentialThrowSite: + // FIXME: Print something here + out << "(RecordedPotentialThrowSite)\n"; + break; } } diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 8d84c52172f78..190a8add556ce 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -435,6 +435,18 @@ bool ConstraintSystem::containsIDEInspectionTarget( Context.SourceMgr); } +void ConstraintSystem::recordPotentialThrowSite( + CatchNode catchNode, PotentialThrowSite site) { + potentialThrowSites.push_back({catchNode, site}); + if (solverState) + recordChange(SolverTrail::Change::RecordedPotentialThrowSite(catchNode)); +} + +void ConstraintSystem::removePotentialThrowSite(CatchNode catchNode) { + ASSERT(potentialThrowSites.back().first == catchNode); + potentialThrowSites.pop_back(); +} + void ConstraintSystem::recordPotentialThrowSite( PotentialThrowSite::Kind kind, Type type, ConstraintLocatorBuilder locator) { @@ -461,9 +473,8 @@ void ConstraintSystem::recordPotentialThrowSite( // do..catch statements without an explicit `throws` clause do infer // thrown types. if (auto doCatch = catchNode.dyn_cast()) { - potentialThrowSites.push_back( - {catchNode, - PotentialThrowSite{kind, type, getConstraintLocator(locator)}}); + PotentialThrowSite site{kind, type, getConstraintLocator(locator)}; + recordPotentialThrowSite(catchNode, site); return; } @@ -476,9 +487,8 @@ void ConstraintSystem::recordPotentialThrowSite( if (!closureEffects(closure).isThrowing()) return; - potentialThrowSites.push_back( - {catchNode, - PotentialThrowSite{kind, type, getConstraintLocator(locator)}}); + PotentialThrowSite site{kind, type, getConstraintLocator(locator)}; + recordPotentialThrowSite(catchNode, site); } Type ConstraintSystem::getCaughtErrorType(CatchNode catchNode) { From 1d177d0187199b59d7542b6b801c45d245d08264 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 4 Oct 2024 13:49:44 -0400 Subject: [PATCH 19/26] Sema: Record expression patterns in the trail --- include/swift/Sema/CSTrail.def | 1 + include/swift/Sema/ConstraintSystem.h | 25 +++++++++++++++---------- lib/Sema/CSSolver.cpp | 10 ++++------ lib/Sema/CSTrail.cpp | 4 ++++ 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/include/swift/Sema/CSTrail.def b/include/swift/Sema/CSTrail.def index 41e6fe3dfa14f..dd9acea5e80ce 100644 --- a/include/swift/Sema/CSTrail.def +++ b/include/swift/Sema/CSTrail.def @@ -40,6 +40,7 @@ LOCATOR_CHANGE(ResolvedOverload) EXPR_CHANGE(AppliedPropertyWrapper) EXPR_CHANGE(RecordedImpliedResult) +EXPR_CHANGE(RecordedExprPattern) CHANGE(AddedTypeVariable) CHANGE(AddedConstraint) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index f98b4e9008474..ab96f6cdaea3f 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -1591,7 +1591,7 @@ class Solution { /// A map of expressions to the ExprPatterns that they are being solved as /// a part of. - llvm::MapVector exprPatterns; + llvm::DenseMap exprPatterns; /// The set of parameters that have been inferred to be 'isolated'. std::vector isolatedParams; @@ -2289,7 +2289,7 @@ class ConstraintSystem { /// A map of expressions to the ExprPatterns that they are being solved as /// a part of. - llvm::SmallMapVector exprPatterns; + llvm::SmallDenseMap exprPatterns; /// The set of parameters that have been inferred to be 'isolated'. llvm::SmallSetVector isolatedParams; @@ -2855,9 +2855,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - /// The length of \c exprPatterns. - unsigned numExprPatterns; - /// The length of \c isolatedParams. unsigned numIsolatedParams; @@ -3401,11 +3398,19 @@ class ConstraintSystem { /// Record a given ExprPattern as the parent of its sub-expression. void setExprPatternFor(Expr *E, ExprPattern *EP) { - assert(E); - assert(EP); - auto inserted = exprPatterns.insert({E, EP}).second; - assert(inserted && "Mapping already defined?"); - (void)inserted; + ASSERT(E); + ASSERT(EP); + bool inserted = exprPatterns.insert({E, EP}).second; + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::RecordedExprPattern(E)); + } + + /// Record a given ExprPattern as the parent of its sub-expression. + void removeExprPatternFor(Expr *E) { + bool erased = exprPatterns.erase(E); + ASSERT(erased); } std::optional diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index fe6d4fd579f1d..efbb06dbab817 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -409,8 +409,10 @@ void ConstraintSystem::applySolution(const Solution &solution) { isolatedParams.insert(param); } - for (auto &pair : solution.exprPatterns) - exprPatterns.insert(pair); + for (auto &pair : solution.exprPatterns) { + if (exprPatterns.count(pair.first) == 0) + setExprPatternFor(pair.first, pair.second); + } for (auto closure : solution.preconcurrencyClosures) { preconcurrencyClosures.insert(closure); @@ -695,7 +697,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); numKeyPaths = cs.KeyPaths.size(); - numExprPatterns = cs.exprPatterns.size(); numIsolatedParams = cs.isolatedParams.size(); numPreconcurrencyClosures = cs.preconcurrencyClosures.size(); numImplicitValueConversions = cs.ImplicitValueConversions.size(); @@ -737,9 +738,6 @@ ConstraintSystem::SolverScope::~SolverScope() { /// Remove any key path expressions. truncate(cs.KeyPaths, numKeyPaths); - // Remove any ExprPattern mappings. - truncate(cs.exprPatterns, numExprPatterns); - // Remove any isolated parameters. truncate(cs.isolatedParams, numIsolatedParams); diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index b2aebc2acd450..85c5725f4466b 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -489,6 +489,10 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { case ChangeKind::RecordedPotentialThrowSite: cs.removePotentialThrowSite(TheCatchNode); break; + + case ChangeKind::RecordedExprPattern: + cs.removeExprPatternFor(TheExpr); + break; } } From b961a7ec51f8498f28d9433d1cbc04ff22ce85ae Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 4 Oct 2024 13:58:54 -0400 Subject: [PATCH 20/26] Sema: Record isolated parameters in the trail --- include/swift/Sema/CSTrail.def | 3 ++- include/swift/Sema/CSTrail.h | 4 ++++ include/swift/Sema/ConstraintSystem.h | 14 +++++++++----- lib/Sema/CSApply.cpp | 8 +++----- lib/Sema/CSSimplify.cpp | 15 ++++++++++++++- lib/Sema/CSSolver.cpp | 9 +++------ lib/Sema/CSTrail.cpp | 18 ++++++++++++++++++ lib/Sema/ConstraintSystem.cpp | 2 +- 8 files changed, 54 insertions(+), 19 deletions(-) diff --git a/include/swift/Sema/CSTrail.def b/include/swift/Sema/CSTrail.def index dd9acea5e80ce..24d6f58dbee0d 100644 --- a/include/swift/Sema/CSTrail.def +++ b/include/swift/Sema/CSTrail.def @@ -66,8 +66,9 @@ CHANGE(RecordedContextualInfo) CHANGE(RecordedTarget) CHANGE(RecordedCaseLabelItemInfo) CHANGE(RecordedPotentialThrowSite) +CHANGE(RecordedIsolatedParam) -LAST_CHANGE(RecordedPotentialThrowSite) +LAST_CHANGE(RecordedIsolatedParam) #undef LOCATOR_CHANGE #undef EXPR_CHANGE diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index 196e755652bed..f461e9cabf421 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -134,6 +134,7 @@ class SolverTrail { DeclContext *TheDeclContext; CaseLabelItem *TheItem; CatchNode TheCatchNode; + ParamDecl *TheParam; }; Change() : Kind(ChangeKind::AddedTypeVariable), TypeVar(nullptr) { } @@ -228,6 +229,9 @@ class SolverTrail { /// Create a change that recorded a potential throw site. static Change RecordedPotentialThrowSite(CatchNode catchNode); + /// Create a change that recorded an isolated parameter. + static Change RecordedIsolatedParam(ParamDecl *param); + /// Undo this change, reverting the constraint graph to the state it /// had prior to this change. /// diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index ab96f6cdaea3f..031a366f2bd6f 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -1594,7 +1594,7 @@ class Solution { llvm::DenseMap exprPatterns; /// The set of parameters that have been inferred to be 'isolated'. - std::vector isolatedParams; + llvm::DenseSet isolatedParams; /// The set of closures that have been inferred to be "isolated by /// preconcurrency". @@ -2292,7 +2292,7 @@ class ConstraintSystem { llvm::SmallDenseMap exprPatterns; /// The set of parameters that have been inferred to be 'isolated'. - llvm::SmallSetVector isolatedParams; + llvm::SmallDenseSet isolatedParams; /// The set of closures that have been inferred to be "isolated by /// preconcurrency". @@ -2855,9 +2855,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - /// The length of \c isolatedParams. - unsigned numIsolatedParams; - /// The length of \c PreconcurrencyClosures. unsigned numPreconcurrencyClosures; @@ -4215,6 +4212,13 @@ class ConstraintSystem { bool resolveClosure(TypeVariableType *typeVar, Type contextualType, ConstraintLocatorBuilder locator); + /// Used by the above to update isolatedParams and record a change in + /// the trail. + void recordIsolatedParam(ParamDecl *param); + + /// Undo the above change. + void removeIsolatedParam(ParamDecl *param); + /// Given the fact that contextual type is now available for the type /// variable representing a pack expansion type, let's resolve the expansion. /// diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index ecfd76aea94be..646283676864f 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -8539,11 +8539,9 @@ bool ExprRewriter::isDistributedThunk(ConcreteDeclRef ref, Expr *context) { // If this is a method reference on an potentially isolated // actor then it cannot be a remote thunk. bool isPotentiallyIsolated = isPotentiallyIsolatedActor( - actor, - [&](ParamDecl *P) { - return P->isIsolated() || - llvm::is_contained(solution.isolatedParams, P); - }); + actor, [&](ParamDecl *P) { + return P->isIsolated() || solution.isolatedParams.count(P); + }); // Adjust the declaration context to the innermost context that is neither // a local function nor a closure, so that the actor reference is checked diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index e3aaed5b409b5..b0ffb967f6b05 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -11513,6 +11513,19 @@ static Type getOpenedResultBuilderTypeFor(ConstraintSystem &cs, return builderType; } +void ConstraintSystem::recordIsolatedParam(ParamDecl *param) { + bool inserted = isolatedParams.insert(param).second; + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::RecordedIsolatedParam(param)); +} + +void ConstraintSystem::removeIsolatedParam(ParamDecl *param) { + bool erased = isolatedParams.erase(param); + ASSERT(erased); +} + bool ConstraintSystem::resolveClosure(TypeVariableType *typeVar, Type contextualType, ConstraintLocatorBuilder locator) { @@ -11658,7 +11671,7 @@ bool ConstraintSystem::resolveClosure(TypeVariableType *typeVar, // Note when a parameter is inferred to be isolated. if (contextualParam->isIsolated() && !flags.isIsolated() && paramDecl) - isolatedParams.insert(paramDecl); + recordIsolatedParam(paramDecl); // Carry-over the ownership specifier from the contextual parameter. auto paramOwnership = diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index efbb06dbab817..f39aecac7170c 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -231,7 +231,7 @@ Solution ConstraintSystem::finalize() { solution.exprPatterns.insert(pattern); for (const auto ¶m : isolatedParams) - solution.isolatedParams.push_back(param); + solution.isolatedParams.insert(param); for (const auto &closure : preconcurrencyClosures) solution.preconcurrencyClosures.push_back(closure); @@ -406,7 +406,8 @@ void ConstraintSystem::applySolution(const Solution &solution) { } for (auto param : solution.isolatedParams) { - isolatedParams.insert(param); + if (isolatedParams.count(param) == 0) + recordIsolatedParam(param); } for (auto &pair : solution.exprPatterns) { @@ -697,7 +698,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); numKeyPaths = cs.KeyPaths.size(); - numIsolatedParams = cs.isolatedParams.size(); numPreconcurrencyClosures = cs.preconcurrencyClosures.size(); numImplicitValueConversions = cs.ImplicitValueConversions.size(); numArgumentLists = cs.ArgumentLists.size(); @@ -738,9 +738,6 @@ ConstraintSystem::SolverScope::~SolverScope() { /// Remove any key path expressions. truncate(cs.KeyPaths, numKeyPaths); - // Remove any isolated parameters. - truncate(cs.isolatedParams, numIsolatedParams); - // Remove any preconcurrency closures. truncate(cs.preconcurrencyClosures, numPreconcurrencyClosures); diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index 85c5725f4466b..2bd8af83682fc 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -319,6 +319,14 @@ SolverTrail::Change::RecordedPotentialThrowSite(CatchNode catchNode) { return result; } +SolverTrail::Change +SolverTrail::Change::RecordedIsolatedParam(ParamDecl *param) { + Change result; + result.Kind = ChangeKind::RecordedIsolatedParam; + result.TheParam = param; + return result; +} + SyntacticElementTargetKey SolverTrail::Change::getSyntacticElementTargetKey() const { ASSERT(Kind == ChangeKind::RecordedTarget); @@ -493,6 +501,10 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { case ChangeKind::RecordedExprPattern: cs.removeExprPatternFor(TheExpr); break; + + case ChangeKind::RecordedIsolatedParam: + cs.removeIsolatedParam(TheParam); + break; } } @@ -707,6 +719,12 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, // FIXME: Print something here out << "(RecordedPotentialThrowSite)\n"; break; + + case ChangeKind::RecordedIsolatedParam: + out << "(RecordedIsolatedParam "; + TheParam->dumpRef(out); + out << ")\n"; + break; } } diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 190a8add556ce..e06bfc2ab15a3 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -4495,7 +4495,7 @@ size_t Solution::getTotalMemory() const { size_in_bytes(targets) + size_in_bytes(caseLabelItems) + size_in_bytes(exprPatterns) + - (isolatedParams.size() * sizeof(void *)) + + size_in_bytes(isolatedParams) + (preconcurrencyClosures.size() * sizeof(void *)) + size_in_bytes(resultBuilderTransformed) + size_in_bytes(appliedPropertyWrappers) + From ac17292dfe7976bec5a7720a90a668ea91c53284 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 4 Oct 2024 14:50:22 -0400 Subject: [PATCH 21/26] Sema: Record preconcurrency closures in the trail --- include/swift/Sema/CSTrail.def | 9 +++++++- include/swift/Sema/CSTrail.h | 4 +--- include/swift/Sema/ConstraintSystem.h | 14 +++++++----- lib/Sema/CSSimplify.cpp | 19 +++++++++++++++- lib/Sema/CSSolver.cpp | 11 ++++----- lib/Sema/CSSyntacticElement.cpp | 2 +- lib/Sema/CSTrail.cpp | 32 +++++++++++++++------------ lib/Sema/ConstraintSystem.cpp | 2 +- 8 files changed, 60 insertions(+), 33 deletions(-) diff --git a/include/swift/Sema/CSTrail.def b/include/swift/Sema/CSTrail.def index 24d6f58dbee0d..258b9a2a0bb42 100644 --- a/include/swift/Sema/CSTrail.def +++ b/include/swift/Sema/CSTrail.def @@ -26,6 +26,10 @@ #define EXPR_CHANGE(Name) CHANGE(Name) #endif +#ifndef CLOSURE_CHANGE +#define CLOSURE_CHANGE(Name) CHANGE(Name) +#endif + #ifndef LAST_CHANGE #define LAST_CHANGE(Name) #endif @@ -42,6 +46,9 @@ EXPR_CHANGE(AppliedPropertyWrapper) EXPR_CHANGE(RecordedImpliedResult) EXPR_CHANGE(RecordedExprPattern) +CLOSURE_CHANGE(RecordedClosureType) +CLOSURE_CHANGE(RecordedPreconcurrencyClosure) + CHANGE(AddedTypeVariable) CHANGE(AddedConstraint) CHANGE(RemovedConstraint) @@ -61,7 +68,6 @@ CHANGE(RecordedKeyPathComponentType) CHANGE(DisabledConstraint) CHANGE(FavoredConstraint) CHANGE(RecordedResultBuilderTransform) -CHANGE(RecordedClosureType) CHANGE(RecordedContextualInfo) CHANGE(RecordedTarget) CHANGE(RecordedCaseLabelItemInfo) @@ -72,5 +78,6 @@ LAST_CHANGE(RecordedIsolatedParam) #undef LOCATOR_CHANGE #undef EXPR_CHANGE +#undef CLOSURE_CHANGE #undef LAST_CHANGE #undef CHANGE \ No newline at end of file diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index f461e9cabf421..314011a284463 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -141,6 +141,7 @@ class SolverTrail { #define LOCATOR_CHANGE(Name) static Change Name(ConstraintLocator *locator); #define EXPR_CHANGE(Name) static Change Name(Expr *expr); +#define CLOSURE_CHANGE(Name) static Change Name(ClosureExpr *closure); #include "swift/Sema/CSTrail.def" /// Create a change that added a type variable. @@ -214,9 +215,6 @@ class SolverTrail { /// Create a change that recorded a result builder transform. static Change RecordedResultBuilderTransform(AnyFunctionRef fn); - /// Create a change that recorded a closure type. - static Change RecordedClosureType(ClosureExpr *closure); - /// Create a change that recorded the contextual type of an AST node. static Change RecordedContextualInfo(ASTNode node); diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 031a366f2bd6f..ceb24ef9c5493 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -1598,7 +1598,7 @@ class Solution { /// The set of closures that have been inferred to be "isolated by /// preconcurrency". - std::vector preconcurrencyClosures; + llvm::DenseSet preconcurrencyClosures; /// The set of functions that have been transformed by a result builder. llvm::MapVector @@ -2296,7 +2296,7 @@ class ConstraintSystem { /// The set of closures that have been inferred to be "isolated by /// preconcurrency". - llvm::SmallSetVector preconcurrencyClosures; + llvm::SmallDenseSet preconcurrencyClosures; /// Maps closure parameters to type variables. llvm::DenseMap @@ -2855,9 +2855,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - /// The length of \c PreconcurrencyClosures. - unsigned numPreconcurrencyClosures; - /// The length of \c ImplicitValueConversions. unsigned numImplicitValueConversions; @@ -4219,6 +4216,13 @@ class ConstraintSystem { /// Undo the above change. void removeIsolatedParam(ParamDecl *param); + /// Used by the above to update preconcurrencyClosures and record a change in + /// the trail. + void recordPreconcurrencyClosure(const ClosureExpr *closure); + + /// Undo the above change. + void removePreconcurrencyClosure(const ClosureExpr *closure); + /// Given the fact that contextual type is now available for the type /// variable representing a pack expansion type, let's resolve the expansion. /// diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index b0ffb967f6b05..70b4e1e002da6 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -11526,6 +11526,23 @@ void ConstraintSystem::removeIsolatedParam(ParamDecl *param) { ASSERT(erased); } +void ConstraintSystem::recordPreconcurrencyClosure( + const ClosureExpr *closure) { + bool inserted = preconcurrencyClosures.insert(closure).second; + ASSERT(inserted); + + if (solverState) { + recordChange(SolverTrail::Change::RecordedPreconcurrencyClosure( + const_cast(closure))); + } +} + +void ConstraintSystem::removePreconcurrencyClosure( + const ClosureExpr *closure) { + bool erased = preconcurrencyClosures.erase(closure); + ASSERT(erased); +} + bool ConstraintSystem::resolveClosure(TypeVariableType *typeVar, Type contextualType, ConstraintLocatorBuilder locator) { @@ -11535,7 +11552,7 @@ bool ConstraintSystem::resolveClosure(TypeVariableType *typeVar, // Note if this closure is isolated by preconcurrency. if (hasPreconcurrencyCallee(locator)) - preconcurrencyClosures.insert(closure); + recordPreconcurrencyClosure(closure); // Let's look through all optionals associated with contextual // type to make it possible to infer parameter/result type of diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index f39aecac7170c..6fa1dbc9c20ac 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -233,8 +233,8 @@ Solution ConstraintSystem::finalize() { for (const auto ¶m : isolatedParams) solution.isolatedParams.insert(param); - for (const auto &closure : preconcurrencyClosures) - solution.preconcurrencyClosures.push_back(closure); + for (auto closure : preconcurrencyClosures) + solution.preconcurrencyClosures.insert(closure); for (const auto &transformed : resultBuilderTransformed) { solution.resultBuilderTransformed.insert(transformed); @@ -416,7 +416,8 @@ void ConstraintSystem::applySolution(const Solution &solution) { } for (auto closure : solution.preconcurrencyClosures) { - preconcurrencyClosures.insert(closure); + if (preconcurrencyClosures.count(closure) == 0) + recordPreconcurrencyClosure(closure); } for (const auto &transformed : solution.resultBuilderTransformed) { @@ -698,7 +699,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); numKeyPaths = cs.KeyPaths.size(); - numPreconcurrencyClosures = cs.preconcurrencyClosures.size(); numImplicitValueConversions = cs.ImplicitValueConversions.size(); numArgumentLists = cs.ArgumentLists.size(); numImplicitCallAsFunctionRoots = cs.ImplicitCallAsFunctionRoots.size(); @@ -738,9 +738,6 @@ ConstraintSystem::SolverScope::~SolverScope() { /// Remove any key path expressions. truncate(cs.KeyPaths, numKeyPaths); - // Remove any preconcurrency closures. - truncate(cs.preconcurrencyClosures, numPreconcurrencyClosures); - // Remove any implicit value conversions. truncate(cs.ImplicitValueConversions, numImplicitValueConversions); diff --git a/lib/Sema/CSSyntacticElement.cpp b/lib/Sema/CSSyntacticElement.cpp index 9f444422d8de5..10e87eeab0517 100644 --- a/lib/Sema/CSSyntacticElement.cpp +++ b/lib/Sema/CSSyntacticElement.cpp @@ -2581,7 +2581,7 @@ bool ConstraintSystem::applySolution(AnyFunctionRef fn, param->setIsolated(true); } - if (llvm::is_contained(solution.preconcurrencyClosures, closure)) + if (solution.preconcurrencyClosures.count(closure)) closure->setIsolatedByPreconcurrency(); // Coerce the result type, if it was written explicitly. diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index 2bd8af83682fc..8eb1bf11d4ec3 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -64,6 +64,14 @@ SolverTrail::~SolverTrail() { result.TheExpr = expr; \ return result; \ } +#define CLOSURE_CHANGE(Name) \ + SolverTrail::Change \ + SolverTrail::Change::Name(ClosureExpr *closure) { \ + Change result; \ + result.Kind = ChangeKind::Name; \ + result.TheClosure = closure; \ + return result; \ + } #include "swift/Sema/CSTrail.def" SolverTrail::Change @@ -247,14 +255,6 @@ SolverTrail::Change::RecordedResultBuilderTransform(AnyFunctionRef fn) { return result; } -SolverTrail::Change -SolverTrail::Change::RecordedClosureType(ClosureExpr *closure) { - Change result; - result.Kind = ChangeKind::RecordedClosureType; - result.TheClosure = closure; - return result; -} - SolverTrail::Change SolverTrail::Change::RecordedContextualInfo(ASTNode node) { Change result; @@ -505,6 +505,10 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { case ChangeKind::RecordedIsolatedParam: cs.removeIsolatedParam(TheParam); break; + + case ChangeKind::RecordedPreconcurrencyClosure: + cs.removePreconcurrencyClosure(TheClosure); + break; } } @@ -530,6 +534,12 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, simple_display(out, TheExpr); \ out << ")\n"; \ break; +#define CLOSURE_CHANGE(Name) \ + case ChangeKind::Name: \ + out << "(" << #Name << " "; \ + simple_display(out, TheClosure); \ + out << ")\n"; \ + break; #include "swift/Sema/CSTrail.def" case ChangeKind::AddedTypeVariable: @@ -693,12 +703,6 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, out << ")\n"; break; - case ChangeKind::RecordedClosureType: - out << "(RecordedClosureType "; - simple_display(out, TheClosure); - out << ")\n"; - break; - case ChangeKind::RecordedContextualInfo: // FIXME: Print short form of ASTNode out << "(RecordedContextualInfo)\n"; diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index e06bfc2ab15a3..590559eb953f8 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -4496,7 +4496,7 @@ size_t Solution::getTotalMemory() const { size_in_bytes(caseLabelItems) + size_in_bytes(exprPatterns) + size_in_bytes(isolatedParams) + - (preconcurrencyClosures.size() * sizeof(void *)) + + size_in_bytes(preconcurrencyClosures) + size_in_bytes(resultBuilderTransformed) + size_in_bytes(appliedPropertyWrappers) + size_in_bytes(argumentLists) + From 881a0100b2d46a2dd87225be7fca19697ceb1bf3 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 4 Oct 2024 15:16:54 -0400 Subject: [PATCH 22/26] Sema: Record implicit value conversions in the trail --- include/swift/Sema/CSTrail.def | 19 +++++---- include/swift/Sema/CSTrail.h | 6 +-- include/swift/Sema/ConstraintSystem.h | 49 +++------------------ lib/Sema/CSSimplify.cpp | 11 +++++ lib/Sema/CSSolver.cpp | 9 ++-- lib/Sema/CSTrail.cpp | 61 +++++---------------------- lib/Sema/ConstraintSystem.cpp | 12 +----- 7 files changed, 42 insertions(+), 125 deletions(-) diff --git a/include/swift/Sema/CSTrail.def b/include/swift/Sema/CSTrail.def index 258b9a2a0bb42..b99373a843668 100644 --- a/include/swift/Sema/CSTrail.def +++ b/include/swift/Sema/CSTrail.def @@ -19,7 +19,7 @@ #endif #ifndef LOCATOR_CHANGE -#define LOCATOR_CHANGE(Name) CHANGE(Name) +#define LOCATOR_CHANGE(Name, Map) CHANGE(Name) #endif #ifndef EXPR_CHANGE @@ -34,13 +34,15 @@ #define LAST_CHANGE(Name) #endif -LOCATOR_CHANGE(RecordedAppliedDisjunction) -LOCATOR_CHANGE(RecordedMatchCallArgumentResult) -LOCATOR_CHANGE(RecordedOpenedTypes) -LOCATOR_CHANGE(RecordedOpenedExistentialType) -LOCATOR_CHANGE(RecordedPackExpansionEnvironment) -LOCATOR_CHANGE(RecordedDefaultedConstraint) -LOCATOR_CHANGE(ResolvedOverload) +LOCATOR_CHANGE(RecordedDisjunctionChoice, DisjunctionChoices) +LOCATOR_CHANGE(RecordedAppliedDisjunction, AppliedDisjunctions) +LOCATOR_CHANGE(RecordedMatchCallArgumentResult, argumentMatchingChoices) +LOCATOR_CHANGE(RecordedOpenedTypes, OpenedTypes) +LOCATOR_CHANGE(RecordedOpenedExistentialType, OpenedExistentialTypes) +LOCATOR_CHANGE(RecordedPackExpansionEnvironment, PackExpansionEnvironments) +LOCATOR_CHANGE(RecordedDefaultedConstraint, DefaultedConstraints) +LOCATOR_CHANGE(ResolvedOverload, ResolvedOverloads) +LOCATOR_CHANGE(RecordedImplicitValueConversion, ImplicitValueConversions) EXPR_CHANGE(AppliedPropertyWrapper) EXPR_CHANGE(RecordedImpliedResult) @@ -60,7 +62,6 @@ CHANGE(UpdatedTypeVariable) CHANGE(AddedConversionRestriction) CHANGE(AddedFix) CHANGE(AddedFixedRequirement) -CHANGE(RecordedDisjunctionChoice) CHANGE(RecordedOpenedPackExpansionType) CHANGE(RecordedPackEnvironment) CHANGE(RecordedNodeType) diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index 314011a284463..04e4e352ceba2 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -139,7 +139,7 @@ class SolverTrail { Change() : Kind(ChangeKind::AddedTypeVariable), TypeVar(nullptr) { } -#define LOCATOR_CHANGE(Name) static Change Name(ConstraintLocator *locator); +#define LOCATOR_CHANGE(Name, _) static Change Name(ConstraintLocator *locator); #define EXPR_CHANGE(Name) static Change Name(Expr *expr); #define CLOSURE_CHANGE(Name) static Change Name(ClosureExpr *closure); #include "swift/Sema/CSTrail.def" @@ -187,10 +187,6 @@ class SolverTrail { unsigned reqKind, Type requirementTy); - /// Create a change that recorded a disjunction choice. - static Change RecordedDisjunctionChoice(ConstraintLocator *locator, - unsigned index); - /// Create a change that recorded the opening of a pack expansion type. static Change RecordedOpenedPackExpansionType(PackExpansionType *expansion); diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index ceb24ef9c5493..e4cbc625976a8 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2331,7 +2331,7 @@ class ConstraintSystem { /// The set of implicit value conversions performed by the solver on /// a current path to reach a solution. - llvm::SmallMapVector + llvm::SmallDenseMap ImplicitValueConversions; /// The worklist of "active" constraints that should be revisited @@ -2415,11 +2415,6 @@ class ConstraintSystem { } } - void removeDefaultedConstraint(ConstraintLocator *locator) { - bool erased = DefaultedConstraints.erase(locator); - ASSERT(erased); - } - /// A cache that stores the @dynamicCallable required methods implemented by /// types. llvm::DenseMap @@ -2855,9 +2850,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - /// The length of \c ImplicitValueConversions. - unsigned numImplicitValueConversions; - /// The length of \c KeyPaths. unsigned numKeyPaths; @@ -3453,12 +3445,6 @@ class ConstraintSystem { void recordOpenedExistentialType(ConstraintLocator *locator, OpenedArchetypeType *opened); - /// Undo the above change. - void removeOpenedExistentialType(ConstraintLocator *locator) { - bool erased = OpenedExistentialTypes.erase(locator); - ASSERT(erased); - } - /// Get the opened element generic environment for the given locator. GenericEnvironment *getPackElementEnvironment(ConstraintLocator *locator, CanType shapeClass); @@ -3467,12 +3453,6 @@ class ConstraintSystem { void recordPackExpansionEnvironment(ConstraintLocator *locator, std::pair uuidAndShape); - /// Undo the above change. - void removePackExpansionEnvironment(ConstraintLocator *locator) { - bool erased = PackExpansionEnvironments.erase(locator); - ASSERT(erased); - } - /// Get the opened element generic environment for the given pack element. PackExpansionExpr *getPackEnvironment(PackElementExpr *packElement) const; @@ -3655,12 +3635,6 @@ class ConstraintSystem { void recordMatchCallArgumentResult(ConstraintLocator *locator, MatchCallArgumentResult result); - /// Undo the above change. - void removeMatchCallArgumentResult(ConstraintLocator *locator) { - bool erased = argumentMatchingChoices.erase(locator); - ASSERT(erased); - } - /// Record implicitly generated `callAsFunction` with root at the /// given expression, located at \c locator. void recordCallAsFunction(UnresolvedDotExpr *root, ArgumentList *arguments, @@ -4438,9 +4412,6 @@ class ConstraintSystem { void recordOpenedType( ConstraintLocator *locator, ArrayRef openedTypes); - /// Undo the above change. - void removeOpenedType(ConstraintLocator *locator); - /// Record the set of opened types for the given locator. void recordOpenedTypes( ConstraintLocatorBuilder locator, @@ -4962,8 +4933,6 @@ class ConstraintSystem { void recordResolvedOverload(ConstraintLocator *locator, SelectedOverload choice); - void removeResolvedOverload(ConstraintLocator *locator); - /// Resolve the given overload set to the given choice. void resolveOverload(ConstraintLocator *locator, Type boundType, OverloadChoice choice, DeclContext *useDC); @@ -5253,6 +5222,10 @@ class ConstraintSystem { TypeMatchOptions flags, ConstraintLocatorBuilder locator); + /// Update ImplicitValueConversions and record a change in the trail. + void recordImplicitValueConversion(ConstraintLocator *locator, + ConversionRestrictionKind restriction); + /// Simplify a conversion constraint by applying the given /// reduction rule, which is known to apply at the outermost level. SolutionKind simplifyRestrictedConstraint( @@ -5393,22 +5366,10 @@ class ConstraintSystem { /// Record a particular disjunction choice and add a change to the trail. void recordDisjunctionChoice(ConstraintLocator *locator, unsigned index); - /// Undo the above change. - void removeDisjunctionChoice(ConstraintLocator *locator) { - bool erased = DisjunctionChoices.erase(locator); - ASSERT(erased); - } - /// Record applied disjunction and add a change to the trail. void recordAppliedDisjunction(ConstraintLocator *locator, FunctionType *type); - /// Undo the above change. - void removeAppliedDisjunction(ConstraintLocator *locator) { - bool erased = AppliedDisjunctions.erase(locator); - ASSERT(erased); - } - /// Filter the set of disjunction terms, keeping only those where the /// predicate returns \c true. /// diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 70b4e1e002da6..151d36769b3f4 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -14082,6 +14082,17 @@ void ConstraintSystem::addRestrictedConstraint( TMF_GenerateConstraints, locator); } +void ConstraintSystem::recordImplicitValueConversion( + ConstraintLocator *locator, + ConversionRestrictionKind restriction) { + bool inserted = ImplicitValueConversions.insert( + {getConstraintLocator(locator), restriction}).second; + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::RecordedImplicitValueConversion(locator)); +} + /// Given that we have a conversion constraint between two types, and /// that the given constraint-reduction rule applies between them at /// the top level, apply it and generate any necessary recursive diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 6fa1dbc9c20ac..a141a00f1cb62 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -437,7 +437,10 @@ void ConstraintSystem::applySolution(const Solution &solution) { } for (auto &valueConversion : solution.ImplicitValueConversions) { - ImplicitValueConversions.insert(valueConversion); + if (ImplicitValueConversions.count(valueConversion.first) == 0) { + recordImplicitValueConversion(valueConversion.first, + valueConversion.second); + } } // Register the argument lists. @@ -699,7 +702,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); numKeyPaths = cs.KeyPaths.size(); - numImplicitValueConversions = cs.ImplicitValueConversions.size(); numArgumentLists = cs.ArgumentLists.size(); numImplicitCallAsFunctionRoots = cs.ImplicitCallAsFunctionRoots.size(); numSynthesizedConformances = cs.SynthesizedConformances.size(); @@ -738,9 +740,6 @@ ConstraintSystem::SolverScope::~SolverScope() { /// Remove any key path expressions. truncate(cs.KeyPaths, numKeyPaths); - // Remove any implicit value conversions. - truncate(cs.ImplicitValueConversions, numImplicitValueConversions); - // Remove any argument lists no longer in scope. truncate(cs.ArgumentLists, numArgumentLists); diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index 8eb1bf11d4ec3..c315003468a85 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -48,7 +48,7 @@ SolverTrail::~SolverTrail() { ASSERT(Changes.empty() && "Trail corrupted"); } -#define LOCATOR_CHANGE(Name) \ +#define LOCATOR_CHANGE(Name, _) \ SolverTrail::Change \ SolverTrail::Change::Name(ConstraintLocator *locator) { \ Change result; \ @@ -184,16 +184,6 @@ SolverTrail::Change::AddedFixedRequirement(GenericTypeParamType *GP, return result; } -SolverTrail::Change -SolverTrail::Change::RecordedDisjunctionChoice(ConstraintLocator *locator, - unsigned index) { - Change result; - result.Kind = ChangeKind::RecordedDisjunctionChoice; - result.TheLocator = locator; - result.Options = index; - return result; -} - SolverTrail::Change SolverTrail::Change::RecordedOpenedPackExpansionType(PackExpansionType *expansionTy) { Change result; @@ -360,6 +350,14 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { auto &cg = cs.getConstraintGraph(); switch (Kind) { +#define LOCATOR_CHANGE(Name, Map) \ + case ChangeKind::Name: { \ + bool erased = cs.Map.erase(TheLocator); \ + ASSERT(erased); \ + break; \ + } +#include "swift/Sema/CSTrail.def" + case ChangeKind::AddedTypeVariable: cg.removeNode(TypeVar); break; @@ -409,42 +407,14 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { FixedRequirement.ReqTy); break; - case ChangeKind::RecordedDisjunctionChoice: - cs.removeDisjunctionChoice(TheLocator); - break; - - case ChangeKind::RecordedAppliedDisjunction: - cs.removeAppliedDisjunction(TheLocator); - break; - - case ChangeKind::RecordedMatchCallArgumentResult: - cs.removeMatchCallArgumentResult(TheLocator); - break; - - case ChangeKind::RecordedOpenedTypes: - cs.removeOpenedType(TheLocator); - break; - - case ChangeKind::RecordedOpenedExistentialType: - cs.removeOpenedExistentialType(TheLocator); - break; - case ChangeKind::RecordedOpenedPackExpansionType: cs.removeOpenedPackExpansionType(TheExpansion); break; - case ChangeKind::RecordedPackExpansionEnvironment: - cs.removePackExpansionEnvironment(TheLocator); - break; - case ChangeKind::RecordedPackEnvironment: cs.removePackEnvironment(TheElement); break; - case ChangeKind::RecordedDefaultedConstraint: - cs.removeDefaultedConstraint(TheLocator); - break; - case ChangeKind::RecordedNodeType: cs.restoreType(Node.Node, Node.OldType); break; @@ -470,10 +440,6 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { cs.removePropertyWrapper(TheExpr); break; - case ChangeKind::ResolvedOverload: - cs.removeResolvedOverload(TheLocator); - break; - case ChangeKind::RecordedClosureType: cs.removeClosureType(TheClosure); break; @@ -522,7 +488,7 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, switch (Kind) { -#define LOCATOR_CHANGE(Name) \ +#define LOCATOR_CHANGE(Name, _) \ case ChangeKind::Name: \ out << "(" << #Name << " at "; \ TheLocator->dump(&cs.getASTContext().SourceMgr, out); \ @@ -641,13 +607,6 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, out << ")\n"; break; - case ChangeKind::RecordedDisjunctionChoice: - out << "(RecordedDisjunctionChoice at "; - TheLocator->dump(&cs.getASTContext().SourceMgr, out); - out << " index "; - out << Options << ")\n"; - break; - case ChangeKind::RecordedOpenedPackExpansionType: out << "(RecordedOpenedPackExpansionType for "; TheExpansion->print(out, PO); diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 590559eb953f8..54047294ca0bc 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -298,7 +298,7 @@ void ConstraintSystem::recordDisjunctionChoice( ASSERT(inserted); if (solverState) - recordChange(SolverTrail::Change::RecordedDisjunctionChoice(locator, index)); + recordChange(SolverTrail::Change::RecordedDisjunctionChoice(locator)); } void ConstraintSystem::recordAppliedDisjunction( @@ -1691,11 +1691,6 @@ void ConstraintSystem::recordOpenedType( recordChange(SolverTrail::Change::RecordedOpenedTypes(locator)); } -void ConstraintSystem::removeOpenedType(ConstraintLocator *locator) { - bool erased = OpenedTypes.erase(locator); - ASSERT(erased); -} - void ConstraintSystem::recordOpenedTypes( ConstraintLocatorBuilder locator, const OpenedTypeMap &replacements, @@ -3754,11 +3749,6 @@ void ConstraintSystem::recordResolvedOverload(ConstraintLocator *locator, recordChange(SolverTrail::Change::ResolvedOverload(locator)); } -void ConstraintSystem::removeResolvedOverload(ConstraintLocator *locator) { - bool erased = ResolvedOverloads.erase(locator); - ASSERT(erased); -} - void ConstraintSystem::resolveOverload(ConstraintLocator *locator, Type boundType, OverloadChoice choice, From f2412f318e3f4beb4a4d4000975e4dea4f99a3ae Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 4 Oct 2024 15:23:04 -0400 Subject: [PATCH 23/26] Sema: Record key path expressions in the trail --- include/swift/Sema/CSTrail.def | 3 ++- include/swift/Sema/CSTrail.h | 3 +++ include/swift/Sema/ConstraintSystem.h | 22 +++++++++++----------- lib/Sema/CSSimplify.cpp | 16 ++++++++++++++-- lib/Sema/CSSolver.cpp | 11 ++++++----- lib/Sema/CSTrail.cpp | 18 ++++++++++++++++++ 6 files changed, 54 insertions(+), 19 deletions(-) diff --git a/include/swift/Sema/CSTrail.def b/include/swift/Sema/CSTrail.def index b99373a843668..c53081c014542 100644 --- a/include/swift/Sema/CSTrail.def +++ b/include/swift/Sema/CSTrail.def @@ -74,8 +74,9 @@ CHANGE(RecordedTarget) CHANGE(RecordedCaseLabelItemInfo) CHANGE(RecordedPotentialThrowSite) CHANGE(RecordedIsolatedParam) +CHANGE(RecordedKeyPath) -LAST_CHANGE(RecordedIsolatedParam) +LAST_CHANGE(RecordedKeyPath) #undef LOCATOR_CHANGE #undef EXPR_CHANGE diff --git a/include/swift/Sema/CSTrail.h b/include/swift/Sema/CSTrail.h index 04e4e352ceba2..62488de609685 100644 --- a/include/swift/Sema/CSTrail.h +++ b/include/swift/Sema/CSTrail.h @@ -226,6 +226,9 @@ class SolverTrail { /// Create a change that recorded an isolated parameter. static Change RecordedIsolatedParam(ParamDecl *param); + /// Create a change that recorded a key path expression. + static Change RecordedKeyPath(KeyPathExpr *expr); + /// Undo this change, reverting the constraint graph to the state it /// had prior to this change. /// diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index e4cbc625976a8..6203360c59289 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -1567,9 +1567,9 @@ class Solution { /// The key path expression and its root type, value type, and decl context /// introduced by this solution. - llvm::MapVector> + llvm::DenseMap> KeyPaths; /// Contextual types introduced by this solution. @@ -2265,9 +2265,9 @@ class ConstraintSystem { KeyPathComponentTypes; /// Maps a key path root, value, and decl context to the key path expression. - llvm::MapVector> + llvm::DenseMap> KeyPaths; /// Maps AST entries to their targets. @@ -2850,9 +2850,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - /// The length of \c KeyPaths. - unsigned numKeyPaths; - /// The length of \c ArgumentLists. unsigned numArgumentLists; @@ -3641,10 +3638,13 @@ class ConstraintSystem { ConstraintLocator *locator); /// Record root, value, and declContext of keypath expression for use across - /// constraint system. - void recordKeyPath(KeyPathExpr *keypath, TypeVariableType *root, + /// constraint system, and add a change to the trail. + void recordKeyPath(const KeyPathExpr *keypath, TypeVariableType *root, TypeVariableType *value, DeclContext *dc); + /// Undo the above change. + void removeKeyPath(const KeyPathExpr *keypath); + /// Walk a closure AST to determine its effects. /// /// \returns a function's extended info describing the effects, as diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 151d36769b3f4..a441eb2217ed2 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -14983,10 +14983,22 @@ void ConstraintSystem::recordCallAsFunction(UnresolvedDotExpr *root, getConstraintLocator(root, ConstraintLocator::ApplyArgument), arguments); } -void ConstraintSystem::recordKeyPath(KeyPathExpr *keypath, +void ConstraintSystem::recordKeyPath(const KeyPathExpr *keypath, TypeVariableType *root, TypeVariableType *value, DeclContext *dc) { - KeyPaths.insert(std::make_pair(keypath, std::make_tuple(root, value, dc))); + bool inserted = KeyPaths.insert( + std::make_pair(keypath, std::make_tuple(root, value, dc))).second; + ASSERT(inserted); + + if (solverState) { + recordChange(SolverTrail::Change::RecordedKeyPath( + const_cast(keypath))); + } +} + +void ConstraintSystem::removeKeyPath(const KeyPathExpr *keypath) { + bool erased = KeyPaths.erase(keypath); + ASSERT(erased); } ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index a141a00f1cb62..a4cd7fe39d0fc 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -378,7 +378,12 @@ void ConstraintSystem::applySolution(const Solution &solution) { // Add key paths. for (const auto &keypath : solution.KeyPaths) { - KeyPaths.insert(keypath); + if (KeyPaths.count(keypath.first) == 0) { + recordKeyPath(keypath.first, + std::get<0>(keypath.second), + std::get<1>(keypath.second), + std::get<2>(keypath.second)); + } } // Add the contextual types. @@ -701,7 +706,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); - numKeyPaths = cs.KeyPaths.size(); numArgumentLists = cs.ArgumentLists.size(); numImplicitCallAsFunctionRoots = cs.ImplicitCallAsFunctionRoots.size(); numSynthesizedConformances = cs.SynthesizedConformances.size(); @@ -737,9 +741,6 @@ ConstraintSystem::SolverScope::~SolverScope() { // constraints introduced by the current scope. cs.solverState->rollback(this); - /// Remove any key path expressions. - truncate(cs.KeyPaths, numKeyPaths); - // Remove any argument lists no longer in scope. truncate(cs.ArgumentLists, numArgumentLists); diff --git a/lib/Sema/CSTrail.cpp b/lib/Sema/CSTrail.cpp index c315003468a85..6f9ea91605125 100644 --- a/lib/Sema/CSTrail.cpp +++ b/lib/Sema/CSTrail.cpp @@ -317,6 +317,14 @@ SolverTrail::Change::RecordedIsolatedParam(ParamDecl *param) { return result; } +SolverTrail::Change +SolverTrail::Change::RecordedKeyPath(KeyPathExpr *expr) { + Change result; + result.Kind = ChangeKind::RecordedKeyPath; + result.KeyPath.Expr = expr; + return result; +} + SyntacticElementTargetKey SolverTrail::Change::getSyntacticElementTargetKey() const { ASSERT(Kind == ChangeKind::RecordedTarget); @@ -475,6 +483,10 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const { case ChangeKind::RecordedPreconcurrencyClosure: cs.removePreconcurrencyClosure(TheClosure); break; + + case ChangeKind::RecordedKeyPath: + cs.removeKeyPath(KeyPath.Expr); + break; } } @@ -688,6 +700,12 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out, TheParam->dumpRef(out); out << ")\n"; break; + + case ChangeKind::RecordedKeyPath: + out << "(RecordedKeyPath "; + simple_display(out, KeyPath.Expr); + out << ")\n"; + break; } } From 72a60728fcc648946f141a40c3413a69c215dc09 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 4 Oct 2024 17:17:42 -0400 Subject: [PATCH 24/26] Sema: Record argument lists in the trail --- include/swift/Sema/CSTrail.def | 1 + include/swift/Sema/ConstraintSystem.h | 12 +++++++----- lib/Sema/CSSimplify.cpp | 2 +- lib/Sema/CSSolver.cpp | 7 ++----- lib/Sema/ConstraintSystem.cpp | 24 ++++++++++++++++-------- 5 files changed, 27 insertions(+), 19 deletions(-) diff --git a/include/swift/Sema/CSTrail.def b/include/swift/Sema/CSTrail.def index c53081c014542..99b096392aeaa 100644 --- a/include/swift/Sema/CSTrail.def +++ b/include/swift/Sema/CSTrail.def @@ -43,6 +43,7 @@ LOCATOR_CHANGE(RecordedPackExpansionEnvironment, PackExpansionEnvironments) LOCATOR_CHANGE(RecordedDefaultedConstraint, DefaultedConstraints) LOCATOR_CHANGE(ResolvedOverload, ResolvedOverloads) LOCATOR_CHANGE(RecordedImplicitValueConversion, ImplicitValueConversions) +LOCATOR_CHANGE(RecordedArgumentList, ArgumentLists) EXPR_CHANGE(AppliedPropertyWrapper) EXPR_CHANGE(RecordedImpliedResult) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 6203360c59289..5a7b9f30e7cf2 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -1610,7 +1610,7 @@ class Solution { /// A mapping from the constraint locators for references to various /// names (e.g., member references, normal name references, possible /// constructions) to the argument lists for the call to that locator. - llvm::MapVector argumentLists; + llvm::DenseMap argumentLists; /// The set of implicitly generated `.callAsFunction` root expressions. llvm::DenseMap @@ -2397,7 +2397,7 @@ class ConstraintSystem { /// A mapping from the constraint locators for references to various /// names (e.g., member references, normal name references, possible /// constructions) to the argument lists for the call to that locator. - llvm::MapVector ArgumentLists; + llvm::DenseMap ArgumentLists; public: /// A map from argument expressions to their applied property wrapper expressions. @@ -2801,6 +2801,11 @@ class ConstraintSystem { /// Associate an argument list with a call at a given locator. void associateArgumentList(ConstraintLocator *locator, ArgumentList *args); + /// Same as associateArgumentList() except the locator points at the + /// argument list itself. Records a change in the trail. + void recordArgumentList(ConstraintLocator *locator, + ArgumentList *args); + /// If the given node is a function expression with a parent ApplyExpr, /// returns the apply, otherwise returns the node itself. ASTNode includingParentApply(ASTNode node); @@ -2850,9 +2855,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - /// The length of \c ArgumentLists. - unsigned numArgumentLists; - /// The length of \c ImplicitCallAsFunctionRoots. unsigned numImplicitCallAsFunctionRoots; diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index a441eb2217ed2..f4a9a6fbc9066 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -14665,7 +14665,7 @@ ConstraintSystem::simplifyRestrictedConstraintImpl( getASTContext(), {Argument(SourceLoc(), Identifier(), nullptr)}, /*firstTrailingClosureIndex=*/std::nullopt, AllocationArena::ConstraintSolver); - ArgumentLists.insert({argumentsLoc, argList}); + recordArgumentList(argumentsLoc, argList); } auto *memberTypeLoc = getConstraintLocator( diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index a4cd7fe39d0fc..9a11df9b5d347 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -450,7 +450,8 @@ void ConstraintSystem::applySolution(const Solution &solution) { // Register the argument lists. for (auto &argListMapping : solution.argumentLists) { - ArgumentLists.insert(argListMapping); + if (ArgumentLists.count(argListMapping.first) == 0) + recordArgumentList(argListMapping.first, argListMapping.second); } for (auto &implicitRoot : solution.ImplicitCallAsFunctionRoots) { @@ -706,7 +707,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); - numArgumentLists = cs.ArgumentLists.size(); numImplicitCallAsFunctionRoots = cs.ImplicitCallAsFunctionRoots.size(); numSynthesizedConformances = cs.SynthesizedConformances.size(); @@ -741,9 +741,6 @@ ConstraintSystem::SolverScope::~SolverScope() { // constraints introduced by the current scope. cs.solverState->rollback(this); - // Remove any argument lists no longer in scope. - truncate(cs.ArgumentLists, numArgumentLists); - // Remove any implicitly generated root expressions for `.callAsFunction` // which are no longer in scope. truncate(cs.ImplicitCallAsFunctionRoots, numImplicitCallAsFunctionRoots); diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 54047294ca0bc..7dc24376fd19d 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -3542,12 +3542,13 @@ void ConstraintSystem::bindOverloadType( // Associate an argument list for the implicit x[dynamicMember:] subscript // if we haven't already. - auto *&argList = ArgumentLists[getArgumentInfoLocator(callLoc)]; - if (!argList) { - argList = ArgumentList::createImplicit( + auto *argLoc = getArgumentInfoLocator(callLoc); + if (ArgumentLists.find(argLoc) == ArgumentLists.end()) { + auto *argList = ArgumentList::createImplicit( ctx, {Argument(SourceLoc(), ctx.Id_dynamicMember, /*expr*/ nullptr)}, /*firstTrailingClosureIndex=*/std::nullopt, AllocationArena::ConstraintSolver); + recordArgumentList(argLoc, argList); } auto *callerTy = FunctionType::get( @@ -6521,13 +6522,20 @@ ArgumentList *ConstraintSystem::getArgumentList(ConstraintLocator *locator) { return nullptr; } +void ConstraintSystem::recordArgumentList(ConstraintLocator *locator, + ArgumentList *args) { + bool inserted = ArgumentLists.insert({locator, args}).second; + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::RecordedArgumentList(locator)); +} + void ConstraintSystem::associateArgumentList(ConstraintLocator *locator, ArgumentList *args) { - assert(locator && locator->getAnchor()); - auto *argInfoLoc = getArgumentInfoLocator(locator); - auto inserted = ArgumentLists.insert({argInfoLoc, args}).second; - assert(inserted && "Multiple argument lists at locator?"); - (void)inserted; + ASSERT(locator && locator->getAnchor()); + auto *argLoc = getArgumentInfoLocator(locator); + recordArgumentList(argLoc, args); } ArgumentList *Solution::getArgumentList(ConstraintLocator *locator) const { From 12eb7cec26cfd25c2225b0c751b1a4e0001a0268 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 4 Oct 2024 17:25:28 -0400 Subject: [PATCH 25/26] Sema: Record implicit callAsFunction() roots in the trail --- include/swift/Sema/CSTrail.def | 1 + include/swift/Sema/ConstraintSystem.h | 11 +++-------- lib/Sema/CSSimplify.cpp | 18 +++++++++++------- lib/Sema/CSSolver.cpp | 8 ++------ lib/Sema/ConstraintSystem.cpp | 2 +- 5 files changed, 18 insertions(+), 22 deletions(-) diff --git a/include/swift/Sema/CSTrail.def b/include/swift/Sema/CSTrail.def index 99b096392aeaa..f08b6c8689689 100644 --- a/include/swift/Sema/CSTrail.def +++ b/include/swift/Sema/CSTrail.def @@ -44,6 +44,7 @@ LOCATOR_CHANGE(RecordedDefaultedConstraint, DefaultedConstraints) LOCATOR_CHANGE(ResolvedOverload, ResolvedOverloads) LOCATOR_CHANGE(RecordedImplicitValueConversion, ImplicitValueConversions) LOCATOR_CHANGE(RecordedArgumentList, ArgumentLists) +LOCATOR_CHANGE(RecordedImplicitCallAsFunctionRoot, ImplicitCallAsFunctionRoots) EXPR_CHANGE(AppliedPropertyWrapper) EXPR_CHANGE(RecordedImpliedResult) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 5a7b9f30e7cf2..f47b39103e62a 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2423,7 +2423,7 @@ class ConstraintSystem { /// A cache of implicitly generated dot-member expressions used as roots /// for some `.callAsFunction` calls. The key here is "base" locator for /// the `.callAsFunction` member reference. - llvm::SmallMapVector + llvm::SmallDenseMap ImplicitCallAsFunctionRoots; /// The set of conformances synthesized during solving (i.e. for @@ -2855,9 +2855,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - /// The length of \c ImplicitCallAsFunctionRoots. - unsigned numImplicitCallAsFunctionRoots; - /// The length of \c SynthesizedConformances. unsigned numSynthesizedConformances; @@ -3634,10 +3631,8 @@ class ConstraintSystem { void recordMatchCallArgumentResult(ConstraintLocator *locator, MatchCallArgumentResult result); - /// Record implicitly generated `callAsFunction` with root at the - /// given expression, located at \c locator. - void recordCallAsFunction(UnresolvedDotExpr *root, ArgumentList *arguments, - ConstraintLocator *locator); + void recordImplicitCallAsFunctionRoot( + ConstraintLocator *locator, UnresolvedDotExpr *root); /// Record root, value, and declContext of keypath expression for use across /// constraint system, and add a change to the trail. diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index f4a9a6fbc9066..846c9c9f876bb 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -12899,7 +12899,11 @@ createImplicitRootForCallAsFunction(ConstraintSystem &cs, Type refType, // Record a type of the new reference in the constraint system. cs.setType(implicitRef, refType); // Record new `.callAsFunction` in the constraint system. - cs.recordCallAsFunction(implicitRef, arguments, calleeLocator); + cs.recordImplicitCallAsFunctionRoot(calleeLocator, implicitRef); + + auto *implicitRefLocator = cs.getConstraintLocator( + implicitRef, ConstraintLocator::ApplyArgument); + cs.associateArgumentList(implicitRefLocator, arguments); } return implicitRef; @@ -14974,13 +14978,13 @@ void ConstraintSystem::recordMatchCallArgumentResult( recordChange(SolverTrail::Change::RecordedMatchCallArgumentResult(locator)); } -void ConstraintSystem::recordCallAsFunction(UnresolvedDotExpr *root, - ArgumentList *arguments, - ConstraintLocator *locator) { - ImplicitCallAsFunctionRoots.insert({locator, root}); +void ConstraintSystem::recordImplicitCallAsFunctionRoot( + ConstraintLocator *locator, UnresolvedDotExpr *root) { + bool inserted = ImplicitCallAsFunctionRoots.insert({locator, root}).second; + ASSERT(inserted); - associateArgumentList( - getConstraintLocator(root, ConstraintLocator::ApplyArgument), arguments); + if (solverState) + recordChange(SolverTrail::Change::RecordedImplicitCallAsFunctionRoot(locator)); } void ConstraintSystem::recordKeyPath(const KeyPathExpr *keypath, diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 9a11df9b5d347..58787da0d060c 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -455,7 +455,8 @@ void ConstraintSystem::applySolution(const Solution &solution) { } for (auto &implicitRoot : solution.ImplicitCallAsFunctionRoots) { - ImplicitCallAsFunctionRoots.insert(implicitRoot); + if (ImplicitCallAsFunctionRoots.count(implicitRoot.first) == 0) + recordImplicitCallAsFunctionRoot(implicitRoot.first, implicitRoot.second); } for (auto &synthesized : solution.SynthesizedConformances) { @@ -707,7 +708,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); - numImplicitCallAsFunctionRoots = cs.ImplicitCallAsFunctionRoots.size(); numSynthesizedConformances = cs.SynthesizedConformances.size(); PreviousScore = cs.CurrentScore; @@ -741,10 +741,6 @@ ConstraintSystem::SolverScope::~SolverScope() { // constraints introduced by the current scope. cs.solverState->rollback(this); - // Remove any implicitly generated root expressions for `.callAsFunction` - // which are no longer in scope. - truncate(cs.ImplicitCallAsFunctionRoots, numImplicitCallAsFunctionRoots); - // Remove any implicitly synthesized conformances. truncate(cs.SynthesizedConformances, numSynthesizedConformances); diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 7dc24376fd19d..9cf9c4269aa0e 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -4491,7 +4491,7 @@ size_t Solution::getTotalMemory() const { size_in_bytes(resultBuilderTransformed) + size_in_bytes(appliedPropertyWrappers) + size_in_bytes(argumentLists) + - ImplicitCallAsFunctionRoots.getMemorySize() + + size_in_bytes(ImplicitCallAsFunctionRoots) + size_in_bytes(SynthesizedConformances); } From 4a82d384b79300da19d8a275c7717de531e6c7b0 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 4 Oct 2024 17:30:40 -0400 Subject: [PATCH 26/26] Sema: Record synthesized conformances in the trail --- include/swift/Sema/CSTrail.def | 1 + include/swift/Sema/ConstraintSystem.h | 10 +++++----- lib/Sema/CSSimplify.cpp | 14 +++++++++++++- lib/Sema/CSSolver.cpp | 7 ++----- 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/include/swift/Sema/CSTrail.def b/include/swift/Sema/CSTrail.def index f08b6c8689689..cc9d82ac8a918 100644 --- a/include/swift/Sema/CSTrail.def +++ b/include/swift/Sema/CSTrail.def @@ -45,6 +45,7 @@ LOCATOR_CHANGE(ResolvedOverload, ResolvedOverloads) LOCATOR_CHANGE(RecordedImplicitValueConversion, ImplicitValueConversions) LOCATOR_CHANGE(RecordedArgumentList, ArgumentLists) LOCATOR_CHANGE(RecordedImplicitCallAsFunctionRoot, ImplicitCallAsFunctionRoots) +LOCATOR_CHANGE(RecordedSynthesizedConformance, SynthesizedConformances) EXPR_CHANGE(AppliedPropertyWrapper) EXPR_CHANGE(RecordedImpliedResult) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index f47b39103e62a..ad8f6a24b006f 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -1618,7 +1618,7 @@ class Solution { /// The set of conformances synthesized during solving (i.e. for /// ad-hoc distributed `SerializationRequirement` conformances). - llvm::MapVector + llvm::DenseMap SynthesizedConformances; /// Record a new argument matching choice for given locator that maps a @@ -2428,7 +2428,7 @@ class ConstraintSystem { /// The set of conformances synthesized during solving (i.e. for /// ad-hoc distributed `SerializationRequirement` conformances). - llvm::MapVector + llvm::DenseMap SynthesizedConformances; private: @@ -2855,9 +2855,6 @@ class ConstraintSystem { /// FIXME: Remove this. unsigned numFixes; - /// The length of \c SynthesizedConformances. - unsigned numSynthesizedConformances; - /// The previous score. Score PreviousScore; @@ -5064,6 +5061,9 @@ class ConstraintSystem { ConstraintLocatorBuilder locator, TypeMatchOptions flags); + void recordSynthesizedConformance(ConstraintLocator *locator, + ProtocolConformanceRef conformance); + /// Attempt to simplify the given conformance constraint. /// /// \param type The type being tested. diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 846c9c9f876bb..0ef228869f576 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -8329,6 +8329,16 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint( return matchExistentialTypes(type, protocol, kind, flags, locator); } +void ConstraintSystem::recordSynthesizedConformance( + ConstraintLocator *locator, + ProtocolConformanceRef conformance) { + bool inserted = SynthesizedConformances.insert({locator, conformance}).second; + ASSERT(inserted); + + if (solverState) + recordChange(SolverTrail::Change::RecordedSynthesizedConformance(locator)); +} + ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint( Type type, ProtocolDecl *protocol, @@ -8487,7 +8497,9 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint( ProtocolConformanceRef synthesized(protocol); auto witnessLoc = getConstraintLocator( locator.getAnchor(), LocatorPathElt::Witness(witness)); - SynthesizedConformances.insert({witnessLoc, synthesized}); + // FIXME: Why are we recording the same locator more than once here? + if (SynthesizedConformances.count(witnessLoc) == 0) + recordSynthesizedConformance(witnessLoc, synthesized); return recordConformance(synthesized); }; diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 58787da0d060c..6859c41fc25c7 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -460,7 +460,8 @@ void ConstraintSystem::applySolution(const Solution &solution) { } for (auto &synthesized : solution.SynthesizedConformances) { - SynthesizedConformances.insert(synthesized); + if (SynthesizedConformances.count(synthesized.first) == 0) + recordSynthesizedConformance(synthesized.first, synthesized.second); } // Register any fixes produced along this path. @@ -708,7 +709,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numTypeVariables = cs.TypeVariables.size(); numFixes = cs.Fixes.size(); - numSynthesizedConformances = cs.SynthesizedConformances.size(); PreviousScore = cs.CurrentScore; @@ -741,9 +741,6 @@ ConstraintSystem::SolverScope::~SolverScope() { // constraints introduced by the current scope. cs.solverState->rollback(this); - // Remove any implicitly synthesized conformances. - truncate(cs.SynthesizedConformances, numSynthesizedConformances); - // Reset the previous score. cs.CurrentScore = PreviousScore;