Skip to content

Commit 5071e96

Browse files
slavapestovxedin
slavapestov
authored and
xedin
committed
Sema: Don't generate OneWayEqual constraints for pattern bindings
1 parent 74f8960 commit 5071e96

File tree

7 files changed

+83
-162
lines changed

7 files changed

+83
-162
lines changed

‎include/swift/Sema/ConstraintSystem.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4504,7 +4504,6 @@ class ConstraintSystem {
45044504
/// \returns a possibly-sanitized initializer, or null if an error occurred.
45054505
[[nodiscard]]
45064506
Type generateConstraints(Pattern *P, ConstraintLocatorBuilder locator,
4507-
bool bindPatternVarsOneWay,
45084507
PatternBindingDecl *patternBinding,
45094508
unsigned patternIndex);
45104509

‎include/swift/Sema/SyntacticElementTarget.h‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,6 @@ 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-
121117
union {
122118
struct {
123119
/// The pattern binding declaration for an initialization, if any.
@@ -258,14 +254,14 @@ class SyntacticElementTarget {
258254
/// Form a target for the initialization of a pattern from an expression.
259255
static SyntacticElementTarget
260256
forInitialization(Expr *initializer, DeclContext *dc, Type patternType,
261-
Pattern *pattern, bool bindPatternVarsOneWay);
257+
Pattern *pattern);
262258

263259
/// Form a target for the initialization of a pattern binding entry from
264260
/// an expression.
265261
static SyntacticElementTarget
266262
forInitialization(Expr *initializer, Type patternType,
267263
PatternBindingDecl *patternBinding,
268-
unsigned patternBindingIndex, bool bindPatternVarsOneWay);
264+
unsigned patternBindingIndex);
269265

270266
/// Form an expression target for a ReturnStmt.
271267
static SyntacticElementTarget
@@ -500,16 +496,6 @@ class SyntacticElementTarget {
500496
return false;
501497
}
502498

503-
/// Whether to bind the types of any variables within the pattern via
504-
/// one-way constraints.
505-
bool shouldBindPatternVarsOneWay() const {
506-
if (kind == Kind::expression)
507-
return expression.bindPatternVarsOneWay;
508-
if (kind == Kind::forEachPreamble)
509-
return !ignoreForEachWhereClause() && forEachStmt.stmt->getWhere();
510-
return false;
511-
}
512-
513499
/// Whether or not an opaque value placeholder should be injected into the
514500
/// first \c wrappedValue argument of an apply expression so the initializer
515501
/// expression can be turned into a property wrapper generator function.

‎lib/Sema/CSGen.cpp‎

Lines changed: 71 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -1802,14 +1802,8 @@ namespace {
18021802
///
18031803
/// \param locator The locator to use for generated constraints and
18041804
/// type variables.
1805-
///
1806-
/// \param bindPatternVarsOneWay When true, generate fresh type variables
1807-
/// for the types of each variable declared within the pattern, along
1808-
/// with a one-way constraint binding that to the type to which the
1809-
/// variable will be ascribed or inferred.
18101805
Type getTypeForPattern(
18111806
Pattern *pattern, ConstraintLocatorBuilder locator,
1812-
bool bindPatternVarsOneWay,
18131807
PatternBindingDecl *patternBinding = nullptr,
18141808
unsigned patternBindingIndex = 0) {
18151809
assert(pattern);
@@ -1828,15 +1822,13 @@ namespace {
18281822
auto *subPattern = paren->getSubPattern();
18291823
auto underlyingType = getTypeForPattern(
18301824
subPattern,
1831-
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)),
1832-
bindPatternVarsOneWay);
1825+
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)));
18331826

