Skip to content

Commit c0787f3

Browse files
committed
Sema: Fix soundness hole with variable initializers and opaque return types
Just because the type of the initializer expression is an opaque return type, does not mean it is the opaque return type *for the variable being initialized*. It looks like there is a bit of duplicated logic and layering violations going on so I only fixed one caller of openOpaqueType(). This addresses the test case in the issue. For the remaining calls I added FIXMEs to investigate what is going on. Fixes swiftlang#73245. Fixes rdar://127180656.
1 parent 5945a89 commit c0787f3

File tree

8 files changed

+65
-24
lines changed

8 files changed

+65
-24
lines changed

include/swift/AST/Decl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3492,7 +3492,7 @@ class OpaqueTypeDecl final :
34923492
///
34933493
/// This is more complex than just checking `getNamingDecl` because the
34943494
/// function could also be the getter of a storage declaration.
3495-
bool isOpaqueReturnTypeOfFunction(const AbstractFunctionDecl *func) const;
3495+
bool isOpaqueReturnTypeOf(const Decl *owner) const;
34963496

34973497
/// Get the ordinal of the anonymous opaque parameter of this decl with type
34983498
/// repr `repr`, as introduce implicitly by an occurrence of "some" in return

include/swift/Sema/ConstraintSystem.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4223,6 +4223,11 @@ class ConstraintSystem {
42234223
Type openOpaqueType(OpaqueTypeArchetypeType *type,
42244224
ConstraintLocatorBuilder locator);
42254225

4226+
/// Recurse over the given type and open any opaque archetype types.
4227+
Type openOpaqueType(Type type, ContextualTypePurpose context,
4228+
ConstraintLocatorBuilder locator,
4229+
Decl *ownerDecl);
4230+
42264231
/// "Open" a pack expansion type by replacing it with a type variable,
42274232
/// opening its pattern and shape types and connecting them to the
42284233
/// aforementioned variable via special constraints.
@@ -4240,10 +4245,6 @@ class ConstraintSystem {
42404245
ASSERT(erased);
42414246
}
42424247

4243-
/// Recurse over the given type and open any opaque archetype types.
4244-
Type openOpaqueType(Type type, ContextualTypePurpose context,
4245-
ConstraintLocatorBuilder locator);
4246-
42474248
/// "Open" the given function type.
42484249
///
42494250
/// If the function type is non-generic, this is equivalent to calling

lib/AST/Decl.cpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9990,18 +9990,19 @@ OpaqueTypeDecl *OpaqueTypeDecl::get(
99909990
OpaqueReturnTypeReprs);
99919991
}
99929992

