Skip to content

Commit b9e27d7

Browse files
author
Benjamin Driscoll
committed
[TypeChecker] Fix @_typeEraser, @resultBuilder, and Optional bugs from opaque result type changes
1. Move @_typeEraser and Optional initialization logic from `generateConstraints` to `typeCheckExpression` so that `setContextualType` is called with the right type. 2. Push retrieval of the contextual type down from the `generateConstraints` overload where it currently happens into `addContextualConversionConstraint` since result builders do not end up invoking the former. 3. Add `setContextualType` where needed to fix the assertion failures caused by `addContextualConversionConstraint` trying to get the contextual type.
1 parent 560110b commit b9e27d7

File tree

6 files changed

+116
-85
lines changed

6 files changed

+116
-85
lines changed

‎include/swift/Sema/ConstraintSystem.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3012,6 +3012,11 @@ class ConstraintSystem {
30123012
return E;
30133013
}
30143014

3015+
/// Register `node`'s contextual type, applying the necessary transformations
3016+
/// for the contextual type to be used in solving (e.g. opening opaque types
3017+
/// and creating type variables for placeholders).
3018+
///
3019+
/// May only be called once per AST node.
30153020
void setContextualType(ASTNode node, TypeLoc T,
30163021
ContextualTypePurpose purpose);
30173022

@@ -3022,6 +3027,7 @@ class ConstraintSystem {
30223027
return known->second;
30233028
}
30243029

