Skip to content

Sema: More accurate undo() of PotentialBindings::retract() #79134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 5, 2025
63 changes: 19 additions & 44 deletions include/swift/Sema/CSBindings.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,12 @@ enum class LiteralBindingKind : uint8_t {
/// along with information that can be used to construct related
/// bindings, e.g., the supertypes of a given type.
struct PotentialBinding {
friend class BindingSet;

/// The type to which the type variable can be bound.
Type BindingType;

/// The kind of bindings permitted.
AllowedBindingKind Kind;

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

public:
PotentialBinding(Type type, AllowedBindingKind kind, Constraint *source)
: PotentialBinding(
type, kind,
Expand Down Expand Up @@ -219,25 +215,11 @@ struct LiteralRequirement {

struct PotentialBindings {
/// The set of all constraints that have been added via infer().
llvm::SmallPtrSet<Constraint *, 2> Constraints;
llvm::SmallSetVector<Constraint *, 2> Constraints;

/// The set of potential bindings.
llvm::SmallVector<PotentialBinding, 4> Bindings;

/// The set of protocol requirements placed on this type variable.
llvm::SmallVector<Constraint *, 4> Protocols;

/// The set of unique literal protocol requirements placed on this
/// type variable or inferred transitively through subtype chains.
///
/// Note that ordering is important when it comes to bindings, we'd
/// like to add any "direct" default types first to attempt them
/// before transitive ones.
llvm::SmallPtrSet<Constraint *, 2> Literals;

/// The set of constraints which would be used to infer default types.
llvm::SmallPtrSet<Constraint *, 2> Defaults;

/// The set of constraints which delay attempting this type variable.
llvm::TinyPtrVector<Constraint *> DelayedBy;

Expand All @@ -247,22 +229,17 @@ struct PotentialBindings {
/// bindings (contained in the binding type e.g. `Foo<$T0>`), or
/// reachable through subtype/conversion relationship e.g.
/// `$T0 subtype of $T1` or `$T0 arg conversion $T1`.
llvm::SmallDenseSet<std::pair<TypeVariableType *, Constraint *>, 2>
AdjacentVars;

ASTNode AssociatedCodeCompletionToken = ASTNode();
llvm::SmallVector<std::pair<TypeVariableType *, Constraint *>, 2> AdjacentVars;

/// A set of all not-yet-resolved type variables this type variable
/// is a subtype of, supertype of or is equivalent to. This is used
/// to determine ordering inside of a chain of subtypes to help infer
/// transitive bindings and protocol requirements.
llvm::SmallSetVector<std::pair<TypeVariableType *, Constraint *>, 4> SubtypeOf;
llvm::SmallSetVector<std::pair<TypeVariableType *, Constraint *>, 4> SupertypeOf;
llvm::SmallSetVector<std::pair<TypeVariableType *, Constraint *>, 4> EquivalentTo;

void addDefault(Constraint *constraint);
llvm::SmallVector<std::pair<TypeVariableType *, Constraint *>, 4> SubtypeOf;
llvm::SmallVector<std::pair<TypeVariableType *, Constraint *>, 4> SupertypeOf;
llvm::SmallVector<std::pair<TypeVariableType *, Constraint *>, 4> EquivalentTo;

void addLiteral(Constraint *constraint);
ASTNode AssociatedCodeCompletionToken = ASTNode();

/// Add a potential binding to the list of bindings,
/// coalescing supertype bounds when we are able to compute the meet.
Expand Down Expand Up @@ -385,28 +362,26 @@ class BindingSet {

public:
swift::SmallSetVector<PotentialBinding, 4> Bindings;

/// The set of protocol conformance requirements placed on this type variable.
llvm::SmallVector<Constraint *, 4> Protocols;

/// The set of unique literal protocol requirements placed on this
/// type variable or inferred transitively through subtype chains.
///
/// Note that ordering is important when it comes to bindings, we'd
/// like to add any "direct" default types first to attempt them
/// before transitive ones.
llvm::SmallMapVector<ProtocolDecl *, LiteralRequirement, 2> Literals;

llvm::SmallDenseMap<CanType, Constraint *, 2> Defaults;

/// The set of transitive protocol requirements inferred through
/// subtype/conversion/equivalence relations with other type variables.
std::optional<llvm::SmallPtrSet<Constraint *, 4>> TransitiveProtocols;

BindingSet(ConstraintSystem &CS, TypeVariableType *TypeVar,
const PotentialBindings &info)
: CS(CS), TypeVar(TypeVar), Info(info) {
for (const auto &binding : info.Bindings)
addBinding(binding, /*isTransitive=*/false);

for (auto *literal : info.Literals)
addLiteralRequirement(literal);

for (auto *constraint : info.Defaults)
addDefault(constraint);

for (auto &entry : info.AdjacentVars)
AdjacentVars.insert(entry.first);
}
const PotentialBindings &info);

ConstraintSystem &getConstraintSystem() const { return CS; }

Expand Down Expand Up @@ -512,7 +487,7 @@ class BindingSet {
}

ArrayRef<Constraint *> getConformanceRequirements() const {
return Info.Protocols;
return Protocols;
}

unsigned getNumViableLiteralBindings() const;
Expand Down
14 changes: 13 additions & 1 deletion include/swift/Sema/CSTrail.def
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@
#define GRAPH_NODE_CHANGE(Name) CHANGE(Name)
#endif

#ifndef BINDING_RELATION_CHANGE
#define BINDING_RELATION_CHANGE(Name) CHANGE(Name)
#endif

#ifndef SCORE_CHANGE
#define SCORE_CHANGE(Name) CHANGE(Name)
#endif
Expand Down Expand Up @@ -73,6 +77,12 @@ GRAPH_NODE_CHANGE(AddedConstraint)
GRAPH_NODE_CHANGE(RemovedConstraint)
GRAPH_NODE_CHANGE(InferredBindings)
GRAPH_NODE_CHANGE(RetractedBindings)
GRAPH_NODE_CHANGE(RetractedDelayedBy)

BINDING_RELATION_CHANGE(RetractedAdjacentVar)
BINDING_RELATION_CHANGE(RetractedSubtypeOf)
BINDING_RELATION_CHANGE(RetractedSupertypeOf)
BINDING_RELATION_CHANGE(RetractedEquivalentTo)

SCORE_CHANGE(IncreasedScore)
SCORE_CHANGE(DecreasedScore)
Expand All @@ -96,14 +106,16 @@ CHANGE(RecordedPotentialThrowSite)
CHANGE(RecordedIsolatedParam)
CHANGE(RecordedKeyPath)
CHANGE(RetiredConstraint)
CHANGE(RetractedBinding)

LAST_CHANGE(RetiredConstraint)
LAST_CHANGE(RetractedBinding)

#undef LOCATOR_CHANGE
#undef EXPR_CHANGE
#undef CLOSURE_CHANGE
#undef CONSTRAINT_CHANGE
#undef GRAPH_NODE_CHANGE
#undef BINDING_RELATION_CHANGE
#undef SCORE_CHANGE
#undef LAST_CHANGE
#undef CHANGE
27 changes: 27 additions & 0 deletions include/swift/Sema/CSTrail.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ namespace constraints {
class Constraint;
struct SyntacticElementTargetKey;

namespace inference {
struct PotentialBinding;
}

class SolverTrail {
public:

Expand Down Expand Up @@ -89,6 +93,12 @@ class SolverTrail {
TypeVariableType *OtherTypeVar;
} Relation;

struct {
TypeVariableType *TypeVar;
TypeVariableType *OtherTypeVar;
Constraint *Constraint;
} BindingRelation;

struct {
/// The type variable being updated.
TypeVariableType *TypeVar;
Expand Down Expand Up @@ -128,6 +138,15 @@ class SolverTrail {
Constraint *Constraint;
} Retiree;

struct {
TypeVariableType *TypeVar;

/// These two fields together with 'Options' above store the contents
/// of a PotentialBinding.
Type BindingType;
PointerUnion<Constraint *, ConstraintLocator *> BindingSource;
} Binding;

ConstraintFix *TheFix;
ConstraintLocator *TheLocator;
PackExpansionType *TheExpansion;
Expand Down Expand Up @@ -155,6 +174,9 @@ class SolverTrail {
#define SCORE_CHANGE(Name) static Change Name(ScoreKind kind, unsigned value);
#define GRAPH_NODE_CHANGE(Name) static Change Name(TypeVariableType *typeVar, \
Constraint *constraint);
#define BINDING_RELATION_CHANGE(Name) static Change Name(TypeVariableType *typeVar, \
TypeVariableType *otherTypeVar, \
Constraint *constraint);
#include "swift/Sema/CSTrail.def"

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

/// Create a change that removed a binding from a type variable's potential
/// bindings.
static Change RetractedBinding(TypeVariableType *typeVar,
inference::PotentialBinding binding);

/// Undo this change, reverting the constraint graph to the state it
/// had prior to this change.
///
Expand Down
5 changes: 0 additions & 5 deletions include/swift/Sema/ConstraintGraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -448,11 +448,6 @@ class ConstraintGraph {
void unrelateTypeVariables(TypeVariableType *typeVar,
TypeVariableType *otherTypeVar);

/// Infer bindings from the given constraint.
///
/// Note that this it only meant to be called by SolverTrail::Change::undo().
void inferBindings(TypeVariableType *typeVar, Constraint *constraint);

/// Retract bindings from the given constraint.
///
/// Note that this it only meant to be called by SolverTrail::Change::undo().
Expand Down
Loading