Skip to content

Commit 775cca6

Browse files
committed
Constrain type checking of expressions in optional pattern bindings
so that they must result in an optional type. Add constraint locator path for identifying constraints/variables that are part of the convert type passed into the system.
1 parent 14f7799 commit 775cca6

File tree

7 files changed

+92
-5
lines changed

7 files changed

+92
-5
lines changed

lib/Sema/CSDiag.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,11 @@ void constraints::simplifyLocator(Expr *&anchor,
312312
}
313313
}
314314
break;
315+
316+
case ConstraintLocator::ContextualType:
317+
// This was just for identifying purposes, strip it off.
318+
path = path.slice(1);
319+
continue;
315320

316321
default:
317322
// FIXME: Lots of other cases to handle.

lib/Sema/CSSolver.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1413,16 +1413,19 @@ ConstraintSystem::solveImpl(Expr *&expr,
14131413
if (getContextualTypePurpose() == CTP_YieldByReference)
14141414
constraintKind = ConstraintKind::Bind;
14151415

1416+
auto *convertTypeLocator = getConstraintLocator(
1417+
getConstraintLocator(expr), ConstraintLocator::ContextualType);
1418+
14161419
if (allowFreeTypeVariables == FreeTypeVariableBinding::UnresolvedType) {
14171420
convertType = convertType.transform([&](Type type) -> Type {
14181421
if (type->is<UnresolvedType>())
1419-
return createTypeVariable(getConstraintLocator(expr));
1422+
return createTypeVariable(convertTypeLocator);
14201423
return type;
14211424
});
14221425
}
14231426

14241427
addConstraint(constraintKind, getType(expr), convertType,
1425-
getConstraintLocator(expr), /*isFavored*/ true);
1428+
convertTypeLocator, /*isFavored*/ true);
14261429
}
14271430

14281431
// Notify the listener that we've built the constraint system.

lib/Sema/ConstraintLocator.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ void ConstraintLocator::Profile(llvm::FoldingSetNodeID &id, Expr *anchor,
8080
case TypeParameterRequirement:
8181
case ImplicitlyUnwrappedDisjunctionChoice:
8282
case DynamicLookupResult:
83+
case ContextualType:
8384
if (unsigned numValues = numNumericValuesInPathElement(elt.getKind())) {
8485
id.AddInteger(elt.getValue());
8586
if (numValues > 1)
@@ -258,6 +259,10 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) {
258259
case DynamicLookupResult:
259260
out << "dynamic lookup result";
260261
break;
262+
263+
case ContextualType:
264+
out << "contextual type";
265+
break;
261266
}
262267
}
263268

lib/Sema/ConstraintLocator.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,10 @@ class ConstraintLocator : public llvm::FoldingSetNode {
131131
TypeParameterRequirement,
132132
/// \brief Locator for a binding from an IUO disjunction choice.
133133
ImplicitlyUnwrappedDisjunctionChoice,
134-
/// \brief A result of an expressoin involving dynamic lookup.
134+
/// \brief A result of an expression involving dynamic lookup.
135135
DynamicLookupResult,
136+
/// \brief The desired contextual type passed in to the constraint system.
137+
ContextualType,
136138
};
137139

