[SE-0293] Extend property wrappers to function and closure parameters #34272

hborla · 2020-10-11T22:59:06Z

Implementation of SE-0293: Extend property wrappers to function and closure parameters.

Approach:

Functions with property wrapper parameters use the backing property type in the function signature everywhere except the constraint system. When a function is resolved and its type is opened, the property wrapper type is "unwrapped" and replaced with the wrapped-value type or the projected-value type, depending on the argument label.
When the property wrapper type is replaced, the property wrapper is "applied" by generating the appropriate constraints between the backing wrapper type and the wrapped or projected value type. The wrapper type and init kind (between wrapped-value and projected-value) are recorded in the constraint system.
CSApply will wrap functions with wrapper parameters in an autoclosure that accepts the wrapped-value or projected-value type, which initializes the property wrapper and calls the underlying function. Other arguments that don't have property wrappers are simply forwarded.
- Note: a lot of the code that builds the "forwarding" parameter list is duplicated in buildCurryThunk. I intend to clean this up.
- If the function is immediately applied, no autoclosure is necessary and instead the property wrapper initialization expression is injected around the appropriate argument in coerceCallArguments.
- Instead of injecting the initializer expression directly in CSApply, an AppliedPropertyWrapperExpr is used. When SILGen sees this expression, it will emit an application of the appropriate property wrapper generator function.
When a function is emitted in SILGen, property-wrapped parameters are replaced with the backing property wrapper (which is synthesized as a ParamDecl). The local computed properties are emitted the same way as local property wrappers - by visiting the "auxiliary decls" during SILGen.
When visiting parameter decls, SILGen will also emit both property wrapper generator functions if the parameter has them. If the enclosing function is public, property wrapper generator functions are serialized and given public non-ABI linkage.

A few things that are still a WIP:

Unapplied references to functions with effects (e.g. throws, async)
Diagnostics for invalid wrapper attributes in overrides and protocol witnesses
"Implementation detail" property wrappers from the latest revision
Closure parameter type inference
Testing!

Not implemented in this PR:

Nonmutating setter synthesis for inferred property wrappers (e.g. $ closure parameters with no attribute)
Property wrapper parameters in subscript declarations

Resolves: rdar://66866426, rdar://71902161

hborla · 2020-10-12T00:00:48Z

@swift-ci please build toolchain

hborla · 2020-10-12T15:54:49Z

@swift-ci please build toolchain macOS platform

hborla · 2020-10-12T23:50:33Z

@swift-ci please build toolchain

hborla · 2020-11-09T00:48:47Z

@swift-ci please build toolchain

hborla · 2020-12-03T17:59:00Z

@swift-ci please build toolchain

hborla · 2021-01-27T06:19:27Z

@swift-ci please build toolchain

hborla · 2021-01-27T16:22:28Z

@swift-ci please build toolchain

hborla · 2021-01-28T20:42:48Z

@swift-ci please build toolchain

hborla · 2021-01-29T17:11:07Z

@swift-ci please build toolchain

hborla · 2021-01-29T17:11:22Z

@swift-ci please smoke test

to have a mutating wrappedValue getter.

where the programmer tries to pass a projection argument to a wrapped parameter without 'var projectedValue'.

closure is resolved. This can't be diagnosed earlier because a property wrapper attribute may be inferred in the constraint system.

wrapper projection argument to a wrapped parameter with arguments in the wrapper attribute.

…pper parameters in subscript declarations.

property wrapper in the property wrapper requests.

inference for closure parameters.

position in CSGen. This effectively reverts 6caca26014.

…ressions in PropertyWrapperBackingPropertyInfo.

take in a projected value.

dedicated initializer context for this expressions.

initialization expressions for parameters with attached property wrappers.

around arguments to property wrapper parameters.

generator functions for parameters of public functions.

… use the correct substitution map when emitting the property wrapper generator application in SILGen.

the parameter has an attached property wrapper.

…ppers in protocol requirements.

hborla · 2021-02-26T02:35:52Z

@swift-ci please test

hborla · 2021-02-26T02:36:00Z

@swift-ci please test source compatibility

hborla · 2021-02-26T04:30:33Z

@swift-ci please smoke test

swift-ci · 2021-02-26T06:52:34Z

Build failed
Swift Test OS X Platform
Git Sha - b3d54b6

hborla · 2021-02-26T07:03:24Z

@swift-ci please smoke test macOS platform

hborla · 2021-02-26T08:10:40Z

@swift-ci please smoke test macOS platform

davezarzycki · 2021-02-28T19:27:01Z

lib/Sema/ConstraintSystem.cpp

+    auto *functionType = fullFunctionType->getResult()->getAs<FunctionType>();
+    functionType = unwrapPropertyWrapperParameterTypes(*this, funcDecl, functionRefKind,
+                                                       functionType, locator);
+    openedType = FunctionType::get(fullFunctionType->getParams(), functionType);


Is there a reason why we're not doing any FunctionType::ExtInfo propagation/preservation here? In other words, are the default ExtInfo flags the goal or do they just happen to work?

And as a general observation that long predates your pull request, it's kind of frustrating that the FunctionType constructor let's one elide (and therefore default) the ExtInfo state. This wouldn't ordinarily be a problem for most constructors but Swift is constantly unpacking and rebuilding function types and I find that the ExtInfo preservation/management to be haphazard. I've been tempted to propose something but I don't know what people think/feel about ExtInfo.

Ah, I think you're right that dropping them just happens to work here. Thanks for catching - I'll fix this in a followup PR.

Agreed that the default ExtInfo() makes it too easy to accidentally drop existing ExtInfo when transforming a function type like this. I actually had to fix the same mistake in the body of unwrapPropertyWrapperParameterTypes as well

