Skip to content

[Property wrappers] Fix handling of @autoclosure in init(wrappedValue:) #30537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -5191,6 +5191,18 @@ class VarDecl : public AbstractStorageDecl {
/// a suitable `init(initialValue:)`.
bool isPropertyMemberwiseInitializedWithWrappedType() const;

/// Whether the innermost property wrapper's initializer's 'wrappedValue' parameter
/// is marked with '@autoclosure' and '@escaping'.
bool isInnermostPropertyWrapperInitUsesEscapingAutoClosure() const;

/// Return the interface type of the value used for the 'wrappedValue:'
/// parameter when initializing a property wrapper.
///
/// If the property has an attached property wrapper and the 'wrappedValue:'
/// parameter is an autoclosure, return a function type returning the stored
/// value. Otherwise, return the interface type of the stored value.
Type getPropertyWrapperInitValueInterfaceType() const;

/// If this property is the backing storage for a property with an attached
/// property wrapper, return the original property.
///
Expand Down
4 changes: 4 additions & 0 deletions include/swift/AST/PropertyWrappers.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ struct PropertyWrapperTypeInfo {
HasInitialValueInit
} wrappedValueInit = NoWrappedValueInit;

/// Whether the init(wrappedValue:), if it exists, has the wrappedValue
/// argument as an escaping autoclosure.
bool isWrappedValueInitUsingEscapingAutoClosure = false;