9993-
bool OpaqueTypeDecl::isOpaqueReturnTypeOfFunction(
9994-
const AbstractFunctionDecl *func) const {
9995-
// Either the function is declared with its own opaque return type...
9996-
if (getNamingDecl() == func)
9993+
bool OpaqueTypeDecl::isOpaqueReturnTypeOf(const Decl *ownerDecl) const {
9994+
if (getNamingDecl() == ownerDecl)
99979995
return true;
99989996

9999-
// ...or the function is a getter for a property or subscript with an
10000-
// opaque return type.
10001-
if (auto accessor = dyn_cast<AccessorDecl>(func)) {
9997+
if (auto accessor = dyn_cast<AccessorDecl>(ownerDecl)) {
100029998
return accessor->isGetter() && getNamingDecl() == accessor->getStorage();
100039999
}
1000410000

10001+
if (auto patternBinding = dyn_cast<PatternBindingDecl>(ownerDecl)) {
10002+
if (auto *varDecl = dyn_cast<VarDecl>(getNamingDecl()))
10003+
return varDecl->getParentPatternBinding() == patternBinding;
10004+
}
10005+
1000510006
return false;
1000610007
}
1000710008

lib/Sema/BuilderTransform.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -956,7 +956,7 @@ TypeChecker::applyResultBuilderBodyTransform(FuncDecl *func, Type builderType) {
956956
// result type of this function.
957957
ConstraintKind resultConstraintKind = ConstraintKind::Conversion;
958958
if (auto opaque = resultContextType->getAs<OpaqueTypeArchetypeType>()) {
959-
if (opaque->getDecl()->isOpaqueReturnTypeOfFunction(func)) {
959+
if (opaque->getDecl()->isOpaqueReturnTypeOf(func)) {
960960
resultConstraintKind = ConstraintKind::Equal;
961961
}
962962
}

lib/Sema/CSGen.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2129,8 +2129,9 @@ namespace {
21292129
// Now that we know we're actually going to use the type, get the
21302130
// version for use in a constraint.
21312131
contextualType = CS.getContextualType(expr, /*forConstraint=*/true);
2132+
// FIXME: This is the wrong place to be opening the opaque type.
21322133
contextualType = CS.openOpaqueType(
2133-
contextualType, contextualPurpose, locator);
2134+
contextualType, contextualPurpose, locator, /*ownerDecl=*/nullptr);
21342135
Type arrayElementType = contextualType->isArrayType();
21352136
CS.addConstraint(ConstraintKind::LiteralConformsTo, contextualType,
21362137
arrayProto->getDeclaredInterfaceType(),
@@ -2242,8 +2243,10 @@ namespace {
22422243
// Now that we know we're actually going to use the type, get the
22432244
// version for use in a constraint.
22442245
contextualType = CS.getContextualType(expr, /*forConstraint=*/true);
2246+
// FIXME: This is the wrong place to be opening the opaque type.
22452247
auto openedType =
2246-
CS.openOpaqueType(contextualType, contextualPurpose, locator);
2248+
CS.openOpaqueType(contextualType, contextualPurpose, locator,
2249+
/*ownerDecl=*/nullptr);
22472250
auto dictionaryKeyValue =
22482251
ConstraintSystem::isDictionaryType(openedType);
22492252
Type contextualDictionaryKeyType;
@@ -2814,7 +2817,8 @@ namespace {
28142817

28152818
Type replacedType = CS.replaceInferableTypesWithTypeVars(type, locator);
28162819
Type openedType =
2817-
CS.openOpaqueType(replacedType, CTP_Initialization, locator);
2820+
CS.openOpaqueType(replacedType, CTP_Initialization, locator,
2821+
patternBinding);
28182822
assert(openedType);
28192823

28202824
auto *subPattern = cast<TypedPattern>(pattern)->getSubPattern();

lib/Sema/CSSimplify.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15927,7 +15927,9 @@ void ConstraintSystem::addContextualConversionConstraint(
1592715927
}
1592815928

1592915929
// Add the constraint.
15930-
auto openedType = openOpaqueType(conversionType, purpose, locator);
15930+
// FIXME: This is the wrong place to be opening the opaque type.
15931+
auto openedType = openOpaqueType(conversionType, purpose, locator,
15932+
/*ownerDecl=*/nullptr);
1593115933
addConstraint(constraintKind, getType(expr), openedType, locator,
1593215934
/*isFavored*/ true);
1593315935
}

lib/Sema/TypeOfReference.cpp

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -359,23 +359,31 @@ Type ConstraintSystem::openOpaqueType(OpaqueTypeArchetypeType *opaque,
359359
}
360360

361361
Type ConstraintSystem::openOpaqueType(Type type, ContextualTypePurpose context,
362-
ConstraintLocatorBuilder locator) {
362+
ConstraintLocatorBuilder locator,
363+
Decl *ownerDecl) {
364+
// FIXME: Require non-null ownerDecl and fix remaining callers
365+
// ASSERT(ownerDecl);
366+
363367
// Early return if `type` is `NULL` or if there are no opaque archetypes (in
364368
// which case there is certainly nothing for us to do).
365369
if (!type || !type->hasOpaqueArchetype())
366370
return type;
367371

368-
if (!(context == CTP_Initialization || context == CTP_ReturnStmt))
372+
if (context != CTP_Initialization && context != CTP_ReturnStmt)
369373
return type;
370374

371375
auto shouldOpen = [&](OpaqueTypeArchetypeType *opaqueType) {
372-
if (context != CTP_ReturnStmt)
373-
return true;
376+
if (context == CTP_Initialization) {
377+
if (!ownerDecl)
378+
return true;
374379

375-
if (auto *func = dyn_cast<AbstractFunctionDecl>(DC))
376-
return opaqueType->getDecl()->isOpaqueReturnTypeOfFunction(func);
380+
return opaqueType->getDecl()->isOpaqueReturnTypeOf(ownerDecl);
381+
} else {
382+
if (auto *func = dyn_cast<AbstractFunctionDecl>(DC))
383+
return opaqueType->getDecl()->isOpaqueReturnTypeOf(func);
377384

378-
return true;
385+
return true;
386+
}
379387
};
380388

381389
return type.transformRec([&](Type type) -> std::optional<Type> {
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// RUN: %target-typecheck-verify-swift
2+
3+
protocol P {
4+
associatedtype A
5+
func f() -> A
6+
}
7+
8+
struct S: P {
9+
func f() -> some Any { return 123 }
10+
}
11+
12+
let x1: S.A = "hello world"
13+
// expected-error@-1 {{cannot convert value of type 'String' to specified type 'S.A' (aka 'some Any')}}
14+
15+
let x2: S.A = { return "hello world" }()
16+
// expected-error@-1 {{cannot convert value of type 'String' to closure result type 'S.A' (aka 'some Any')}}
17+
let x3: () -> S.A = { return "hello world" }
18+
// expected-error@-1 {{cannot convert value of type 'String' to closure result type 'S.A' (aka 'some Any')}}
19+
20+
let x4: some Any = 123
21+
let x5: some Any = { return 123 }()
22+
23+
// FIXME: This should work
24+
let x6: () -> some Any = { return 123 }
25+
// expected-error@-1 {{cannot convert value of type 'Int' to closure result type 'some Any'}}

0 commit comments

Comments
 (0)