Thanks for clarifying. A link to the followup PR would be appreciated. I just posted to the forum about making ExtInfo preservation/propagation more explicit:

https://forums.swift.org/t/rfc-make-extinfo-param-to-functiontype-construction-always-explicit/45182

Just to close the loop, I merged #36234 (but GitHub thinks the merge failed, weird). Thanks for helping me better understand the intent here and now that #36234 is merged, hopefully intent will be a little more clear in the future. :-)

hborla force-pushed the property-wrapper-parameters branch 2 times, most recently from 4b1430f to dbd53bf Compare October 11, 2020 23:59

hborla added the swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review label Oct 12, 2020

This comment has been minimized.

Sign in to view

swiftlang deleted a comment from swift-ci Oct 13, 2020

This comment has been minimized.

Sign in to view

hborla force-pushed the property-wrapper-parameters branch 2 times, most recently from db5aeaf to ede2ea8 Compare November 8, 2020 19:31

This comment has been minimized.

Sign in to view

hborla force-pushed the property-wrapper-parameters branch from 101fcbc to 10bcee5 Compare January 12, 2021 04:08

hborla force-pushed the property-wrapper-parameters branch 6 times, most recently from 83fe02f to 4941843 Compare January 27, 2021 05:09

hborla added 20 commits February 25, 2021 18:35

[Property Wrappers] Don't allow property wrappers applied to parameters

a4e3968

to have a mutating wrappedValue getter.

[Property Wrappers] Add a constraint fix and diagnostic for the case

ea3fe03

where the programmer tries to pass a projection argument to a wrapped parameter without 'var projectedValue'.

[Sema] Diagnose invalid $ prefixes on closure parameters after the

12161fa

closure is resolved. This can't be diagnosed earlier because a property wrapper attribute may be inferred in the constraint system.

[ConstraintSystem] Emit an error message when trying to pass a property

522eedc

wrapper projection argument to a wrapped parameter with arguments in the wrapper attribute.

[NFC][Property Wrappers] Start to add Sema tests for SE-0293.

36008a1

[Property Wrappers] Add an unsupported error message for property wra…

2c2d671

…pper parameters in subscript declarations.

[Property Wrappers] Implement support for parameters with an implicit

96f4315

property wrapper in the property wrapper requests.

[ConstraintSystem] Implement implicit property wrapper attribute

133dec0

inference for closure parameters.

[NFC][Property Wrappers] Start to add SILGen tests for SE-0293.

e40d7eb

[CSGen] Move generateWrappedPropertyTypeConstraints back to its original

aa0da03

position in CSGen. This effectively reverts 6caca26014.

[Property Wrappers] Store property wrapper "init from projection" exp…

13692fe

…ressions in PropertyWrapperBackingPropertyInfo.

[SILGen] Teach SILGen to emit property wrapper generator functions that

648c575

take in a projected value.

[Sema] Add an implicit applied property wrapper expression and a new

b821c8d

dedicated initializer context for this expressions.

[Property Wrappers] Compute generic wrapped-value and projected-value

9cdecf4

initialization expressions for parameters with attached property wrappers.

[ConstraintSystem] Inject implicit applied property wrapper expressions

99e0666

around arguments to property wrapper parameters.

[SILGen] Serialize and give PublicNonABI linkage to property wrapper

bea89c6

generator functions for parameters of public functions.

[Property Wrappers] Store the callee in AppliedPropertyWrapperExpr to…

c29eecd

… use the correct substitution map when emitting the property wrapper generator application in SILGen.

[ConstraintSystem] Don't allow dollar prefixes in argument labels unless

db38727

the parameter has an attached property wrapper.

[NFC][ConstraintSystem] Rename applyPropertyWrapperParameter.

b7a170c

[Property Wrappers] Don't allow parameters with attached property wra…

b3d54b6

…ppers in protocol requirements.

hborla force-pushed the property-wrapper-parameters branch from 7f34a8b to b3d54b6 Compare February 26, 2021 02:35

hborla merged commit eedbb9d into swiftlang:main Feb 26, 2021

hborla deleted the property-wrapper-parameters branch February 27, 2021 04:49

davezarzycki reviewed Feb 28, 2021

View reviewed changes

[SE-0293] Extend property wrappers to function and closure parameters #34272

[SE-0293] Extend property wrappers to function and closure parameters #34272

Uh oh!

Conversation

hborla commented Oct 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hborla commented Oct 12, 2020

Uh oh!

hborla commented Oct 12, 2020

Uh oh!

hborla commented Oct 12, 2020

Uh oh!

This comment has been minimized.

This comment has been minimized.

hborla commented Nov 9, 2020

Uh oh!

This comment has been minimized.

This comment has been minimized.

hborla commented Dec 3, 2020

Uh oh!

This comment has been minimized.

This comment has been minimized.

hborla commented Jan 27, 2021

Uh oh!

hborla commented Jan 27, 2021

Uh oh!

hborla commented Jan 28, 2021

Uh oh!

hborla commented Jan 29, 2021

Uh oh!

hborla commented Jan 29, 2021

Uh oh!

hborla commented Feb 26, 2021

Uh oh!

hborla commented Feb 26, 2021

Uh oh!

hborla commented Feb 26, 2021

Uh oh!

swift-ci commented Feb 26, 2021

Uh oh!

hborla commented Feb 26, 2021

Uh oh!

hborla commented Feb 26, 2021

Uh oh!

davezarzycki Feb 28, 2021

Choose a reason for hiding this comment

Uh oh!

hborla Feb 28, 2021

Choose a reason for hiding this comment

Uh oh!

davezarzycki Mar 1, 2021

Choose a reason for hiding this comment

Uh oh!

davezarzycki Mar 12, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hborla commented Oct 11, 2020 •

edited

Loading