138140
/// \brief Determine the number of numeric values used for the given path
@@ -167,6 +169,7 @@ class ConstraintLocator : public llvm::FoldingSetNode {
167169
case OpenedGeneric:
168170
case ImplicitlyUnwrappedDisjunctionChoice:
169171
case DynamicLookupResult:
172+
case ContextualType:
170173
return 0;
171174

172175
case GenericArgument:
@@ -231,6 +234,7 @@ class ConstraintLocator : public llvm::FoldingSetNode {
231234
case TypeParameterRequirement:
232235
case ImplicitlyUnwrappedDisjunctionChoice:
233236
case DynamicLookupResult:
237+
case ContextualType:
234238
return 0;
235239

236240
case FunctionArgument:

lib/Sema/TypeCheckConstraints.cpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2040,9 +2040,18 @@ Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
20402040
if (options.contains(TypeCheckExprFlags::AllowUnresolvedTypeVariables))
20412041
allowFreeTypeVariables = FreeTypeVariableBinding::UnresolvedType;
20422042

2043+
Type convertTo = convertType.getType();
2044+
if (options.contains(TypeCheckExprFlags::ExpressionTypeMustBeOptional)) {
2045+
assert(!convertTo && "convertType and type check options conflict");
2046+
auto *convertTypeLocator = cs.getConstraintLocator(
2047+
cs.getConstraintLocator(expr), ConstraintLocator::ContextualType);
2048+
Type var = cs.createTypeVariable(convertTypeLocator);
2049+
convertTo = getOptionalType(expr->getLoc(), var);
2050+
}
2051+
20432052
// Attempt to solve the constraint system.
20442053
SmallVector<Solution, 4> viable;
2045-
if (cs.solve(expr, convertType.getType(), listener, viable,
2054+
if (cs.solve(expr, convertTo, listener, viable,
20462055
allowFreeTypeVariables))
20472056
return Type();
20482057

@@ -2439,6 +2448,8 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,
24392448

24402449
TypeLoc contextualType;
24412450
auto contextualPurpose = CTP_Unused;
2451+
TypeCheckExprOptions flags = TypeCheckExprFlags::ConvertTypeIsOnlyAHint;
2452+
24422453
if (pattern->hasType()) {
24432454
contextualType = TypeLoc::withoutLoc(pattern->getType());
24442455
contextualPurpose = CTP_Initialization;
@@ -2453,10 +2464,11 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,
24532464
if (isa<NamedPattern>(inner) || isa<AnyPattern>(inner))
24542465
contextualType = typedPattern->getTypeLoc();
24552466
}
2467+
} else if (isa<OptionalSomePattern>(pattern)) {
2468+
flags |= TypeCheckExprFlags::ExpressionTypeMustBeOptional;
24562469
}
24572470

24582471
// Type-check the initializer.
2459-
TypeCheckExprOptions flags = TypeCheckExprFlags::ConvertTypeIsOnlyAHint;
24602472
if (skipApplyingSolution)
24612473
flags |= TypeCheckExprFlags::SkipApplyingSolution;
24622474

lib/Sema/TypeChecker.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,10 @@ enum class TypeCheckExprFlags {
261261

262262
/// This is an inout yield.
263263
IsInOutYield = 0x200,
264+
265+
/// If set, a conversion constraint should be specfied so that the result of
266+
/// the expression is an optional type.
267+
ExpressionTypeMustBeOptional = 0x400,
264268
};
265269

266270
using TypeCheckExprOptions = OptionSet<TypeCheckExprFlags>;

test/Constraints/patterns.swift

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,3 +351,57 @@ func testOne() {
351351
if case One.E.SomeError = error {} // expected-error{{generic enum type 'One.E' is ambiguous without explicit generic parameters when matching value of type 'Error'}}
352352
}
353353
}
354+
355+
// SR-8347
356+
// constrain initializer expressions of optional some pattern bindings to be optional
357+
func test8347() -> String {
358+
struct C {
359+
subscript (s: String) -> String? {
360+
return ""
361+
}
362+
subscript (s: String) -> [String] {
363+
return [""]
364+
}
365+
366+
func f() -> String? {
367+
return ""
368+
}
369+
func f() -> Int {
370+
return 3
371+
}
372+
373+
func g() -> String {
374+
return ""
375+
}
376+
377+
func h() -> String {
378+
return ""
379+
}
380+
func h() -> Double {
381+
return 3.0
382+
}
383+
func h() -> Int? { //expected-note{{found this candidate}}
384+
return 2
385+
}
386+
func h() -> Float? { //expected-note{{found this candidate}}
387+
return nil
388+
}
389+
390+
}
391+
392+
let c = C()
393+
if let s = c[""] {
394+
return s
395+
}
396+
if let s = c.f() {
397+
return s
398+
}
399+
if let s = c.g() { //expected-error{{initializer for conditional binding must have Optional type, not 'String'}}
400+
return s
401+
}
402+
if let s = c.h() { //expected-error{{ambiguous use of 'h()'}}
403+
return s
404+
}
405+
}
406+
407+

0 commit comments

Comments
 (0)