18341827
return setType(underlyingType);
18351828
}
18361829
case PatternKind::Binding: {
18371830
auto *subPattern = cast<BindingPattern>(pattern)->getSubPattern();
1838-
auto type = getTypeForPattern(subPattern, locator,
1839-
bindPatternVarsOneWay);
1831+
auto type = getTypeForPattern(subPattern, locator);
18401832
// Var doesn't affect the type.
18411833
return setType(type);
18421834
}
@@ -1932,100 +1924,65 @@ namespace {
19321924
var->getNameStr().starts_with("$__builder");
19331925
};
19341926

1935-
// When we are supposed to bind pattern variables, create a fresh
1936-
// type variable and a one-way constraint to assign it to either the
1937-
// deduced type or the externally-imposed type.
1938-
Type oneWayVarType;
1939-
if (bindPatternVarsOneWay) {
1940-
oneWayVarType = CS.createTypeVariable(
1941-
CS.getConstraintLocator(locator), TVO_CanBindToNoEscape);
1942-
1943-
// If there is externally-imposed type, and the variable
1944-
// is marked as `weak`, let's fallthrough and allow the
1945-
// `one-way` constraint to be fixed in diagnostic mode.
1946-
//
1947-
// That would make sure that type of this variable is
1948-
// recorded in the constraint system, which would then
1949-
// be used instead of `getVarType` upon discovering a
1950-
// reference to this variable in subsequent expression(s).
1951-
//
1952-
// If we let constraint generation fail here, it would trigger
1953-
// interface type request via `var->getType()` that would
1954-
// attempt to validate `weak` attribute, and produce a
1955-
// diagnostic in the middle of the solver path.
1956-
1957-
CS.addConstraint(ConstraintKind::OneWayEqual, oneWayVarType,
1958-
varType, locator);
1959-
1960-
if (useLocatableTypes())
1961-
oneWayVarType = makeTypeLocatableIfPossible(oneWayVarType);
1962-
}
1963-
1964-
// Ascribe a type to the declaration so it's always available to
1965-
// constraint system.
1966-
if (oneWayVarType) {
1967-
CS.setType(var, oneWayVarType);
1968-
} else {
1969-
// Otherwise, let's use the type of the pattern. The type
1970-
// of the declaration has to be r-value, so let's add an
1971-
// equality constraint if pattern type has any type variables
1972-
// that are allowed to be l-value.
1973-
bool foundLValueVars = false;
1974-
1975-
// Note that it wouldn't be always correct to allocate a single type
1976-
// variable, that disallows l-value types, to use as a declaration
1977-
// type because equality constraint would drop TVO_CanBindToLValue
1978-
// from the right-hand side (which is not the case for `OneWayEqual`)
1979-
// e.g.:
1980-
//
1981-
// struct S { var x, y: Int }
1982-
//
1983-
// func test(s: S) {
1984-
// let (x, y) = (s.x, s.y)
1985-
// }
1986-
//
1987-
// Single type variable approach results in the following constraint:
1988-
// `$T_x_y = ($T_s_x, $T_s_y)` where both `$T_s_x` and `$T_s_y` have
1989-
// to allow l-value, but `$T_x_y` does not. Early simplification of `=`
1990-
// constraint (due to right-hand side being a "concrete" tuple type)
1991-
// would drop l-value option from `$T_s_x` and `$T_s_y` which leads to
1992-
// a failure during member lookup because `x` and `y` are both
1993-
// `@lvalue Int`. To avoid that, declaration type would mimic pattern
1994-
// type with all l-value options stripped, so the equality constraint
1995-
// becomes `($T_x, $_T_y) = ($T_s_x, $T_s_y)` which doesn't result in
1996-
// stripping of l-value flag from the right-hand side since
1997-
// simplification can only happen when either side is resolved.
1998-
auto declTy = varType.transformRec([&](Type type) -> std::optional<Type> {
1999-
if (auto *typeVar = type->getAs<TypeVariableType>()) {
2000-
if (typeVar->getImpl().canBindToLValue()) {
2001-
foundLValueVars = true;
2002-
2003-
// Drop l-value from the options but preserve the rest.
2004-
auto options = typeVar->getImpl().getRawOptions();
2005-
options &= ~TVO_CanBindToLValue;
2006-
2007-
return Type(CS.createTypeVariable(typeVar->getImpl().getLocator(),
2008-
options));
2009-
}
1927+
// Otherwise, let's use the type of the pattern. The type
1928+
// of the declaration has to be r-value, so let's add an
1929+
// equality constraint if pattern type has any type variables
1930+
// that are allowed to be l-value.
1931+
bool foundLValueVars = false;
1932+
1933+
// Note that it wouldn't be always correct to allocate a single type
1934+
// variable, that disallows l-value types, to use as a declaration
1935+
// type because equality constraint would drop TVO_CanBindToLValue
1936+
// from the right-hand side (which is not the case for `OneWayEqual`)
1937+
// e.g.:
1938+
//
1939+
// struct S { var x, y: Int }
1940+
//
1941+
// func test(s: S) {
1942+
// let (x, y) = (s.x, s.y)
1943+
// }
1944+
//
1945+
// Single type variable approach results in the following constraint:
1946+
// `$T_x_y = ($T_s_x, $T_s_y)` where both `$T_s_x` and `$T_s_y` have
1947+
// to allow l-value, but `$T_x_y` does not. Early simplification of `=`
1948+
// constraint (due to right-hand side being a "concrete" tuple type)
1949+
// would drop l-value option from `$T_s_x` and `$T_s_y` which leads to
1950+
// a failure during member lookup because `x` and `y` are both
1951+
// `@lvalue Int`. To avoid that, declaration type would mimic pattern
1952+
// type with all l-value options stripped, so the equality constraint
1953+
// becomes `($T_x, $_T_y) = ($T_s_x, $T_s_y)` which doesn't result in
1954+
// stripping of l-value flag from the right-hand side since
1955+
// simplification can only happen when either side is resolved.
1956+
auto declTy = varType.transformRec([&](Type type) -> std::optional<Type> {
1957+
if (auto *typeVar = type->getAs<TypeVariableType>()) {
1958+
if (typeVar->getImpl().canBindToLValue()) {
1959+
foundLValueVars = true;
1960+
1961+
// Drop l-value from the options but preserve the rest.
1962+
auto options = typeVar->getImpl().getRawOptions();
1963+
options &= ~TVO_CanBindToLValue;
1964+
1965+
return Type(CS.createTypeVariable(typeVar->getImpl().getLocator(),
1966+
options));
20101967
}
2011-
return std::nullopt;
2012-
});
2013-
2014-
// If pattern types allows l-value types, let's create an
2015-
// equality constraint between r-value only declaration type
2016-
// and l-value pattern type that would take care of looking
2017-
// through l-values when necessary.
2018-
if (foundLValueVars) {
2019-
CS.addConstraint(ConstraintKind::Equal, declTy, varType,
2020-
CS.getConstraintLocator(locator));
20211968
}
1969+
return std::nullopt;
1970+
});
20221971

2023-
if (useLocatableTypes())
2024-
declTy = makeTypeLocatableIfPossible(declTy);
2025-
2026-
CS.setType(var, declTy);
1972+
// If pattern types allows l-value types, let's create an
1973+
// equality constraint between r-value only declaration type
1974+
// and l-value pattern type that would take care of looking
1975+
// through l-values when necessary.
1976+
if (foundLValueVars) {
1977+
CS.addConstraint(ConstraintKind::Equal, declTy, varType,
1978+
CS.getConstraintLocator(locator));
20271979
}
20281980

1981+
if (useLocatableTypes())
1982+
declTy = makeTypeLocatableIfPossible(declTy);
1983+
1984+
CS.setType(var, declTy);
1985+
20291986
return setType(varType);
20301987
}
20311988

@@ -2053,8 +2010,7 @@ namespace {
20532010
// ascribed type.
20542011
Type subPatternType = getTypeForPattern(
20552012
subPattern,
2056-
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)),
2057-
bindPatternVarsOneWay);
2013+
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)));
20582014

