Skip to content

Commit f35dd66

Browse files
slavapestovslavapestov
committed
Revert "Sema: Don't generate OneWayEqual constraints for pattern bindings"
This reverts commit 5071e96.
1 parent 5b4218c commit f35dd66

File tree

7 files changed

+161
-85
lines changed

7 files changed

+161
-85
lines changed

‎include/swift/Sema/ConstraintSystem.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4516,6 +4516,7 @@ class ConstraintSystem {
45164516
/// \returns a possibly-sanitized initializer, or null if an error occurred.
45174517
[[nodiscard]]
45184518
Type generateConstraints(Pattern *P, ConstraintLocatorBuilder locator,
4519+
bool bindPatternVarsOneWay,
45194520
PatternBindingDecl *patternBinding,
45204521
unsigned patternIndex);
45214522

‎include/swift/Sema/SyntacticElementTarget.h‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,10 @@ class SyntacticElementTarget {
114114
/// Whether the expression result will be discarded at the end.
115115
bool isDiscarded;
116116

117+
/// Whether to bind the variables encountered within the pattern to
118+
/// fresh type variables via one-way constraints.
119+
bool bindPatternVarsOneWay;
120+
117121
union {
118122
struct {
119123
/// The pattern binding declaration for an initialization, if any.
@@ -251,14 +255,14 @@ class SyntacticElementTarget {
251255
/// Form a target for the initialization of a pattern from an expression.
252256
static SyntacticElementTarget
253257
forInitialization(Expr *initializer, DeclContext *dc, Type patternType,
254-
Pattern *pattern);
258+
Pattern *pattern, bool bindPatternVarsOneWay);
255259

256260
/// Form a target for the initialization of a pattern binding entry from
257261
/// an expression.
258262
static SyntacticElementTarget
259263
forInitialization(Expr *initializer, Type patternType,
260264
PatternBindingDecl *patternBinding,
261-
unsigned patternBindingIndex);
265+
unsigned patternBindingIndex, bool bindPatternVarsOneWay);
262266

263267
/// Form an expression target for a ReturnStmt.
264268
static SyntacticElementTarget
@@ -493,6 +497,14 @@ class SyntacticElementTarget {
493497
return false;
494498
}
495499

500+
/// Whether to bind the types of any variables within the pattern via
501+
/// one-way constraints.
502+
bool shouldBindPatternVarsOneWay() const {
503+
if (kind == Kind::expression)
504+
return expression.bindPatternVarsOneWay;
505+
return false;
506+
}
507+
496508
/// Whether or not an opaque value placeholder should be injected into the
497509
/// first \c wrappedValue argument of an apply expression so the initializer
498510
/// expression can be turned into a property wrapper generator function.

‎lib/Sema/CSGen.cpp‎

Lines changed: 128 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1799,8 +1799,14 @@ namespace {
17991799
///
18001800
/// \param locator The locator to use for generated constraints and
18011801
/// type variables.
1802+
///
1803+
/// \param bindPatternVarsOneWay When true, generate fresh type variables
1804+
/// for the types of each variable declared within the pattern, along
1805+
/// with a one-way constraint binding that to the type to which the
1806+
/// variable will be ascribed or inferred.
18021807
Type getTypeForPattern(
18031808
Pattern *pattern, ConstraintLocatorBuilder locator,
1809+
bool bindPatternVarsOneWay,
18041810
PatternBindingDecl *patternBinding = nullptr,
18051811
unsigned patternBindingIndex = 0) {
18061812
assert(pattern);
@@ -1819,13 +1825,15 @@ namespace {
18191825
auto *subPattern = paren->getSubPattern();
18201826
auto underlyingType = getTypeForPattern(
18211827
subPattern,
1822-
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)));
1828+
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)),
1829+
bindPatternVarsOneWay);
18231830

18241831
return setType(underlyingType);
18251832
}
18261833
case PatternKind::Binding: {
18271834
auto *subPattern = cast<BindingPattern>(pattern)->getSubPattern();
1828-
auto type = getTypeForPattern(subPattern, locator);
1835+
auto type = getTypeForPattern(subPattern, locator,
1836+
bindPatternVarsOneWay);
18291837
// Var doesn't affect the type.
18301838
return setType(type);
18311839
}
@@ -1921,64 +1929,99 @@ namespace {
19211929
var->getNameStr().starts_with("$__builder");
19221930
};
19231931

