Skip to content

Commit 6dcf22c

Browse files
committed
[CS] Reverse the type order in a couple of pattern equality constraints
Order them such that if they were changed to conversions, they would be sound. This shouldn't make a difference, but unfortunately it turns out pattern equality is not symmetric. As such, tweak the pattern equality logic to account for the reversed types. This allows us to remove a special case from function matching.
1 parent c9ad303 commit 6dcf22c

File tree

2 files changed

+29
-22
lines changed

2 files changed

+29
-22
lines changed

lib/Sema/CSGen.cpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2647,8 +2647,11 @@ namespace {
26472647
if (!subPatternType)
26482648
return Type();
26492649

2650+
// NOTE: The order here is important! Pattern matching equality is
2651+
// not symmetric (we need to fix that either by using a different
2652+
// constraint, or actually making it symmetric).
26502653
CS.addConstraint(
2651-
ConstraintKind::Equal, subPatternType, openedType,
2654+
ConstraintKind::Equal, openedType, subPatternType,
26522655
locator.withPathElement(LocatorPathElt::PatternMatch(pattern)));
26532656

26542657
// FIXME [OPAQUE SUPPORT]: the distinction between where we want opaque
@@ -2759,8 +2762,11 @@ namespace {
27592762
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)),
27602763
castType, bindPatternVarsOneWay);
27612764

2765+
// NOTE: The order here is important! Pattern matching equality is
2766+
// not symmetric (we need to fix that either by using a different
2767+
// constraint, or actually making it symmetric).
27622768
CS.addConstraint(
2763-
ConstraintKind::Equal, subPatternType, castType,
2769+
ConstraintKind::Equal, castType, subPatternType,
27642770
locator.withPathElement(LocatorPathElt::PatternMatch(pattern)));
27652771
}
27662772
return setType(isType);
@@ -2889,7 +2895,13 @@ namespace {
28892895
// FIXME: Verify ExtInfo state is correct, not working by accident.
28902896
FunctionType::ExtInfo info;
28912897
Type functionType = FunctionType::get(params, outputType, info);
2892-
// TODO: Convert to FunctionInput/FunctionResult constraints.
2898+
2899+
// TODO: Convert to own constraint? Note that ApplicableFn isn't quite
2900+
// right, as pattern matching has data flowing *into* the apply result
2901+
// and call arguments, no the other way around.
2902+
// NOTE: The order here is important! Pattern matching equality is
2903+
// not symmetric (we need to fix that either by using a different
2904+
// constraint, or actually making it symmetric).
28932905
CS.addConstraint(
28942906
ConstraintKind::Equal, functionType, memberType,
28952907
locator.withPathElement(LocatorPathElt::PatternMatch(pattern)));

lib/Sema/CSSimplify.cpp

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2125,7 +2125,9 @@ class TupleMatcher {
21252125
const auto &elt1 = tuple1->getElement(i);
21262126
const auto &elt2 = tuple2->getElement(i);
21272127

2128-
if (elt1.hasName() && elt1.getName() != elt2.getName())
2128+
// FIXME: The fact that this isn't symmetric is wrong since this logic
2129+
// is called for bind and equal constraints...
2130+
if (elt2.hasName() && elt1.getName() != elt2.getName())
21292131
return true;
21302132

21312133
pairs.emplace_back(elt1.getType(), elt2.getType(), i, i);
@@ -3502,21 +3504,12 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
35023504
// FIXME: We should check value ownership too, but it's not completely
35033505
// trivial because of inout-to-pointer conversions.
35043506

3505-
// For equality contravariance doesn't matter, but let's make sure
3506-
// that types are matched in original order because that is important
3507-
// when function types are equated as part of pattern matching.
3508-
auto paramType1 = kind == ConstraintKind::Equal ? func1Param.getOldType()
3509-
: func2Param.getOldType();
3510-
3511-
auto paramType2 = kind == ConstraintKind::Equal ? func2Param.getOldType()
3512-
: func1Param.getOldType();
3513-
3514-
// Compare the parameter types.
3515-
auto result = matchTypes(paramType1, paramType2, subKind, subflags,
3516-
(func1Params.size() == 1
3517-
? argumentLocator
3518-
: argumentLocator.withPathElement(
3519-
LocatorPathElt::TupleElement(i))));
3507+
// Compare the parameter types, taking contravariance into account.
3508+
auto result = matchTypes(
3509+
func2Param.getOldType(), func1Param.getOldType(), subKind, subflags,
3510+
(func1Params.size() == 1 ? argumentLocator
3511+
: argumentLocator.withPathElement(
3512+
LocatorPathElt::TupleElement(i))));
35203513
if (result.isFailure())
35213514
return result;
35223515
}
@@ -6356,7 +6349,7 @@ bool ConstraintSystem::repairFailures(
63566349
if (auto *OA = var->getAttrs().getAttribute<ReferenceOwnershipAttr>())
63576350
ROK = OA->get();
63586351

6359-
if (!rhs->getOptionalObjectType() &&
6352+
if (!lhs->getOptionalObjectType() &&
63606353
optionalityOf(ROK) == ReferenceOwnershipOptionality::Required) {
63616354
conversionsOrFixes.push_back(
63626355
AllowNonOptionalWeak::create(*this, getConstraintLocator(NP)));
@@ -14299,8 +14292,10 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
1429914292
if (recordFix(fix))
1430014293
return SolutionKind::Error;
1430114294

14302-
(void)matchTypes(type1, OptionalType::get(type2),
14303-
ConstraintKind::Equal,
14295+
// NOTE: The order here is important! Pattern matching equality is
14296+
// not symmetric (we need to fix that either by using a different
14297+
// constraint, or actually making it symmetric).
14298+
(void)matchTypes(OptionalType::get(type1), type2, ConstraintKind::Equal,
1430414299
TypeMatchFlags::TMF_ApplyingFix, locator);
1430514300

1430614301
return SolutionKind::Solved;

0 commit comments

Comments
 (0)