20592015
// NOTE: The order here is important! Pattern matching equality is
20602016
// not symmetric (we need to fix that either by using a different
@@ -2082,8 +2038,7 @@ namespace {
20822038
auto *eltPattern = tupleElt.getPattern();
20832039
Type eltTy = getTypeForPattern(
20842040
eltPattern,
2085-
locator.withPathElement(LocatorPathElt::PatternMatch(eltPattern)),
2086-
bindPatternVarsOneWay);
2041+
locator.withPathElement(LocatorPathElt::PatternMatch(eltPattern)));
20872042

20882043
tupleTypeElts.push_back(TupleTypeElt(eltTy, tupleElt.getLabel()));
20892044
}
@@ -2096,8 +2051,7 @@ namespace {
20962051
// The subpattern must have optional type.
20972052
Type subPatternType = getTypeForPattern(
20982053
subPattern,
2099-
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)),
2100-
bindPatternVarsOneWay);
2054+
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)));
21012055

21022056
return setType(OptionalType::get(subPatternType));
21032057
}
@@ -2127,8 +2081,7 @@ namespace {
21272081
if (auto *subPattern = isPattern->getSubPattern()) {
21282082
auto subPatternType = getTypeForPattern(
21292083
subPattern,
2130-
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)),
2131-
bindPatternVarsOneWay);
2084+
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)));
21322085