1924-
// Otherwise, let's use the type of the pattern. The type
1925-
// of the declaration has to be r-value, so let's add an
1926-
// equality constraint if pattern type has any type variables
1927-
// that are allowed to be l-value.
1928-
bool foundLValueVars = false;
1929-
1930-
// Note that it wouldn't be always correct to allocate a single type
1931-
// variable, that disallows l-value types, to use as a declaration
1932-
// type because equality constraint would drop TVO_CanBindToLValue
1933-
// from the right-hand side (which is not the case for `OneWayEqual`)
1934-
// e.g.:
1935-
//
1936-
// struct S { var x, y: Int }
1937-
//
1938-
// func test(s: S) {
1939-
// let (x, y) = (s.x, s.y)
1940-
// }
1941-
//
1942-
// Single type variable approach results in the following constraint:
1943-
// `$T_x_y = ($T_s_x, $T_s_y)` where both `$T_s_x` and `$T_s_y` have
1944-
// to allow l-value, but `$T_x_y` does not. Early simplification of `=`
1945-
// constraint (due to right-hand side being a "concrete" tuple type)
1946-
// would drop l-value option from `$T_s_x` and `$T_s_y` which leads to
1947-
// a failure during member lookup because `x` and `y` are both
1948-
// `@lvalue Int`. To avoid that, declaration type would mimic pattern
1949-
// type with all l-value options stripped, so the equality constraint
1950-
// becomes `($T_x, $_T_y) = ($T_s_x, $T_s_y)` which doesn't result in
1951-
// stripping of l-value flag from the right-hand side since
1952-
// simplification can only happen when either side is resolved.
1953-
auto declTy = varType.transformRec([&](Type type) -> std::optional<Type> {
1954-
if (auto *typeVar = type->getAs<TypeVariableType>()) {
1955-
if (typeVar->getImpl().canBindToLValue()) {
1956-
foundLValueVars = true;
1957-
1958-
// Drop l-value from the options but preserve the rest.
1959-
auto options = typeVar->getImpl().getRawOptions();
1960-
options &= ~TVO_CanBindToLValue;
1961-
1962-
return Type(CS.createTypeVariable(typeVar->getImpl().getLocator(),
1963-
options));
1964-
}
1965-
}
1966-
return std::nullopt;
1967-
});
1932+
// When we are supposed to bind pattern variables, create a fresh
1933+
// type variable and a one-way constraint to assign it to either the
1934+
// deduced type or the externally-imposed type.
1935+
Type oneWayVarType;
1936+
if (bindPatternVarsOneWay) {
1937+
oneWayVarType = CS.createTypeVariable(
1938+
CS.getConstraintLocator(locator), TVO_CanBindToNoEscape);
1939+
1940+
// If there is externally-imposed type, and the variable
1941+
// is marked as `weak`, let's fallthrough and allow the
1942+
// `one-way` constraint to be fixed in diagnostic mode.
1943+
//
1944+
// That would make sure that type of this variable is
1945+
// recorded in the constraint system, which would then
1946+
// be used instead of `getVarType` upon discovering a
1947+
// reference to this variable in subsequent expression(s).
1948+
//
1949+
// If we let constraint generation fail here, it would trigger
1950+
// interface type request via `var->getType()` that would
1951+
// attempt to validate `weak` attribute, and produce a
1952+
// diagnostic in the middle of the solver path.
19681953

1969-
// If pattern types allows l-value types, let's create an
1970-
// equality constraint between r-value only declaration type
1971-
// and l-value pattern type that would take care of looking
1972-
// through l-values when necessary.
1973-
if (foundLValueVars) {
1974-
CS.addConstraint(ConstraintKind::Equal, declTy, varType,
1975-
CS.getConstraintLocator(locator));
1954+
CS.addConstraint(ConstraintKind::OneWayEqual, oneWayVarType,
1955+
varType, locator);
1956+
1957+
if (useLocatableTypes())
1958+
oneWayVarType = makeTypeLocatableIfPossible(oneWayVarType);
19761959
}
19771960

1978-
if (useLocatableTypes())
1979-
declTy = makeTypeLocatableIfPossible(declTy);
1961+
// Ascribe a type to the declaration so it's always available to
1962+
// constraint system.
1963+
if (oneWayVarType) {
1964+
CS.setType(var, oneWayVarType);
1965+
} else {
1966+
// Otherwise, let's use the type of the pattern. The type
1967+
// of the declaration has to be r-value, so let's add an
1968+
// equality constraint if pattern type has any type variables
1969+
// that are allowed to be l-value.
1970+
bool foundLValueVars = false;
1971+
1972+
// Note that it wouldn't be always correct to allocate a single type
1973+
// variable, that disallows l-value types, to use as a declaration
1974+
// type because equality constraint would drop TVO_CanBindToLValue
1975+
// from the right-hand side (which is not the case for `OneWayEqual`)
1976+
// e.g.:
1977+
//
1978+
// struct S { var x, y: Int }
1979+
//
1980+
// func test(s: S) {
1981+
// let (x, y) = (s.x, s.y)
1982+
// }
1983+
//
1984+
// Single type variable approach results in the following constraint:
1985+
// `$T_x_y = ($T_s_x, $T_s_y)` where both `$T_s_x` and `$T_s_y` have
1986+
// to allow l-value, but `$T_x_y` does not. Early simplification of `=`
1987+
// constraint (due to right-hand side being a "concrete" tuple type)
1988+
// would drop l-value option from `$T_s_x` and `$T_s_y` which leads to
1989+
// a failure during member lookup because `x` and `y` are both
1990+
// `@lvalue Int`. To avoid that, declaration type would mimic pattern
1991+
// type with all l-value options stripped, so the equality constraint
1992+
// becomes `($T_x, $_T_y) = ($T_s_x, $T_s_y)` which doesn't result in
1993+
// stripping of l-value flag from the right-hand side since
1994+
// simplification can only happen when either side is resolved.
1995+
auto declTy = varType.transformRec([&](Type type) -> std::optional<Type> {
1996+
if (auto *typeVar = type->getAs<TypeVariableType>()) {
1997+
if (typeVar->getImpl().canBindToLValue()) {
1998+
foundLValueVars = true;
1999+
2000+
// Drop l-value from the options but preserve the rest.
2001+
auto options = typeVar->getImpl().getRawOptions();
2002+
options &= ~TVO_CanBindToLValue;
2003+
2004+
return Type(CS.createTypeVariable(typeVar->getImpl().getLocator(),
2005+
options));
2006+
}
2007+
}
2008+
return std::nullopt;
2009+
});
2010+
2011+
// If pattern types allows l-value types, let's create an
2012+
// equality constraint between r-value only declaration type
2013+
// and l-value pattern type that would take care of looking
2014+
// through l-values when necessary.
2015+
if (foundLValueVars) {
2016+
CS.addConstraint(ConstraintKind::Equal, declTy, varType,
2017+
CS.getConstraintLocator(locator));
2018+
}
2019+
2020+
if (useLocatableTypes())
2021+
declTy = makeTypeLocatableIfPossible(declTy);
19802022

1981-
CS.setType(var, declTy);
2023+
CS.setType(var, declTy);
2024+
}
19822025

19832026
return setType(varType);
19842027
}
@@ -2007,7 +2050,8 @@ namespace {
20072050
// ascribed type.
20082051
Type subPatternType = getTypeForPattern(
20092052
subPattern,
2010-
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)));
2053+
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)),
2054+
bindPatternVarsOneWay);
20112055