/// The initializer that will be called to default-initialize a
/// value with an attached property wrapper.
enum {
Expand Down
45 changes: 39 additions & 6 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5930,6 +5930,25 @@ bool VarDecl::isPropertyMemberwiseInitializedWithWrappedType() const {
return allAttachedPropertyWrappersHaveInitialValueInit();
}

bool VarDecl::isInnermostPropertyWrapperInitUsesEscapingAutoClosure() const {
auto customAttrs = getAttachedPropertyWrappers();
if (customAttrs.empty())
return false;

unsigned innermostWrapperIndex = customAttrs.size() - 1;
auto typeInfo = getAttachedPropertyWrapperTypeInfo(innermostWrapperIndex);
return typeInfo.isWrappedValueInitUsingEscapingAutoClosure;
}

Type VarDecl::getPropertyWrapperInitValueInterfaceType() const {
Type valueInterfaceTy = getValueInterfaceType();

if (isInnermostPropertyWrapperInitUsesEscapingAutoClosure())
return FunctionType::get({}, valueInterfaceTy);

return valueInterfaceTy;
}

Identifier VarDecl::getObjCPropertyName() const {
if (auto attr = getAttrs().getAttribute<ObjCAttr>()) {
if (auto name = attr->getName())
Expand Down Expand Up @@ -6324,6 +6343,8 @@ Expr *swift::findOriginalPropertyWrapperInitialValue(VarDecl *var,
return { false, E };

if (auto call = dyn_cast<CallExpr>(E)) {
ASTContext &ctx = innermostNominal->getASTContext();

// We're looking for an implicit call.
if (!call->isImplicit())
return { true, E };
Expand All @@ -6333,9 +6354,19 @@ Expr *swift::findOriginalPropertyWrapperInitialValue(VarDecl *var,
// property.
if (auto tuple = dyn_cast<TupleExpr>(call->getArg())) {
if (tuple->getNumElements() > 0) {
auto elem = tuple->getElement(0);
if (elem->isImplicit() && isa<CallExpr>(elem)) {
return { true, E };
for (unsigned i : range(tuple->getNumElements())) {
if (tuple->getElementName(i) == ctx.Id_wrappedValue ||
tuple->getElementName(i) == ctx.Id_initialValue) {
auto elem = tuple->getElement(i)->getSemanticsProvidingExpr();

// Look through autoclosures.
if (auto autoclosure = dyn_cast<AutoClosureExpr>(elem))
elem = autoclosure->getSingleExpressionBody();

if (elem->isImplicit() && isa<CallExpr>(elem)) {
return { true, E };
}
}
}
}
}
Expand All @@ -6348,7 +6379,6 @@ Expr *swift::findOriginalPropertyWrapperInitialValue(VarDecl *var,

// Find the implicit initialValue/wrappedValue argument.
if (auto tuple = dyn_cast<TupleExpr>(call->getArg())) {
ASTContext &ctx = innermostNominal->getASTContext();
for (unsigned i : range(tuple->getNumElements())) {
if (tuple->getElementName(i) == ctx.Id_wrappedValue ||
tuple->getElementName(i) == ctx.Id_initialValue) {
Expand All @@ -6368,8 +6398,11 @@ Expr *swift::findOriginalPropertyWrapperInitialValue(VarDecl *var,
if (initArg) {
initArg = initArg->getSemanticsProvidingExpr();
if (auto autoclosure = dyn_cast<AutoClosureExpr>(initArg)) {
initArg =
autoclosure->getSingleExpressionBody()->getSemanticsProvidingExpr();
if (!var->isInnermostPropertyWrapperInitUsesEscapingAutoClosure()) {
// Remove the autoclosure part only for non-escaping autoclosures
initArg =
autoclosure->getSingleExpressionBody()->getSemanticsProvidingExpr();
}
}
}
return initArg;
Expand Down
9 changes: 5 additions & 4 deletions lib/SIL/TypeLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1895,8 +1895,10 @@ static CanAnyFunctionType getStoredPropertyInitializerInterfaceType(
// wrapper that was initialized with '=', the stored property initializer
// will be in terms of the original property's type.
if (auto originalProperty = VD->getOriginalWrappedProperty()) {
if (originalProperty->isPropertyMemberwiseInitializedWithWrappedType())
resultTy = originalProperty->getValueInterfaceType()->getCanonicalType();
if (originalProperty->isPropertyMemberwiseInitializedWithWrappedType()) {
resultTy = originalProperty->getPropertyWrapperInitValueInterfaceType()
->getCanonicalType();
}
}

auto sig = DC->getGenericSignatureOfContext();
Expand All @@ -1915,8 +1917,7 @@ static CanAnyFunctionType getPropertyWrapperBackingInitializerInterfaceType(

auto *DC = VD->getInnermostDeclContext();
CanType inputType =
VD->getParentPattern()->getType()->mapTypeOutOfContext()
->getCanonicalType();
VD->getPropertyWrapperInitValueInterfaceType()->getCanonicalType();

auto sig = DC->getGenericSignatureOfContext();

Expand Down
2 changes: 1 addition & 1 deletion lib/SILGen/SILGenConstructor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,7 @@ static Type getInitializationTypeInContext(
if (auto singleVar = pattern->getSingleVar()) {
if (auto originalProperty = singleVar->getOriginalWrappedProperty()) {
if (originalProperty->isPropertyMemberwiseInitializedWithWrappedType())
interfaceType = originalProperty->getValueInterfaceType();
interfaceType = originalProperty->getPropertyWrapperInitValueInterfaceType();
}
}

Expand Down
23 changes: 23 additions & 0 deletions lib/SILGen/SILGenExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2397,9 +2397,32 @@ RValue RValueEmitter::visitCaptureListExpr(CaptureListExpr *E, SGFContext C) {
return visit(E->getClosureBody(), C);
}

static OpaqueValueExpr *opaqueValueExprToSubstituteForAutoClosure(
const AbstractClosureExpr *e) {
// When we find an autoclosure that just calls an opaque closure,
// this is a case where we've created the opaque closure as a
// stand-in for the autoclosure itself. Such an opaque closure is
// created when we have a property wrapper's 'init(wrappedValue:)'
// taking an autoclosure argument.
if (auto ace = dyn_cast<AutoClosureExpr>(e)) {
if (auto ce = dyn_cast<CallExpr>(ace->getSingleExpressionBody())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems really, really brittle. Is there a way we can keep this in sync with the synthesis code?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't come up with a good way, no. In general we really need a better way to pull out the initial value expression from the type-checked AST for the initialization of the backing storage, because that code also needed too much tweaking. Maybe there's some better way to grab the expression at the right point in CSApply.cpp so we can save it.

if (auto ove = dyn_cast<OpaqueValueExpr>(ce->getFn())) {
if (!ace->isImplicit() || !ove->isImplicit() || !ove->isPlaceholder())
return nullptr;

if (ace->getType()->isEqual(ove->getType()))
return ove;
}
}
}
return nullptr;
}

RValue RValueEmitter::visitAbstractClosureExpr(AbstractClosureExpr *e,
SGFContext C) {
if (auto ove = opaqueValueExprToSubstituteForAutoClosure(e))
return visitOpaqueValueExpr(ove, C);

// Emit the closure body.
SGF.SGM.emitClosure(e);

Expand Down
5 changes: 3 additions & 2 deletions lib/SILGen/SILGenFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -750,7 +750,8 @@ void SILGenFunction::emitGeneratorFunction(SILDeclRef function, Expr *value,
ctx.getIdentifier("$input_value"),
dc);
param->setSpecifier(ParamSpecifier::Owned);
param->setInterfaceType(function.getDecl()->getInterfaceType());
auto vd = cast<VarDecl>(function.getDecl());
param->setInterfaceType(vd->getPropertyWrapperInitValueInterfaceType());

params = ParameterList::create(ctx, SourceLoc(), {param}, SourceLoc());
}
Expand Down Expand Up @@ -800,7 +801,7 @@ void SILGenFunction::emitGeneratorFunction(SILDeclRef function, VarDecl *var) {
// will be in terms of the original property's type.
if (auto originalProperty = var->getOriginalWrappedProperty()) {
if (originalProperty->isPropertyMemberwiseInitializedWithWrappedType()) {
interfaceType = originalProperty->getValueInterfaceType();
interfaceType = originalProperty->getPropertyWrapperInitValueInterfaceType();
}
}

Expand Down
14 changes: 10 additions & 4 deletions lib/SILGen/SILGenLValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1302,7 +1302,7 @@ namespace {
{
}

bool hasPropertyWrapper() const {
bool canRewriteSetAsPropertyWrapperInit() const {
if (auto *VD = dyn_cast<VarDecl>(Storage)) {
// If this is not a wrapper property that can be initialized from
// a value of the wrapped type, we can't perform the initialization.
Expand All @@ -1321,6 +1321,13 @@ namespace {
return false;
}

// If this property wrapper uses autoclosure in it's initializer,
// the argument types of the setter and initializer shall be
// different, so we don't rewrite an assignment into an
// initialization.
if (VD->isInnermostPropertyWrapperInitUsesEscapingAutoClosure())
return false;

return true;
}

Expand Down Expand Up @@ -1389,9 +1396,8 @@ namespace {
assert(getAccessorDecl()->isSetter());
SILDeclRef setter = Accessor;

if (IsOnSelfParameter &&
if (IsOnSelfParameter && canRewriteSetAsPropertyWrapperInit() &&
!Storage->isStatic() &&
hasPropertyWrapper() &&
isBackingVarVisible(cast<VarDecl>(Storage),
SGF.FunctionDC)) {
// This is wrapped property. Instead of emitting a setter, emit an
Expand Down Expand Up @@ -4294,7 +4300,7 @@ static bool trySetterPeephole(SILGenFunction &SGF, SILLocation loc,
}

auto &setterComponent = static_cast<GetterSetterComponent&>(component);
if (setterComponent.hasPropertyWrapper())
if (setterComponent.canRewriteSetAsPropertyWrapperInit())
return false;
setterComponent.emitAssignWithSetter(SGF, loc, std::move(dest),
std::move(src));
Expand Down
8 changes: 7 additions & 1 deletion lib/Sema/CodeSynthesis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl,
accessLevel = std::min(accessLevel, var->getFormalAccess());

auto varInterfaceType = var->getValueInterfaceType();
bool isAutoClosure = false;

if (var->getAttrs().hasAttribute<LazyAttr>()) {
// If var is a lazy property, its value is provided for the underlying
Expand All @@ -221,7 +222,11 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl,
// accept a value of the original property type. Otherwise, the
// memberwise initializer will be in terms of the backing storage
// type.
if (!var->isPropertyMemberwiseInitializedWithWrappedType()) {
if (var->isPropertyMemberwiseInitializedWithWrappedType()) {
varInterfaceType = var->getPropertyWrapperInitValueInterfaceType();
isAutoClosure =
var->isInnermostPropertyWrapperInitUsesEscapingAutoClosure();
} else {
varInterfaceType = backingPropertyType;
}
}
Expand All @@ -233,6 +238,7 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl,
arg->setSpecifier(ParamSpecifier::Default);
arg->setInterfaceType(varInterfaceType);
arg->setImplicit();
arg->setAutoClosure(isAutoClosure);

// Don't allow the parameter to accept temporary pointer conversions.
arg->setNonEphemeralIfPossible();
Expand Down
65 changes: 62 additions & 3 deletions lib/Sema/TypeCheckPropertyWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,42 @@ static SubscriptDecl *findEnclosingSelfSubscript(ASTContext &ctx,
return subscript;
}

/// Whether the argument with label 'argumentLabel' in the initializer 'init'
/// is an escaping autoclosure argument.
static bool isEscapingAutoclosureArgument(const ConstructorDecl *init,
Identifier argumentLabel) {
if (!init)
return false;

Optional<size_t> parameterIndex = None;
auto params = init->getParameters();
for (size_t i = 0; i < params->size(); i++) {
if (params->get(i)->getArgumentName() == argumentLabel) {
parameterIndex = i;
break;
}
}

if (!parameterIndex.hasValue())
return false;

size_t paramIndex = parameterIndex.getValue();
if (!params->get(paramIndex)->isAutoClosure())
return false;

if (auto initTy = init->getInterfaceType()->getAs<AnyFunctionType>()) {
if (auto funcTy = initTy->getResult()->getAs<FunctionType>()) {
if (funcTy->getNumParams() > paramIndex) {
Type paramTy = funcTy->getParams()[paramIndex].getPlainType();
if (auto paramFuncTy = paramTy->getAs<FunctionType>())
return !paramFuncTy->isNoEscape();
}
}
}

return false;
}

llvm::Expected<PropertyWrapperTypeInfo>
PropertyWrapperTypeInfoRequest::evaluate(
Evaluator &eval, NominalTypeDecl *nominal) const {
Expand All @@ -313,12 +349,16 @@ PropertyWrapperTypeInfoRequest::evaluate(

PropertyWrapperTypeInfo result;
result.valueVar = valueVar;
if (findSuitableWrapperInit(ctx, nominal, valueVar,
PropertyWrapperInitKind::WrappedValue))
if (auto init = findSuitableWrapperInit(ctx, nominal, valueVar,
PropertyWrapperInitKind::WrappedValue)) {
result.wrappedValueInit = PropertyWrapperTypeInfo::HasWrappedValueInit;
else if (auto init = findSuitableWrapperInit(
result.isWrappedValueInitUsingEscapingAutoClosure =
isEscapingAutoclosureArgument(init, ctx.Id_wrappedValue);
} else if (auto init = findSuitableWrapperInit(
ctx, nominal, valueVar, PropertyWrapperInitKind::InitialValue)) {
result.wrappedValueInit = PropertyWrapperTypeInfo::HasInitialValueInit;
result.isWrappedValueInitUsingEscapingAutoClosure =
isEscapingAutoclosureArgument(init, ctx.Id_initialValue);

if (init->getLoc().isValid()) {
auto diag = init->diagnose(diag::property_wrapper_init_initialValue);
Expand Down Expand Up @@ -632,13 +672,32 @@ Type swift::computeWrappedValueType(VarDecl *var, Type backingStorageType,
return wrappedValueType;
}

static bool isOpaquePlaceholderClosure(const Expr *value) {
auto ove = dyn_cast<OpaqueValueExpr>(value);
if (!ove || !ove->isPlaceholder())
return false;

if (auto valueFnTy = ove->getType()->getAs<FunctionType>()) {
return (valueFnTy->getNumParams() == 0);
}

return false;
}

Expr *swift::buildPropertyWrapperInitialValueCall(
VarDecl *var, Type backingStorageType, Expr *value,
bool ignoreAttributeArgs) {
// From the innermost wrapper type out, form init(wrapperValue:) calls.
ASTContext &ctx = var->getASTContext();
auto wrapperAttrs = var->getAttachedPropertyWrappers();
Expr *initializer = value;
if (var->isInnermostPropertyWrapperInitUsesEscapingAutoClosure() &&
isOpaquePlaceholderClosure(value)) {
// We can't pass the opaque closure directly as an autoclosure arg.
// So we instead pass a CallExpr calling the opaque closure, which
// the type checker shall wrap in an AutoClosureExpr.
initializer = CallExpr::createImplicit(ctx, value, {}, {});
}
for (unsigned i : llvm::reverse(indices(wrapperAttrs))) {
Type wrapperType =
backingStorageType ? computeWrappedValueType(var, backingStorageType, i)
Expand Down
5 changes: 4 additions & 1 deletion lib/Sema/TypeCheckStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2485,8 +2485,11 @@ PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,

// Form the initialization of the backing property from a value of the
// original property's type.
Type origValueInterfaceType = var->getPropertyWrapperInitValueInterfaceType();
Type origValueType =
var->getDeclContext()->mapTypeIntoContext(origValueInterfaceType);
OpaqueValueExpr *origValue =
new (ctx) OpaqueValueExpr(var->getSourceRange(), var->getType(),
new (ctx) OpaqueValueExpr(var->getSourceRange(), origValueType,
/*isPlaceholder=*/true);
Expr *initializer = buildPropertyWrapperInitialValueCall(
var, storageType, origValue,
Expand Down
Loading