21332086
// NOTE: The order here is important! Pattern matching equality is
21342087
// not symmetric (we need to fix that either by using a different
@@ -2231,8 +2184,7 @@ namespace {
22312184
// types.
22322185
Type subPatternType = getTypeForPattern(
22332186
subPattern,
2234-
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)),
2235-
bindPatternVarsOneWay);
2187+
locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)));
22362188

22372189
SmallVector<AnyFunctionType::Param, 4> params;
22382190
decomposeTuple(subPatternType, params);
@@ -3657,7 +3609,7 @@ static bool generateInitPatternConstraints(ConstraintSystem &cs,
36573609
Type patternType;
36583610
if (auto pattern = target.getInitializationPattern()) {
36593611
patternType = cs.generateConstraints(
3660-
pattern, locator, target.shouldBindPatternVarsOneWay(),
3612+
pattern, locator,
36613613
target.getInitializationPatternBindingDecl(),
36623614
target.getInitializationPatternBindingIndex());
36633615
} else {
@@ -3709,7 +3661,6 @@ generateForEachStmtConstraints(ConstraintSystem &cs, DeclContext *dc,
37093661
static std::optional<SequenceIterationInfo>
37103662
generateForEachStmtConstraints(ConstraintSystem &cs, DeclContext *dc,
37113663
ForEachStmt *stmt, Pattern *typeCheckedPattern,
3712-
bool shouldBindPatternVarsOneWay,
37133664
bool ignoreForEachWhereClause) {
37143665
ASTContext &ctx = cs.getASTContext();
37153666
bool isAsync = stmt->getAwaitLoc().isValid();
@@ -3771,8 +3722,7 @@ generateForEachStmtConstraints(ConstraintSystem &cs, DeclContext *dc,
37713722
ctx, StaticSpellingKind::None, pattern, makeIteratorCall, dc);
37723723

37733724
auto makeIteratorTarget = SyntacticElementTarget::forInitialization(
3774-
makeIteratorCall, /*patternType=*/Type(), PB, /*index=*/0,
3775-
/*shouldBindPatternsOneWay=*/false);
3725+
makeIteratorCall, /*patternType=*/Type(), PB, /*index=*/0);
37763726

37773727
ContextualTypeInfo contextInfo(sequenceProto->getDeclaredInterfaceType(),
37783728
CTP_ForEachSequence);
@@ -3867,8 +3817,7 @@ generateForEachStmtConstraints(ConstraintSystem &cs, DeclContext *dc,
38673817

38683818
// Generate constraints for the pattern.
38693819
Type initType =
3870-
cs.generateConstraints(typeCheckedPattern, elementLocator,
3871-
shouldBindPatternVarsOneWay, nullptr, 0);
3820+
cs.generateConstraints(typeCheckedPattern, elementLocator, nullptr, 0);
38723821
if (!initType)
38733822
return std::nullopt;
38743823

@@ -3938,8 +3887,7 @@ generateForEachPreambleConstraints(ConstraintSystem &cs,
39383887

39393888
// Generate constraints for the pattern.
39403889
Type patternType = cs.generateConstraints(
3941-
pattern, elementLocator, target.shouldBindPatternVarsOneWay(), nullptr,
3942-
0);
3890+
pattern, elementLocator, nullptr, 0);
39433891
if (!patternType)
39443892
return std::nullopt;
39453893

@@ -3957,8 +3905,7 @@ generateForEachPreambleConstraints(ConstraintSystem &cs,
39573905
target.getForEachStmtInfo() = *packIterationInfo;
39583906
} else {
39593907
auto sequenceIterationInfo = generateForEachStmtConstraints(
3960-
cs, dc, stmt, pattern, target.shouldBindPatternVarsOneWay(),
3961-
target.ignoreForEachWhereClause());
3908+
cs, dc, stmt, pattern, target.ignoreForEachWhereClause());
39623909
if (!sequenceIterationInfo) {
39633910
return std::nullopt;
39643911
}
@@ -4115,8 +4062,7 @@ bool ConstraintSystem::generateConstraints(
41154062
}
41164063

41174064
auto target = init ? SyntacticElementTarget::forInitialization(
4118-
init, patternType, patternBinding, index,
4119-
/*bindPatternVarsOneWay=*/true)
4065+
init, patternType, patternBinding, index)
41204066
: SyntacticElementTarget::forUninitializedVar(
41214067
patternBinding, index, patternType);
41224068

@@ -4148,7 +4094,7 @@ bool ConstraintSystem::generateConstraints(
41484094
// Generate constraints to bind all of the internal declarations
41494095
// and verify the pattern.
41504096
Type patternType = generateConstraints(
4151-
pattern, locator, /*shouldBindPatternVarsOneWay*/ true,
4097+
pattern, locator,
41524098
target.getPatternBindingOfUninitializedVar(),
41534099
target.getIndexOfUninitializedVar());
41544100

@@ -4183,11 +4129,10 @@ Expr *ConstraintSystem::generateConstraints(Expr *expr, DeclContext *dc) {
41834129

41844130
Type ConstraintSystem::generateConstraints(
41854131
Pattern *pattern, ConstraintLocatorBuilder locator,
4186-
bool bindPatternVarsOneWay, PatternBindingDecl *patternBinding,
4132+
PatternBindingDecl *patternBinding,
41874133
unsigned patternIndex) {
41884134
ConstraintGenerator cg(*this, nullptr);
4189-
auto ty = cg.getTypeForPattern(pattern, locator, bindPatternVarsOneWay,
4190-
patternBinding, patternIndex);
4135+
auto ty = cg.getTypeForPattern(pattern, locator, patternBinding, patternIndex);
41914136
assert(ty);
41924137

41934138
// Gather the ExprPatterns, and form a conjunction for their expressions.
@@ -4248,8 +4193,7 @@ bool ConstraintSystem::generateConstraints(StmtCondition condition,
42484193
return true;
42494194

42504195
auto target = SyntacticElementTarget::forInitialization(
4251-
condElement.getInitializer(), dc, Type(), pattern,
4252-
/*bindPatternVarsOneWay=*/true);
4196+
condElement.getInitializer(), dc, Type(), pattern);
42534197
if (generateConstraints(target, FreeTypeVariableBinding::Disallow))
42544198
return true;
42554199

0 commit comments

Comments
 (0)