20122056
// NOTE: The order here is important! Pattern matching equality is
20132057
// not symmetric (we need to fix that either by using a different
@@ -2035,7 +2079,8 @@ namespace {
20352079
auto *eltPattern = tupleElt.getPattern();
20362080
Type eltTy = getTypeForPattern(
20372081
eltPattern,
2038-
locator.withPathElement(LocatorPathElt::PatternMatch(eltPattern)));
2082+
locator.withPathElement(LocatorPathElt::PatternMatch(eltPattern)),
2083+
bindPatternVarsOneWay);
20392084

20402085
tupleTypeElts.push_back(TupleTypeElt(eltTy, tupleElt.getLabel()));
20412086
}
@@ -2048,7 +2093,8 @@ namespace {
20482093
// The subpattern must have optional type.
20492094
Type subPatternType = getTypeForPattern(
20502095
subPattern,
2051-
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)));
2096+
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)),
2097+
bindPatternVarsOneWay);
20522098

20532099
return setType(OptionalType::get(subPatternType));
20542100
}
@@ -2078,7 +2124,8 @@ namespace {
20782124
if (auto *subPattern = isPattern->getSubPattern()) {
20792125
auto subPatternType = getTypeForPattern(
20802126
subPattern,
2081-
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)));
2127+
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)),
2128+
bindPatternVarsOneWay);
20822129