3030+
/// Get the transformed contextual type created by `setContextualType`.
30253031
Type getContextualType(ASTNode node) const {
30263032
auto result = getContextualTypeInfo(node);
30273033
if (result)
@@ -3288,8 +3294,9 @@ class ConstraintSystem {
32883294
void addConstraint(Requirement req, ConstraintLocatorBuilder locator,
32893295
bool isFavored = false);
32903296

3291-
/// Add the appropriate constraint for a contextual conversion.
3292-
void addContextualConversionConstraint(Expr *expr, Type conversionType,
3297+
/// Add the appropriate constraint for a contextual conversion. Uses
3298+
/// `ConstraintSystem::getContextualType` to determine the conversion type.
3299+
void addContextualConversionConstraint(Expr *expr,
32933300
ContextualTypePurpose purpose);
32943301

32953302
/// Convenience function to pass an \c ArrayRef to \c addJoinConstraint

‎lib/Sema/BuilderTransform.cpp

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -918,13 +918,19 @@ class BuilderClosureVisitor
918918
}
919919

920920
if (cs) {
921-
SolutionApplicationTarget target(
922-
throwStmt->getSubExpr(), dc, CTP_ThrowStmt, exnType,
923-
/*isDiscarded=*/false);
924-
if (cs->generateConstraints(target, FreeTypeVariableBinding::Disallow))
925-
hadError = true;
921+
SolutionApplicationTarget target(throwStmt->getSubExpr(), dc,
922+
CTP_ThrowStmt, exnType,
923+
/*isDiscarded=*/false);
924+
925+
// Needed for constraint generation
926+
cs->setContextualType(target.getAsExpr(),
927+
target.getExprContextualTypeLoc(),
928+
target.getExprContextualTypePurpose());
926929

927-
cs->setSolutionApplicationTarget(throwStmt, target);
930+
if (cs->generateConstraints(target, FreeTypeVariableBinding::Disallow))
931+
hadError = true;
932+
933+
cs->setSolutionApplicationTarget(throwStmt, target);
928934
}
929935

930936
return nullptr;
@@ -1636,7 +1642,8 @@ Optional<BraceStmt *> TypeChecker::applyResultBuilderBodyTransform(
16361642
ConstraintKind resultConstraintKind = ConstraintKind::Conversion;
16371643
if (auto opaque = resultContextType->getAs<OpaqueTypeArchetypeType>()) {
16381644
if (opaque->getDecl()->isOpaqueReturnTypeOfFunction(func)) {
1639-
resultConstraintKind = ConstraintKind::OpaqueUnderlyingType;
1645+
// FIXME [OPAQUE SUPPORT] should this be `Bind`?
1646+
resultConstraintKind = ConstraintKind::Equal;
16401647
}
16411648
}
16421649

@@ -1805,9 +1812,6 @@ ConstraintSystem::matchResultBuilder(
18051812
}
18061813
}
18071814

1808-
Type transformedType = getType(applied->returnExpr);
1809-
assert(transformedType && "Missing type");
1810-
18111815
// Record the transformation.
18121816
assert(std::find_if(
18131817
resultBuilderTransformed.begin(),
@@ -1830,8 +1834,10 @@ ConstraintSystem::matchResultBuilder(
18301834
}
18311835

18321836
// Bind the body result type to the type of the transformed expression.
1833-
addConstraint(bodyResultConstraintKind, transformedType, bodyResultType,
1834-
locator);
1837+
setContextualType(applied->returnExpr, TypeLoc::withoutLoc(bodyResultType),
1838+
CTP_ReturnStmt);
1839+
addContextualConversionConstraint(applied->returnExpr, CTP_ReturnStmt);
1840+
18351841
return getTypeMatchSuccess();
18361842
}
18371843

‎lib/Sema/CSGen.cpp

Lines changed: 46 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -3771,12 +3771,14 @@ generateForEachStmtConstraints(
37713771
SolutionApplicationTarget whereTarget(
37723772
forEachStmtInfo.whereExpr, dc, CTP_Condition, boolType,
37733773
/*isDiscarded=*/false);
3774-
if (cs.generateConstraints(whereTarget, FreeTypeVariableBinding::Disallow))
3775-
return None;
37763774

3775+
// Needed for constraint generation
37773776
cs.setContextualType(forEachStmtInfo.whereExpr,
37783777
TypeLoc::withoutLoc(boolType), CTP_Condition);
37793778

3779+
if (cs.generateConstraints(whereTarget, FreeTypeVariableBinding::Disallow))
3780+
return None;
3781+
37803782
forEachStmtInfo.whereExpr = whereTarget.getAsExpr();
37813783
}
37823784

@@ -3794,23 +3796,6 @@ bool ConstraintSystem::generateConstraints(
37943796
SolutionApplicationTarget &target,
37953797
FreeTypeVariableBinding allowFreeTypeVariables) {
37963798
if (Expr *expr = target.getAsExpr()) {
3797-
// If the target requires an optional of some type, form a new appropriate
3798-
// type variable and update the target's type with an optional of that
3799-
// type variable.
3800-
if (target.isOptionalSomePatternInit()) {
3801-
assert(!target.getExprContextualType() &&
3802-
"some pattern cannot have contextual type pre-configured");
3803-
auto *convertTypeLocator = getConstraintLocator(
3804-
expr, LocatorPathElt::ContextualType(
3805-
target.getExprContextualTypePurpose()));
3806-
Type var = createTypeVariable(convertTypeLocator, TVO_CanBindToNoEscape);
3807-
target.setExprConversionType(TypeChecker::getOptionalType(expr->getLoc(), var));
3808-
}
3809-
3810-
expr = buildTypeErasedExpr(expr, target.getDeclContext(),
3811-
target.getExprContextualType(),
3812-
target.getExprContextualTypePurpose());
3813-
38143799
// Generate constraints for the main system.
38153800
expr = generateConstraints(expr, target.getDeclContext());
38163801
if (!expr)
@@ -3819,53 +3804,53 @@ bool ConstraintSystem::generateConstraints(
38193804

38203805
// If there is a type that we're expected to convert to, add the conversion
38213806
// constraint.
3822-
if (Type convertType = target.getExprConversionType()) {
3823-
// FIXME [OPAQUE SUPPORT]
3824-
if (Type contextType = getContextualType(target.getAsExpr()))
3825-
convertType = contextType;
3826-
3807+
if (Type convertType = getContextualType(target.getAsExpr())) {
38273808
// Determine whether we know more about the contextual type.
38283809
ContextualTypePurpose ctp = target.getExprContextualTypePurpose();
3829-
auto *convertTypeLocator =
3830-
getConstraintLocator(expr, LocatorPathElt::ContextualType(ctp));
3831-
3832-
auto getLocator = [&](Type ty) -> ConstraintLocator * {
3833-
// If we have a placeholder originating from a PlaceholderTypeRepr,
3834-
// tack that on to the locator.
3835-
if (auto *placeholderTy = ty->getAs<PlaceholderType>())
3836-
if (auto *placeholderRepr = placeholderTy->getOriginator()
3837-
.dyn_cast<PlaceholderTypeRepr *>())
3838-
return getConstraintLocator(
3839-
convertTypeLocator,
3840-
LocatorPathElt::PlaceholderType(placeholderRepr));
3841-
return convertTypeLocator;
3842-
};
3810+
// auto *convertTypeLocator =
3811+
// getConstraintLocator(expr, LocatorPathElt::ContextualType(ctp));
3812+
3813+
// FIXME [OPAQUE SUPPORT]: most of this got moved to `setContextualType`,
3814+
// but `UnresolvedTypes` aren't handled there, so they are broken now!!!
3815+
3816+
// auto getLocator = [&](Type ty) -> ConstraintLocator * {
3817+
// // If we have a placeholder originating from a PlaceholderTypeRepr,
3818+
// // tack that on to the locator.
3819+
// if (auto *placeholderTy = ty->getAs<PlaceholderType>())
3820+
// if (auto *placeholderRepr = placeholderTy->getOriginator()
3821+
// .dyn_cast<PlaceholderTypeRepr *>())
3822+
// return getConstraintLocator(
3823+
// convertTypeLocator,
3824+
// LocatorPathElt::PlaceholderType(placeholderRepr));
3825+
// return convertTypeLocator;
3826+
//};
38433827

38443828
// Substitute type variables in for placeholder types (and unresolved
38453829
// types, if allowed).
3846-
if (allowFreeTypeVariables == FreeTypeVariableBinding::UnresolvedType) {
3847-
convertType = convertType.transform([&](Type type) -> Type {
3848-
if (type->is<UnresolvedType>() || type->is<PlaceholderType>()) {
3849-
return createTypeVariable(getLocator(type),
3850-
TVO_CanBindToNoEscape |
3851-
TVO_PrefersSubtypeBinding |
3852-
TVO_CanBindToHole);
3853-
}
3854-
return type;
3855-
});
3856-
} else {
3857-
convertType = convertType.transform([&](Type type) -> Type {
3858-
if (type->is<PlaceholderType>()) {
3859-
return createTypeVariable(getLocator(type),
3860-
TVO_CanBindToNoEscape |
3861-
TVO_PrefersSubtypeBinding |
3862-
TVO_CanBindToHole);
3863-
}
3864-
return type;
3865-
});
3866-
}
3867-
3868-
addContextualConversionConstraint(expr, convertType, ctp);
3830+
// if (allowFreeTypeVariables == FreeTypeVariableBinding::UnresolvedType)
3831+
// {
3832+
// convertType = convertType.transform([&](Type type) -> Type {
3833+
// if (type->is<UnresolvedType>() || type->is<PlaceholderType>()) {
3834+
// return createTypeVariable(getLocator(type),
3835+
// TVO_CanBindToNoEscape |
3836+
// TVO_PrefersSubtypeBinding |
3837+
// TVO_CanBindToHole);
3838+
// }
3839+
// return type;
3840+
// });
3841+
//} else {
3842+
// convertType = convertType.transform([&](Type type) -> Type {
3843+
// if (type->is<PlaceholderType>()) {
3844+
// return createTypeVariable(getLocator(type),
3845+
// TVO_CanBindToNoEscape |
3846+
// TVO_PrefersSubtypeBinding |
3847+
// TVO_CanBindToHole);
3848+
// }
3849+
// return type;
3850+
// });
3851+
//}
3852+
3853+
addContextualConversionConstraint(expr, ctp);
38693854
}
38703855

38713856
// For an initialization target, generate constraints for the pattern.

‎lib/Sema/CSSimplify.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11959,7 +11959,8 @@ void ConstraintSystem::addConstraint(ConstraintKind kind, Type first,
1195911959
}
1196011960

1196111961
void ConstraintSystem::addContextualConversionConstraint(
11962-
Expr *expr, Type conversionType, ContextualTypePurpose purpose) {
11962+
Expr *expr, ContextualTypePurpose purpose) {
11963+
auto conversionType = getContextualType(expr);
1196311964
if (conversionType.isNull())
1196411965
return;
1196511966

‎lib/Sema/ConstraintSystem.cpp

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5705,20 +5705,28 @@ void ConstraintSystem::setContextualType(ASTNode node, TypeLoc T,
57055705
assert(contextualTypes.count(node) == 0 &&
57065706
"Already set this contextual type");
57075707

5708-
// Open any opaque archetypes
57095708
auto contextualType = T.getType();
5710-
if (contextualType && contextualType->hasOpaqueArchetype()) {
5711-
T.setType(contextualType.transform([&](Type type) -> Type {
5709+
if (contextualType) {
5710+
contextualType = contextualType.transform([&](Type type) -> Type {
5711+
// TODO [OPAQUE SUPPORT]: perhaps we could provide better locators here?
5712+
auto *locator =
5713+
getConstraintLocator(node, LocatorPathElt::ContextualType(purpose));
5714+
57125715
if (type->is<OpaqueTypeArchetypeType>()) {
5713-
// TODO [OPAQUE SUPPORT]: perhaps we could provide a better locator here?
5714-
auto *locator =
5715-
getConstraintLocator(node, LocatorPathElt::ContextualType(purpose));
57165716
return openOpaqueType(type, locator);
57175717
}
5718+
5719+
if (type->is<PlaceholderType>()) {
5720+
return createTypeVariable(locator, TVO_CanBindToNoEscape |
5721+
TVO_PrefersSubtypeBinding |
5722+
TVO_CanBindToHole);
5723+
}
5724+
57185725
return type;
5719-
}));
5726+
});
57205727
}
57215728

5729+
T.setType(contextualType);
57225730
contextualTypes[node] = {T, purpose};
57235731
}
57245732

‎lib/Sema/TypeCheckConstraints.cpp

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -359,8 +359,32 @@ TypeChecker::typeCheckExpression(
359359

360360
ConstraintSystem cs(dc, csOptions);
361361

362-
// Tell the constraint system what the contextual type is. This informs
363-
// diagnostics and is a hint for various performance optimizations.
362+
// If the target requires an optional of some type, form a new appropriate
363+
// type variable and update the target's type with an optional of that
364+
// type variable.
365+
if (target.isOptionalSomePatternInit()) {
366+
assert(!target.getExprContextualType() &&
367+
"some pattern cannot have contextual type pre-configured");
368+
auto *convertTypeLocator = cs.getConstraintLocator(
369+
expr,
370+
LocatorPathElt::ContextualType(target.getExprContextualTypePurpose()));
371+
Type var = cs.createTypeVariable(convertTypeLocator, TVO_CanBindToNoEscape);
372+
373+
// TODO [OPAQUE SUPPORT]: we could probably get rid of expr conversion
374+
// type, now that most things depend on `getContextualType`.
375+
target.setExprConversionType(
376+
TypeChecker::getOptionalType(expr->getLoc(), var));
377+
}
378+
379+
// Must happen before `setContextualType` so that any opaque result types get
380+
// the correct underlying type.
381+
expr = cs.buildTypeErasedExpr(expr, target.getDeclContext(),
382+
target.getExprContextualType(),
383+
target.getExprContextualTypePurpose());
384+
385+
// Tell the constraint system what the contextual type is. This is used in
386+
// constraint generation, informs diagnostics, and is a hint for various
387+
// performance optimizations.
364388
cs.setContextualType(
365389
expr,
366390
target.getExprContextualTypeLoc(),

0 commit comments

Comments
 (0)