Skip to content

Commit dad2637

Browse files
authored
Merge pull request #85193 from hamishknight/wrapping-paper
[CS] A couple of property wrapper fixes
2 parents 13937fd + 260d10f commit dad2637

File tree

9 files changed

+141
-125
lines changed

9 files changed

+141
-125
lines changed

include/swift/Sema/ConstraintSystem.h

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3973,21 +3973,17 @@ class ConstraintSystem {
39733973
isRepresentativeFor(TypeVariableType *typeVar,
39743974
ConstraintLocator::PathElementKind kind) const;
39753975

3976-
/// Gets the VarDecl associated with resolvedOverload, and the type of the
3977-
/// projection if the decl has an associated property wrapper with a projectedValue.
3978-
std::optional<std::pair<VarDecl *, Type>>
3979-
getPropertyWrapperProjectionInfo(SelectedOverload resolvedOverload);
3980-
3981-
/// Gets the VarDecl associated with resolvedOverload, and the type of the
3982-
/// backing storage if the decl has an associated property wrapper.
3983-
std::optional<std::pair<VarDecl *, Type>>
3984-
getPropertyWrapperInformation(SelectedOverload resolvedOverload);
3985-
3986-
/// Gets the VarDecl, and the type of the type property that it wraps if
3987-
/// resolved overload has a decl which is the backing storage for a
3976+
/// Gets the the type of the projection if the decl has an associated property
3977+
/// wrapper with a projectedValue.
3978+
Type getPropertyWrapperProjectionType(SelectedOverload resolvedOverload);
3979+
3980+
/// Gets the type of the backing storage if the decl has an associated
39883981
/// property wrapper.
3989-
std::optional<std::pair<VarDecl *, Type>>
3990-
getWrappedPropertyInformation(SelectedOverload resolvedOverload);
3982+
Type getPropertyWrapperBackingType(SelectedOverload resolvedOverload);
3983+
3984+
/// Gets the type of a wrapped property if resolved overload has
3985+
/// a decl which is the backing storage for a property wrapper.
3986+
Type getWrappedPropertyType(SelectedOverload resolvedOverload);
39913987

39923988
/// Merge the equivalence sets of the two type variables.
39933989
///

lib/Sema/CSSimplify.cpp

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4825,14 +4825,19 @@ static ConstraintFix *fixPropertyWrapperFailure(
48254825
if (!resolvedOverload)
48264826
return nullptr;
48274827

4828+
auto *decl = resolvedOverload->choice.getDeclOrNull();
4829+
auto *VD = dyn_cast_or_null<VarDecl>(decl);
4830+
if (!VD)
4831+
return nullptr;
4832+
48284833
enum class Fix : uint8_t {
48294834
ProjectedValue,
48304835
PropertyWrapper,
48314836
WrappedValue,
48324837
};
48334838

4834-
auto applyFix = [&](Fix fix, VarDecl *decl, Type type) -> ConstraintFix * {
4835-
if (!decl->hasInterfaceType() || !type)
4839+
auto applyFix = [&](Fix fix, Type type) -> ConstraintFix * {
4840+
if (!VD->hasInterfaceType() || !type)
48364841
return nullptr;
48374842

48384843
if (baseTy->isEqual(type))
@@ -4841,40 +4846,34 @@ static ConstraintFix *fixPropertyWrapperFailure(
48414846
if (baseTy->is<TypeVariableType>() || type->is<TypeVariableType>())
48424847
return nullptr;
48434848

4844-
if (!attemptFix(*resolvedOverload, decl, type))
4849+
if (!attemptFix(*resolvedOverload, VD, type))
48454850
return nullptr;
48464851

48474852
switch (fix) {
48484853
case Fix::ProjectedValue:
48494854
case Fix::PropertyWrapper:
4850-
return UsePropertyWrapper::create(cs, decl, fix == Fix::ProjectedValue,
4851-
baseTy, toType.value_or(type),
4852-
locator);
4855+
return UsePropertyWrapper::create(cs, VD, fix == Fix::ProjectedValue,
4856+
baseTy, toType.value_or(type), locator);
48534857

48544858
case Fix::WrappedValue:
4855-
return UseWrappedValue::create(cs, decl, baseTy, toType.value_or(type),
4859+
return UseWrappedValue::create(cs, VD, baseTy, toType.value_or(type),
48564860
locator);
48574861
}
48584862
llvm_unreachable("Unhandled Fix type in switch");
48594863
};
48604864

4861-
if (auto projection =
4862-
cs.getPropertyWrapperProjectionInfo(*resolvedOverload)) {
4863-
if (auto *fix = applyFix(Fix::ProjectedValue, projection->first,
4864-
projection->second))
4865+
if (auto projectTy = cs.getPropertyWrapperProjectionType(*resolvedOverload)) {
4866+
if (auto *fix = applyFix(Fix::ProjectedValue, projectTy))
48654867
return fix;
48664868
}
48674869

4868-
if (auto wrapper = cs.getPropertyWrapperInformation(*resolvedOverload)) {
4869-
if (auto *fix =
4870-
applyFix(Fix::PropertyWrapper, wrapper->first, wrapper->second))
4870+
if (auto backingTy = cs.getPropertyWrapperBackingType(*resolvedOverload)) {
4871+
if (auto *fix = applyFix(Fix::PropertyWrapper, backingTy))
48714872
return fix;
48724873
}
48734874

4874-
if (auto wrappedProperty =
4875-
cs.getWrappedPropertyInformation(*resolvedOverload)) {
4876-
if (auto *fix = applyFix(Fix::WrappedValue, wrappedProperty->first,
4877-
wrappedProperty->second))
4875+
if (auto wrappedTy = cs.getWrappedPropertyType(*resolvedOverload)) {
4876+
if (auto *fix = applyFix(Fix::WrappedValue, wrappedTy))
48784877
return fix;
48794878
}
48804879

lib/Sema/CSSyntacticElement.cpp

Lines changed: 23 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -141,36 +141,6 @@ class TypeVariableRefFinder : public ASTWalker {
141141
if (!var)
142142
return Action::Continue(expr);
143143

144-
if (auto *wrappedVar = var->getOriginalWrappedProperty()) {
145-
// If there is no type it means that the body of the
146-
// closure hasn't been resolved yet, so we can
147-
// just skip it and wait for \c applyPropertyWrapperToParameter
148-
// to assign types.
149-
if (wrappedVar->hasImplicitPropertyWrapper())
150-
return Action::Continue(expr);
151-
152-
auto outermostWrapperAttr =
153-
wrappedVar->getOutermostAttachedPropertyWrapper();
154-
155-
// If the attribute doesn't have a type it could only mean
156-
// that the declaration was incorrect.
157-
if (!CS.hasType(outermostWrapperAttr->getTypeExpr()))
158-
return Action::Continue(expr);
159-
160-
auto wrapperType =
161-
CS.simplifyType(CS.getType(outermostWrapperAttr->getTypeExpr()));
162-
163-
if (var->getName().hasDollarPrefix()) {
164-
// $<name> is the projected value var
165-
CS.setType(var, computeProjectedValueType(wrappedVar, wrapperType));
166-
} else {
167-
// _<name> is the wrapper var
168-
CS.setType(var, wrapperType);
169-
}
170-
171-
return Action::Continue(expr);
172-
}
173-
174144
// If there is no type recorded yet, let's check whether
175145
// it is a placeholder variable implicitly generated by the
176146
// compiler.
@@ -817,6 +787,26 @@ class SyntacticElementConstraintGenerator
817787
patternType);
818788
}
819789

790+
void setPropertyWrapperAuxiliaryTypes(const SyntacticElementTarget &target) {
791+
if (!target.isForInitialization())
792+
return;
793+
794+
auto *wrappedVar = target.getInitializationWrappedVar();
795+
if (!wrappedVar)
796+
return;
797+
798+
auto *outermostAttr = wrappedVar->getOutermostAttachedPropertyWrapper();
799+
auto *wrapperTypeExpr = outermostAttr->getTypeExpr();
800+
801+
auto wrapperTy = cs.simplifyType(cs.getType(wrapperTypeExpr));
802+
if (auto *projectedVal = wrappedVar->getPropertyWrapperProjectionVar()) {
803+
auto projectedTy = computeProjectedValueType(wrappedVar, wrapperTy);
804+
cs.setType(projectedVal, projectedTy);
805+
}
806+
if (auto *backing = wrappedVar->getPropertyWrapperBackingProperty())
807+
cs.setType(backing, wrapperTy);
808+
}
809+
820810
void visitPatternBindingElement(PatternBindingDecl *patternBinding) {
821811
assert(locator->isLastElement<LocatorPathElt::PatternBindingElement>());
822812

@@ -847,6 +837,9 @@ class SyntacticElementConstraintGenerator
847837
hadError = true;
848838
return;
849839
}
840+
841+
// Set the types of any auxiliary property wrapper vars.
842+
setPropertyWrapperAuxiliaryTypes(*target);
850843
}
851844

852845
void visitDecl(Decl *decl) {

lib/Sema/ConstraintSystem.cpp

Lines changed: 47 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1244,72 +1244,63 @@ TypeVariableType *ConstraintSystem::isRepresentativeFor(
12441244
return *member;
12451245
}
12461246

1247-
static std::optional<std::pair<VarDecl *, Type>>
1248-
getPropertyWrapperInformationFromOverload(
1249-
SelectedOverload resolvedOverload, DeclContext *DC,
1250-
llvm::function_ref<std::optional<std::pair<VarDecl *, Type>>(VarDecl *)>
1251-
getInformation) {
1252-
if (auto *decl =
1253-
dyn_cast_or_null<VarDecl>(resolvedOverload.choice.getDeclOrNull())) {
1254-
if (auto declInformation = getInformation(decl)) {
1255-
Type type;
1256-
VarDecl *memberDecl;
1257-
std::tie(memberDecl, type) = *declInformation;
1258-
if (Type baseType = resolvedOverload.choice.getBaseType()) {
1259-
type = baseType->getRValueType()->getTypeOfMember(memberDecl, type);
1260-
}
1261-
return std::make_pair(decl, type);
1262-
}
1263-
}
1264-
return std::nullopt;
1265-
}
1247+
static Type getPropertyWrapperTypeFromOverload(
1248+
ConstraintSystem &cs, SelectedOverload resolvedOverload,
1249+
llvm::function_ref<VarDecl *(VarDecl *)> getWrapperVar) {
1250+
auto *D = dyn_cast_or_null<VarDecl>(resolvedOverload.choice.getDeclOrNull());
1251+
if (!D)
1252+
return Type();
12661253

1267-
std::optional<std::pair<VarDecl *, Type>>
1268-
ConstraintSystem::getPropertyWrapperProjectionInfo(
1269-
SelectedOverload resolvedOverload) {
1270-
return getPropertyWrapperInformationFromOverload(
1271-
resolvedOverload, DC,
1272-
[](VarDecl *decl) -> std::optional<std::pair<VarDecl *, Type>> {
1273-
if (!decl->hasAttachedPropertyWrapper())
1274-
return std::nullopt;
1254+
auto *wrapperVar = getWrapperVar(D);
1255+
if (!wrapperVar)
1256+
return Type();
12751257

1276-
auto projectionVar = decl->getPropertyWrapperProjectionVar();
1277-
if (!projectionVar)
1278-
return std::nullopt;
1258+
// First check to see if we have a type for this wrapper variable, which will
1259+
// the case for e.g local wrappers in closures.
1260+
if (auto ty = cs.getTypeIfAvailable(wrapperVar))
1261+
return ty;
1262+
1263+
// If we don't have a type for the wrapper variable this shouldn't be a
1264+
// VarDecl we're solving for.
1265+
ASSERT(!cs.hasType(D) && "Should have recorded type for wrapper var");
1266+
1267+
// For the backing property we need to query the request to ensure it kicks
1268+
// type-checking if necessary. Otherwise we can query the interface type.
1269+
auto ty = wrapperVar == D->getPropertyWrapperBackingProperty()
1270+
? D->getPropertyWrapperBackingPropertyType()
1271+
: wrapperVar->getInterfaceType();
1272+
if (!ty)
1273+
return Type();
12791274

1280-
return std::make_pair(projectionVar,
1281-
projectionVar->getInterfaceType());
1282-
});
1275+
// If this is a for a property, substitute the base type. Otherwise we have
1276+
// a local property wrapper and need to map the resulting type into context.
1277+
if (auto baseType = resolvedOverload.choice.getBaseType()) {
1278+
ty = baseType->getRValueType()->getTypeOfMember(wrapperVar, ty);
1279+
} else {
1280+
ty = cs.DC->mapTypeIntoContext(ty);
1281+
}
1282+
return ty;
12831283
}
12841284

1285-
std::optional<std::pair<VarDecl *, Type>>
1286-
ConstraintSystem::getPropertyWrapperInformation(
1285+
Type ConstraintSystem::getPropertyWrapperProjectionType(
12871286
SelectedOverload resolvedOverload) {
1288-
return getPropertyWrapperInformationFromOverload(
1289-
resolvedOverload, DC,
1290-
[](VarDecl *decl) -> std::optional<std::pair<VarDecl *, Type>> {
1291-
if (!decl->hasAttachedPropertyWrapper())
1292-
return std::nullopt;
1293-
1294-
auto backingTy = decl->getPropertyWrapperBackingPropertyType();
1295-
if (!backingTy)
1296-
return std::nullopt;
1297-
1298-
return std::make_pair(decl, backingTy);
1299-
});
1287+
return getPropertyWrapperTypeFromOverload(
1288+
*this, resolvedOverload,
1289+
[&](VarDecl *decl) { return decl->getPropertyWrapperProjectionVar(); });
13001290
}
13011291

1302-
std::optional<std::pair<VarDecl *, Type>>
1303-
ConstraintSystem::getWrappedPropertyInformation(
1292+
Type ConstraintSystem::getPropertyWrapperBackingType(
13041293
SelectedOverload resolvedOverload) {
1305-
return getPropertyWrapperInformationFromOverload(
1306-
resolvedOverload, DC,
1307-
[](VarDecl *decl) -> std::optional<std::pair<VarDecl *, Type>> {
1308-
if (auto wrapped = decl->getOriginalWrappedProperty())
1309-
return std::make_pair(decl, wrapped->getInterfaceType());
1294+
return getPropertyWrapperTypeFromOverload(
1295+
*this, resolvedOverload,
1296+
[](VarDecl *decl) { return decl->getPropertyWrapperBackingProperty(); });
1297+
}
13101298

1311-
return std::nullopt;
1312-
});
1299+
Type ConstraintSystem::getWrappedPropertyType(
1300+
SelectedOverload resolvedOverload) {
1301+
return getPropertyWrapperTypeFromOverload(
1302+
*this, resolvedOverload,
1303+
[](VarDecl *decl) { return decl->getOriginalWrappedProperty(); });
13131304
}
13141305

13151306
void ConstraintSystem::addOverloadSet(Type boundType,

lib/Sema/SyntacticElementTarget.cpp

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -306,14 +306,31 @@ void SyntacticElementTarget::markInvalid() const {
306306
return Action::Continue(P);
307307
}
308308

309+
void invalidateVarDecl(VarDecl *VD) {
310+
// Only set invalid if we don't already have an interface type computed.
311+
if (!VD->hasInterfaceType())
312+
VD->setInvalid();
313+
314+
// Also do the same for any auxiliary vars.
315+
VD->visitAuxiliaryVars(/*forNameLookup*/ false, [&](VarDecl *VD) {
316+
invalidateVarDecl(VD);
317+
});
318+
}
319+
309320
PreWalkAction walkToDeclPre(Decl *D) override {
310321
// Mark any VarDecls and PatternBindingDecls as invalid.
311322
if (auto *VD = dyn_cast<VarDecl>(D)) {
312-
// Only set invalid if we don't already have an interface type computed.
313-
if (!VD->hasInterfaceType())
314-
D->setInvalid();
315-
} else if (isa<PatternBindingDecl>(D)) {
316-
D->setInvalid();
323+
invalidateVarDecl(VD);
324+
} else if (auto *PBD = dyn_cast<PatternBindingDecl>(D)) {
325+
PBD->setInvalid();
326+
// Make sure we mark the patterns and initializers as having been
327+
// checked, otherwise `typeCheckPatternBinding` might try to check them
328+
// again.
329+
for (auto i : range(0, PBD->getNumPatternEntries())) {
330+
PBD->setPattern(i, PBD->getPattern(i), /*fullyValidated*/ true);
331+
if (PBD->isInitialized(i))
332+
PBD->setInitializerChecked(i);
333+
}
317334
}
318335
return Action::VisitNodeIf(isa<PatternBindingDecl>(D));
319336
}

test/decl/var/property_wrappers.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1346,6 +1346,15 @@ struct MissingPropertyWrapperUnwrap {
13461346
}
13471347
}
13481348

1349+
_ = {
1350+
struct S {
1351+
@WrapperWithInitialValue var x = 0
1352+
init() {}
1353+
}
1354+
func a<T>(_: WrapperWithInitialValue<T>) {}
1355+
a(S().x) // expected-error {{cannot convert value 'x' of type 'Int' to expected type 'WrapperWithInitialValue<Int>', use wrapper instead}} {{9-9=_}}
1356+
}
1357+
13491358
struct InvalidPropertyDelegateUse {
13501359
// TODO(diagnostics): We need to a tailored diagnostic for extraneous arguments in property delegate initialization
13511360
@Foo var x: Int = 42 // expected-error@:21 {{extra argument 'wrappedValue' in call}}

validation-test/compiler_crashers/ConstraintSystem-getClosureType-36e4ea.swift renamed to validation-test/compiler_crashers_fixed/ConstraintSystem-getClosureType-36e4ea.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// {"kind":"typecheck","signature":"swift::constraints::ConstraintSystem::getClosureType(swift::ClosureExpr const*) const","signatureAssert":"Assertion failed: (result), function getClosureType"}
2-
// RUN: not --crash %target-swift-frontend -typecheck %s
2+
// RUN: not %target-swift-frontend -typecheck %s
33
@propertyWrapper struct a<b {
44
wrappedValue: b
55
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// {"kind":"typecheck","signature":"swift::constraints::ConstraintSystem::getClosureType(swift::ClosureExpr const*) const","signatureAssert":"Assertion failed: (result), function getClosureType"}
2+
// RUN: not %target-swift-frontend -typecheck %s
3+
@propertyWrapper struct a {
4+
}
5+
{
6+
@a var x =
7+
if <#expression#> {
8+
return
9+
}
10+
_x
11+
}
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// {"kind":"typecheck","signature":"swift::TypeChecker::performTypoCorrection(swift::DeclContext*, swift::DeclRefKind, swift::Type, swift::optionset::OptionSet<swift::NameLookupFlags, unsigned int>, swift::TypoCorrectionResults&, swift::GenericSignature, unsigned int)","signatureAssert":"Assertion failed: (!baseTypeOrNull || !baseTypeOrNull->hasTypeParameter() || genericSig), function performTypoCorrection"}
2-
// RUN: not --crash %target-swift-frontend -typecheck %s
2+
// RUN: not %target-swift-frontend -typecheck %s
33
enum a<b {
44
@propertyWrapper class e {wrappedValue:b var projectedValue: a? {
55
@e var wrappedValue: b wrappedValue.d

0 commit comments

Comments
 (0)