Skip to content

Commit cda3cda

Browse files
slavapestovslavapestov
committed
Sema: Rework change recording in PotentialBindings::retract()
Instead of making an undo() do an infer(), let's record fine-grained changes about what was retracted, and directly re-insert the same elements into the data structures.
1 parent b09c673 commit cda3cda

File tree

8 files changed

+191
-27
lines changed

8 files changed

+191
-27
lines changed

‎include/swift/Sema/CSBindings.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,12 @@ enum class LiteralBindingKind : uint8_t {
7171
/// along with information that can be used to construct related
7272
/// bindings, e.g., the supertypes of a given type.
7373
struct PotentialBinding {
74-
friend class BindingSet;
75-
7674
/// The type to which the type variable can be bound.
7775
Type BindingType;
7876

7977
/// The kind of bindings permitted.
8078
AllowedBindingKind Kind;
8179

82-
protected:
8380
/// The source of the type information.
8481
///
8582
/// Determines whether this binding represents a "hole" in
@@ -91,7 +88,6 @@ struct PotentialBinding {
9188
PointerUnion<Constraint *, ConstraintLocator *> source)
9289
: BindingType(type), Kind(kind), BindingSource(source) {}
9390

94-
public:
9591
PotentialBinding(Type type, AllowedBindingKind kind, Constraint *source)
9692
: PotentialBinding(
9793
type, kind,

‎include/swift/Sema/CSTrail.def

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@
3838
#define GRAPH_NODE_CHANGE(Name) CHANGE(Name)
3939
#endif
4040

41+
#ifndef BINDING_RELATION_CHANGE
42+
#define BINDING_RELATION_CHANGE(Name) CHANGE(Name)
43+
#endif
44+
4145
#ifndef SCORE_CHANGE
4246
#define SCORE_CHANGE(Name) CHANGE(Name)
4347
#endif
@@ -73,6 +77,12 @@ GRAPH_NODE_CHANGE(AddedConstraint)
7377
GRAPH_NODE_CHANGE(RemovedConstraint)
7478
GRAPH_NODE_CHANGE(InferredBindings)
7579
GRAPH_NODE_CHANGE(RetractedBindings)
80+
GRAPH_NODE_CHANGE(RetractedDelayedBy)
81+
82+
BINDING_RELATION_CHANGE(RetractedAdjacentVar)
83+
BINDING_RELATION_CHANGE(RetractedSubtypeOf)
84+
BINDING_RELATION_CHANGE(RetractedSupertypeOf)
85+
BINDING_RELATION_CHANGE(RetractedEquivalentTo)
7686

7787
SCORE_CHANGE(IncreasedScore)
7888
SCORE_CHANGE(DecreasedScore)
@@ -96,14 +106,16 @@ CHANGE(RecordedPotentialThrowSite)
96106
CHANGE(RecordedIsolatedParam)
97107
CHANGE(RecordedKeyPath)
98108
CHANGE(RetiredConstraint)
109+
CHANGE(RetractedBinding)
99110

100-
LAST_CHANGE(RetiredConstraint)
111+
LAST_CHANGE(RetractedBinding)
101112

102113
#undef LOCATOR_CHANGE
103114
#undef EXPR_CHANGE
104115
#undef CLOSURE_CHANGE
105116
#undef CONSTRAINT_CHANGE
106117
#undef GRAPH_NODE_CHANGE
118+
#undef BINDING_RELATION_CHANGE
107119
#undef SCORE_CHANGE
108120
#undef LAST_CHANGE
109121
#undef CHANGE

‎include/swift/Sema/CSTrail.h

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ namespace constraints {
3838
class Constraint;
3939
struct SyntacticElementTargetKey;
4040

41+
namespace inference {
42+
struct PotentialBinding;
43+
}
44+
4145
class SolverTrail {
4246
public:
4347

@@ -89,6 +93,12 @@ class SolverTrail {
8993
TypeVariableType *OtherTypeVar;
9094
} Relation;
9195

96+
struct {
97+
TypeVariableType *TypeVar;
98+
TypeVariableType *OtherTypeVar;
99+
Constraint *Constraint;
100+
} BindingRelation;
101+
92102
struct {
93103
/// The type variable being updated.
94104
TypeVariableType *TypeVar;
@@ -128,6 +138,15 @@ class SolverTrail {
128138
Constraint *Constraint;
129139
} Retiree;
130140

141+
struct {
142+
TypeVariableType *TypeVar;
143+
144+
/// These two fields together with 'Options' above store the contents
145+
/// of a PotentialBinding.
146+
Type BindingType;
147+
PointerUnion<Constraint *, ConstraintLocator *> BindingSource;
148+
} Binding;
149+
131150
ConstraintFix *TheFix;
132151
ConstraintLocator *TheLocator;
133152
PackExpansionType *TheExpansion;
@@ -155,6 +174,9 @@ class SolverTrail {
155174
#define SCORE_CHANGE(Name) static Change Name(ScoreKind kind, unsigned value);
156175
#define GRAPH_NODE_CHANGE(Name) static Change Name(TypeVariableType *typeVar, \
157176
Constraint *constraint);
177+
#define BINDING_RELATION_CHANGE(Name) static Change Name(TypeVariableType *typeVar, \
178+
TypeVariableType *otherTypeVar, \
179+
Constraint *constraint);
158180
#include "swift/Sema/CSTrail.def"
159181

160182
/// Create a change that added a type variable.
@@ -226,6 +248,11 @@ class SolverTrail {
226248
static Change RetiredConstraint(llvm::ilist<Constraint>::iterator where,
227249
Constraint *constraint);
228250

251+
/// Create a change that removed a binding from a type variable's potential
252+
/// bindings.
253+
static Change RetractedBinding(TypeVariableType *typeVar,
254+
inference::PotentialBinding binding);
255+
229256
/// Undo this change, reverting the constraint graph to the state it
230257
/// had prior to this change.
231258
///

‎lib/Sema/CSBindings.cpp

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2073,8 +2073,10 @@ void PotentialBindings::retract(ConstraintSystem &CS,
20732073
if (!Constraints.remove(constraint))
20742074
return;
20752075

2076+
bool recordingChanges = CS.solverState && !CS.solverState->Trail.isUndoActive();
2077+
20762078
// Record the change, if there are active scopes.
2077-
if (CS.solverState && !CS.solverState->Trail.isUndoActive())
2079+
if (recordingChanges)
20782080
CS.recordChange(SolverTrail::Change::RetractedBindings(TypeVar, constraint));
20792081

20802082
LLVM_DEBUG(
@@ -2085,39 +2087,61 @@ void PotentialBindings::retract(ConstraintSystem &CS,
20852087

20862088
Bindings.erase(
20872089
llvm::remove_if(Bindings,
2088-
[&constraint](const PotentialBinding &binding) {
2089-
return binding.getSource() == constraint;
2090+
[&](const PotentialBinding &binding) {
2091+
if (binding.getSource() == constraint) {
2092+
if (recordingChanges) {
2093+
CS.recordChange(SolverTrail::Change::RetractedBinding(
2094+
TypeVar, binding));
2095+
}
2096+
return true;
2097+
}
2098+
return false;
20902099
}),
20912100
Bindings.end());
20922101

20932102
DelayedBy.erase(
20942103
llvm::remove_if(DelayedBy,
2095-
[&constraint](Constraint *existing) {
2096-
return existing == constraint;
2104+
[&](Constraint *existing) {
2105+
if (existing == constraint) {
2106+
if (recordingChanges) {
2107+
CS.recordChange(SolverTrail::Change::RetractedDelayedBy(
2108+
TypeVar, constraint));
2109+
}
2110+
return true;
2111+
}
2112+
return false;
20972113
}),
20982114
DelayedBy.end());
20992115

2100-
auto hasMatchingSource =
2101-
[&constraint](
2102-
const std::pair<TypeVariableType *, Constraint *> &adjacency) {
2103-
return adjacency.second == constraint;
2104-
};
2116+
#define CALLBACK(ChangeKind) \
2117+
[&](std::pair<TypeVariableType *, Constraint *> pair) { \
2118+
if (pair.second == constraint) { \
2119+
if (recordingChanges) { \
2120+
CS.recordChange(SolverTrail::Change::ChangeKind( \
2121+
TypeVar, pair.first, pair.second)); \
2122+
} \
2123+
return true; \
2124+
} \
2125+
return false; \
2126+
}
21052127

21062128
AdjacentVars.erase(
2107-
llvm::remove_if(AdjacentVars, hasMatchingSource),
2129+
llvm::remove_if(AdjacentVars, CALLBACK(RetractedAdjacentVar)),
21082130
AdjacentVars.end());
21092131

21102132
SubtypeOf.erase(
2111-
llvm::remove_if(SubtypeOf, hasMatchingSource),
2133+
llvm::remove_if(SubtypeOf, CALLBACK(RetractedSubtypeOf)),
21122134
SubtypeOf.end());
21132135

21142136
SupertypeOf.erase(
2115-
llvm::remove_if(SupertypeOf, hasMatchingSource),
2137+
llvm::remove_if(SupertypeOf, CALLBACK(RetractedSupertypeOf)),
21162138
SupertypeOf.end());
21172139

21182140
EquivalentTo.erase(
2119-
llvm::remove_if(EquivalentTo, hasMatchingSource),
2141+
llvm::remove_if(EquivalentTo, CALLBACK(RetractedEquivalentTo)),
21202142
EquivalentTo.end());
2143+
2144+
#undef CALLBACK
21212145
}
21222146

21232147
void PotentialBindings::reset() {

‎lib/Sema/CSTrail.cpp

Lines changed: 90 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,18 @@ SolverTrail::~SolverTrail() {
9090
result.TheConstraint.Constraint = constraint; \
9191
return result; \
9292
}
93+
#define BINDING_RELATION_CHANGE(Name) \
94+
SolverTrail::Change \
95+
SolverTrail::Change::Name(TypeVariableType *typeVar, \
96+
TypeVariableType *otherTypeVar, \
97+
Constraint *constraint) { \
98+
Change result; \
99+
result.Kind = ChangeKind::Name; \
100+
result.BindingRelation.TypeVar = typeVar; \
101+
result.BindingRelation.OtherTypeVar = otherTypeVar; \
102+
result.BindingRelation.Constraint = constraint; \
103+
return result; \
104+
}
93105
#define SCORE_CHANGE(Name) \
94106
SolverTrail::Change \
95107
SolverTrail::Change::Name(ScoreKind kind, unsigned value) { \
@@ -306,6 +318,19 @@ SolverTrail::Change::RetiredConstraint(ConstraintList::iterator where,
306318
return result;
307319
}
308320

321+
SolverTrail::Change
322+
SolverTrail::Change::RetractedBinding(TypeVariableType *typeVar,
323+
inference::PotentialBinding binding) {
324+
Change result;
325+
result.Kind = ChangeKind::RetractedBinding;
326+
result.Binding.TypeVar = typeVar;
327+
result.Binding.BindingType = binding.BindingType;
328+
result.Binding.BindingSource = binding.BindingSource;
329+
result.Options = unsigned(binding.Kind);
330+
331+
return result;
332+
}
333+
309334
SyntacticElementTargetKey
310335
SolverTrail::Change::getSyntacticElementTargetKey() const {
311336
ASSERT(Kind == ChangeKind::RecordedTarget);
@@ -359,11 +384,9 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const {
359384
cg.addConstraint(TheConstraint.TypeVar, TheConstraint.Constraint);
360385
break;
361386

362-
case ChangeKind::ExtendedEquivalenceClass: {
363-
auto &node = cg[EquivClass.TypeVar];
364-
node.truncateEquivalenceClass(EquivClass.PrevSize);
387+
case ChangeKind::ExtendedEquivalenceClass:
388+
cg[EquivClass.TypeVar].truncateEquivalenceClass(EquivClass.PrevSize);
365389
break;
366-
}
367390

368391
case ChangeKind::RelatedTypeVariables:
369392
cg.unrelateTypeVariables(Relation.TypeVar, Relation.OtherTypeVar);
@@ -373,9 +396,12 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const {
373396
cg.retractBindings(TheConstraint.TypeVar, TheConstraint.Constraint);
374397
break;
375398

376-
case ChangeKind::RetractedBindings:
377-
cg.inferBindings(TheConstraint.TypeVar, TheConstraint.Constraint);
399+
case ChangeKind::RetractedBindings: {
400+
auto &bindings = cg[TheConstraint.TypeVar].getPotentialBindings();
401+
bool inserted = bindings.Constraints.insert(TheConstraint.Constraint);
402+
ASSERT(inserted);
378403
break;
404+
}
379405

380406
case ChangeKind::UpdatedTypeVariable:
381407
Update.TypeVar->getImpl().setRawOptions(Options);
@@ -494,6 +520,45 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const {
494520
cs.InactiveConstraints.insert(Retiree.Where,
495521
Retiree.Constraint);
496522
break;
523+
524+
case ChangeKind::RetractedDelayedBy:
525+
cg[TheConstraint.TypeVar].getPotentialBindings()
526+
.DelayedBy.push_back(TheConstraint.Constraint);
527+
break;
528+
529+
case ChangeKind::RetractedAdjacentVar:
530+
cg[BindingRelation.TypeVar].getPotentialBindings()
531+
.AdjacentVars.emplace_back(BindingRelation.OtherTypeVar,
532+
BindingRelation.Constraint);
533+
break;
534+
535+
case ChangeKind::RetractedSubtypeOf:
536+
cg[BindingRelation.TypeVar].getPotentialBindings()
537+
.SubtypeOf.emplace_back(BindingRelation.OtherTypeVar,
538+
BindingRelation.Constraint);
539+
break;
540+
541+
case ChangeKind::RetractedSupertypeOf:
542+
cg[BindingRelation.TypeVar].getPotentialBindings()
543+
.SupertypeOf.emplace_back(BindingRelation.OtherTypeVar,
544+
BindingRelation.Constraint);
545+
break;
546+
547+
case ChangeKind::RetractedEquivalentTo:
548+
cg[BindingRelation.TypeVar].getPotentialBindings()
549+
.EquivalentTo.emplace_back(BindingRelation.OtherTypeVar,
550+
BindingRelation.Constraint);
551+
break;
552+
553+
case ChangeKind::RetractedBinding: {
554+
PotentialBinding binding(Binding.BindingType,
555+
AllowedBindingKind(Options),
556+
Binding.BindingSource);
557+
558+
auto &bindings = cg[BindingRelation.TypeVar].getPotentialBindings();
559+
bindings.Bindings.push_back(binding);
560+
break;
561+
}
497562
}
498563
}
499564

@@ -541,6 +606,17 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out,
541606
TheConstraint.TypeVar->print(out, PO); \
542607
out << ")\n"; \
543608
break;
609+
#define BINDING_RELATION_CHANGE(Name) \
610+
case ChangeKind::Name: \
611+
out << "(" << #Name << " "; \
612+
BindingRelation.Constraint->print(out, &cs.getASTContext().SourceMgr, \
613+
indent + 2); \
614+
out << " on type variable "; \
615+
BindingRelation.TypeVar->print(out, PO); \
616+
out << " and "; \
617+
BindingRelation.OtherTypeVar->print(out, PO); \
618+
out << ")\n"; \
619+
break;
544620
#define SCORE_CHANGE(Name) \
545621
case ChangeKind::Name: \
546622
out << "(" << #Name << " "; \
@@ -692,6 +768,14 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out,
692768
indent + 2);
693769
out << ")\n";
694770
break;
771+
772+
case ChangeKind::RetractedBinding:
773+
out << "(RetractedBinding ";
774+
Binding.TypeVar->print(out, PO);
775+
out << " with type ";
776+
Binding.BindingType->print(out, PO);
777+
out << " and kind " << Options << ")\n";
778+
break;
695779
}
696780
}
697781

‎test/Constraints/rdar143474242.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// RUN: %target-typecheck-verify-swift
2+
3+
extension Collection {
4+
func myMap<T>(_: (Self.Element) -> T) -> [T] { fatalError() }
5+
}
6+
7+
protocol SignedInteger {}
8+
9+
extension SignedInteger {
10+
init<T: BinaryFloatingPoint>(_: T) { fatalError() }
11+
init<T: BinaryInteger>(_: T) { fatalError() }
12+
}
13+
14+
struct Int32: SignedInteger {
15+
init(_: String) {}
16+
}
17+
18+
func test() {
19+
let _: [(Int32, Float)] = (0..<1).myMap { (Int32($0), 0.0) }
20+
}

‎test/Sema/issue-46000.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ struct Data {}
1111
extension DispatchData {
1212
func asFoundationData<T>(execute: (Data) throws -> T) rethrows -> T {
1313
return try withUnsafeBytes { (ptr: UnsafePointer<Int8>) -> Void in
14-
// expected-error@-1 {{declared closure result 'Void' is incompatible with contextual type 'T'}}
14+
// expected-error@-1 {{cannot convert return expression of type 'Void' to return type 'T'}}
1515
let data = Data()
1616
return try execute(data) // expected-error {{cannot convert value of type 'T' to closure result type 'Void'}}
1717
}

0 commit comments

Comments
 (0)