Skip to content

Sema: Check worseThanBestSolution() in a better place #76682

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 4 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions include/swift/Sema/ConstraintLocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,18 @@ class LocatorPathElt::KeyPathComponent final : public StoredIntegerElement<1> {
}
};

class LocatorPathElt::ProtocolCompositionMemberType final : public StoredIntegerElement<1> {
public:
ProtocolCompositionMemberType(unsigned index)
: StoredIntegerElement(ConstraintLocator::GenericArgument, index) {}

unsigned getIndex() const { return getValue(); }

static bool classof(const LocatorPathElt *elt) {
return elt->getKind() == ConstraintLocator::ProtocolCompositionMemberType;
}
};

class LocatorPathElt::GenericArgument final : public StoredIntegerElement<1> {
public:
GenericArgument(unsigned index)
Expand Down
2 changes: 1 addition & 1 deletion include/swift/Sema/ConstraintLocatorPathElts.def
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ CUSTOM_LOCATOR_PATH_ELT(ContextualType)
SIMPLE_LOCATOR_PATH_ELT(DynamicLookupResult)

/// The superclass of a protocol composition type.
SIMPLE_LOCATOR_PATH_ELT(ProtocolCompositionSuperclassType)
CUSTOM_LOCATOR_PATH_ELT(ProtocolCompositionMemberType)

/// The constraint of an existential type.
SIMPLE_LOCATOR_PATH_ELT(ExistentialConstraintType)
Expand Down
22 changes: 12 additions & 10 deletions lib/AST/TypeJoinMeet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -460,8 +460,18 @@ CanType TypeJoin::visitProtocolType(CanType second) {
auto *secondDecl =
cast<ProtocolDecl>(second->getNominalOrBoundGenericNominal());

if (firstDecl->getInheritedProtocols().empty() &&
secondDecl->getInheritedProtocols().empty())
SmallVector<Type, 4> firstMembers;
for (auto *decl : firstDecl->getInheritedProtocols())
if (!decl->getInvertibleProtocolKind())
firstMembers.push_back(decl->getDeclaredInterfaceType());

SmallVector<Type, 4> secondMembers;
for (auto *decl : secondDecl->getInheritedProtocols())
if (!decl->getInvertibleProtocolKind())
secondMembers.push_back(decl->getDeclaredInterfaceType());

if (firstMembers.empty() &&
secondMembers.empty())
return TheAnyType;

if (firstDecl->inheritsFrom(secondDecl))
Expand All @@ -473,14 +483,6 @@ CanType TypeJoin::visitProtocolType(CanType second) {
// One isn't the supertype of the other, so instead, treat each as
// if it's a protocol composition of its inherited members, and join
// those.
SmallVector<Type, 4> firstMembers;
for (auto *decl : firstDecl->getInheritedProtocols())
firstMembers.push_back(decl->getDeclaredInterfaceType());

SmallVector<Type, 4> secondMembers;
for (auto *decl : secondDecl->getInheritedProtocols())
secondMembers.push_back(decl->getDeclaredInterfaceType());

return computeProtocolCompositionJoin(firstMembers, secondMembers);
}

Expand Down
44 changes: 16 additions & 28 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3630,10 +3630,8 @@ ConstraintSystem::matchDeepEqualityTypes(Type type1, Type type2,
TypeMatchOptions subflags = TMF_GenerateConstraints;

// Handle opaque archetypes.
if (auto arch1 = type1->getAs<ArchetypeType>()) {
auto arch2 = type2->castTo<ArchetypeType>();
auto opaque1 = cast<OpaqueTypeArchetypeType>(arch1);
auto opaque2 = cast<OpaqueTypeArchetypeType>(arch2);
if (auto opaque1 = type1->getAs<OpaqueTypeArchetypeType>()) {
auto opaque2 = type2->castTo<OpaqueTypeArchetypeType>();
assert(opaque1->getDecl() == opaque2->getDecl());

// It's possible to declare a generic requirement like Self == Self.Iterator
Expand Down Expand Up @@ -3707,34 +3705,24 @@ ConstraintSystem::matchDeepEqualityTypes(Type type1, Type type2,
locator);
}

if (type1->isExistentialType()) {
auto layout1 = type1->getExistentialLayout();
auto layout2 = type2->getExistentialLayout();
// Members of protocol compositions have to match.
if (auto pct1 = type1->getAs<ProtocolCompositionType>()) {
auto pct2 = type2->castTo<ProtocolCompositionType>();

// Explicit AnyObject and protocols must match exactly.
if (layout1.hasExplicitAnyObject != layout2.hasExplicitAnyObject)
auto members1 = pct1->getMembers();
auto members2 = pct2->getMembers();
if (members1.size() != members2.size())
return getTypeMatchFailure(locator);

if (layout1.getProtocols().size() != layout2.getProtocols().size())
if (pct1->getInverses() != pct2->getInverses())
return getTypeMatchFailure(locator);

for (unsigned i: indices(layout1.getProtocols())) {
if (layout1.getProtocols()[i] != layout2.getProtocols()[i])
return getTypeMatchFailure(locator);
}

// This is the only interesting case. We might have type variables
// on either side of the superclass constraint, so make sure we
// recursively call matchTypes() here.
if (layout1.explicitSuperclass || layout2.explicitSuperclass) {
if (!layout1.explicitSuperclass || !layout2.explicitSuperclass)
return getTypeMatchFailure(locator);

if (pct1->hasExplicitAnyObject() != pct2->hasExplicitAnyObject())
return getTypeMatchFailure(locator);
for (unsigned i = 0, e = members1.size(); i < e; ++i) {
auto member1 = members1[i];
auto member2 = members2[i];
auto subLocator = locator.withPathElement(
ConstraintLocator::ProtocolCompositionSuperclassType);
auto result = matchTypes(layout1.explicitSuperclass,
layout2.explicitSuperclass,
ConstraintKind::Bind, subflags,
LocatorPathElt::ProtocolCompositionMemberType(i));
auto result = matchTypes(member1, member2, ConstraintKind::Bind, subflags,
subLocator);
if (result.isFailure())
return result;
Expand Down
6 changes: 5 additions & 1 deletion lib/Sema/CSStep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ StepResult ComponentStep::take(bool prevFailed) {
// One of the previous components created by "split"
// failed, it means that we can't solve this component.
if ((prevFailed && DependsOnPartialSolutions.empty()) ||
CS.isTooComplex(Solutions))
CS.isTooComplex(Solutions) || CS.worseThanBestSolution())
return done(/*isSuccess=*/false);

// Setup active scope, only if previous component didn't fail.
Expand Down Expand Up @@ -925,6 +925,10 @@ StepResult ConjunctionStep::resume(bool prevFailed) {
// Rewind back the constraint system information.
ActiveChoice.reset();

// Reset the best score which was updated by `ConstraintSystem::finalize`
// while forming solution(s) for the current element.
CS.solverState->BestScore.reset();

if (CS.isDebugMode())
getDebugLogger() << ")\n";

Expand Down
8 changes: 5 additions & 3 deletions lib/Sema/ConstraintLocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ unsigned LocatorPathElt::getNewSummaryFlags() const {
case ConstraintLocator::UnresolvedMember:
case ConstraintLocator::ParentType:
case ConstraintLocator::ExistentialConstraintType:
case ConstraintLocator::ProtocolCompositionSuperclassType:
case ConstraintLocator::ProtocolCompositionMemberType:
case ConstraintLocator::LValueConversion:
case ConstraintLocator::DynamicType:
case ConstraintLocator::SubscriptMember:
Expand Down Expand Up @@ -278,9 +278,11 @@ void LocatorPathElt::dump(raw_ostream &out) const {
out << "existential constraint type";
break;

case ConstraintLocator::ProtocolCompositionSuperclassType:
out << "protocol composition superclass type";
case ConstraintLocator::ProtocolCompositionMemberType: {
auto memberElt = elt.castTo<LocatorPathElt::ProtocolCompositionMemberType>();
out << "protocol composition member " << llvm::utostr(memberElt.getIndex());
break;
}

case ConstraintLocator::LValueConversion:
out << "@lvalue-to-inout conversion";
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6145,7 +6145,7 @@ void constraints::simplifyLocator(ASTNode &anchor,
case ConstraintLocator::SequenceElementType:
case ConstraintLocator::ConstructorMemberType:
case ConstraintLocator::ExistentialConstraintType:
case ConstraintLocator::ProtocolCompositionSuperclassType:
case ConstraintLocator::ProtocolCompositionMemberType:
break;

case ConstraintLocator::GenericArgument:
Expand Down