20832130
// NOTE: The order here is important! Pattern matching equality is
20842131
// not symmetric (we need to fix that either by using a different
@@ -2181,7 +2228,8 @@ namespace {
21812228
// types.
21822229
Type subPatternType = getTypeForPattern(
21832230
subPattern,
2184-
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)));
2231+
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)),
2232+
bindPatternVarsOneWay);
21852233

21862234
SmallVector<AnyFunctionType::Param, 4> params;
21872235
decomposeTuple(subPatternType, params);
@@ -3608,7 +3656,7 @@ static bool generateInitPatternConstraints(ConstraintSystem &cs,
36083656
Type patternType;
36093657
if (auto pattern = target.getInitializationPattern()) {
36103658
patternType = cs.generateConstraints(
3611-
pattern, locator,
3659+
pattern, locator, target.shouldBindPatternVarsOneWay(),
36123660
target.getInitializationPatternBindingDecl(),
36133661
target.getInitializationPatternBindingIndex());
36143662
} else {
@@ -3659,7 +3707,8 @@ generateForEachStmtConstraints(ConstraintSystem &cs, DeclContext *dc,
36593707
/// expression that conforms to `Swift.Sequence`.
36603708
static std::optional<SequenceIterationInfo>
36613709
generateForEachStmtConstraints(ConstraintSystem &cs, DeclContext *dc,
3662-
ForEachStmt *stmt, Pattern *typeCheckedPattern) {
3710+
ForEachStmt *stmt, Pattern *typeCheckedPattern,
3711+
bool shouldBindPatternVarsOneWay) {
36633712
ASTContext &ctx = cs.getASTContext();
36643713
bool isAsync = stmt->getAwaitLoc().isValid();
36653714
auto *sequenceExpr = stmt->getParsedSequence();
@@ -3735,7 +3784,8 @@ generateForEachStmtConstraints(ConstraintSystem &cs, DeclContext *dc,
37353784
ctx, StaticSpellingKind::None, pattern, makeIteratorCall, dc);
37363785

37373786
auto makeIteratorTarget = SyntacticElementTarget::forInitialization(
3738-
makeIteratorCall, /*patternType=*/Type(), PB, /*index=*/0);
3787+
makeIteratorCall, /*patternType=*/Type(), PB, /*index=*/0,
3788+
/*shouldBindPatternsOneWay=*/false);
37393789

37403790
ContextualTypeInfo contextInfo(sequenceProto->getDeclaredInterfaceType(),
37413791
CTP_ForEachSequence);
@@ -3830,7 +3880,8 @@ generateForEachStmtConstraints(ConstraintSystem &cs, DeclContext *dc,
38303880

38313881
// Generate constraints for the pattern.
38323882
Type initType =
3833-
cs.generateConstraints(typeCheckedPattern, elementLocator, nullptr, 0);
3883+
cs.generateConstraints(typeCheckedPattern, elementLocator,
3884+
shouldBindPatternVarsOneWay, nullptr, 0);
38343885
if (!initType)
38353886
return std::nullopt;
38363887

@@ -3882,7 +3933,8 @@ generateForEachPreambleConstraints(ConstraintSystem &cs,
38823933

38833934
// Generate constraints for the pattern.
38843935
Type patternType = cs.generateConstraints(
3885-
pattern, elementLocator, nullptr, 0);
3936+
pattern, elementLocator, target.shouldBindPatternVarsOneWay(), nullptr,
3937+
0);
38863938
if (!patternType)
38873939
return std::nullopt;
38883940

@@ -3899,8 +3951,8 @@ generateForEachPreambleConstraints(ConstraintSystem &cs,
38993951

39003952
target.getForEachStmtInfo() = *packIterationInfo;
39013953
} else {
3902-
auto sequenceIterationInfo =
3903-
generateForEachStmtConstraints(cs, dc, stmt, pattern);
3954+
auto sequenceIterationInfo = generateForEachStmtConstraints(
3955+
cs, dc, stmt, pattern, target.shouldBindPatternVarsOneWay());
39043956
if (!sequenceIterationInfo) {
39053957
return std::nullopt;
39063958
}
@@ -4057,7 +4109,8 @@ bool ConstraintSystem::generateConstraints(
40574109
}
40584110

40594111
auto target = init ? SyntacticElementTarget::forInitialization(
4060-
init, patternType, patternBinding, index)
4112+
init, patternType, patternBinding, index,
4113+
/*bindPatternVarsOneWay=*/true)
40614114
: SyntacticElementTarget::forUninitializedVar(
40624115
patternBinding, index, patternType);
40634116

@@ -4089,7 +4142,7 @@ bool ConstraintSystem::generateConstraints(
40894142
// Generate constraints to bind all of the internal declarations
40904143
// and verify the pattern.
40914144
Type patternType = generateConstraints(
4092-
pattern, locator,
4145+
pattern, locator, /*shouldBindPatternVarsOneWay*/ true,
40934146
target.getPatternBindingOfUninitializedVar(),
40944147
target.getIndexOfUninitializedVar());
40954148

@@ -4124,10 +4177,11 @@ Expr *ConstraintSystem::generateConstraints(Expr *expr, DeclContext *dc) {
41244177

41254178
Type ConstraintSystem::generateConstraints(
41264179
Pattern *pattern, ConstraintLocatorBuilder locator,
4127-
PatternBindingDecl *patternBinding,
4180+
bool bindPatternVarsOneWay, PatternBindingDecl *patternBinding,
41284181
unsigned patternIndex) {
41294182
ConstraintGenerator cg(*this, nullptr);
4130-
auto ty = cg.getTypeForPattern(pattern, locator, patternBinding, patternIndex);
4183+
auto ty = cg.getTypeForPattern(pattern, locator, bindPatternVarsOneWay,
4184+
patternBinding, patternIndex);
41314185
assert(ty);
41324186

41334187
// Gather the ExprPatterns, and form a conjunction for their expressions.
@@ -4188,7 +4242,8 @@ bool ConstraintSystem::generateConstraints(StmtCondition condition,
41884242
return true;
41894243

41904244
auto target = SyntacticElementTarget::forInitialization(
4191-
condElement.getInitializer(), dc, Type(), pattern);
4245+
condElement.getInitializer(), dc, Type(), pattern,
4246+
/*bindPatternVarsOneWay=*/true);
41924247
if (generateConstraints(target, FreeTypeVariableBinding::Disallow))
41934248
return true;
41944249

0 commit comments

Comments
 (0)