Skip to content

[Sema/SILGen] Allow property wrappers on let declarations#62342

Open
amritpan wants to merge 10 commits intoswiftlang:mainfrom
amritpan:allow-let-property-wrapper
Open

[Sema/SILGen] Allow property wrappers on let declarations#62342
amritpan wants to merge 10 commits intoswiftlang:mainfrom
amritpan:allow-let-property-wrapper

Conversation

@amritpan
Copy link
Copy Markdown
Member

@amritpan amritpan commented Dec 1, 2022 •
edited
Loading

This adds support for let declared property wrappers to the Swift language and is the implementation for SE-NNNN Allow Property Wrappers on Let Declarations.

At the time of this proposal, let declared property wrappers are not allowed:

@propertyWrapper
struct Wrapper {
  init(wrappedValue: Int) {}
  var wrappedValue: Int { 0 }
}

struct S {
  @Wrapper let value: Int  // error: property wrapper can only be applied to a ‘var’
}

This PR implements changes so that the above is legal in Swift and let wrapped properties are allowed as local and instance variables.

@xedin xedin self-requested a review December 1, 2022 19:15
@xedin
Copy link
Copy Markdown
Contributor

xedin commented Dec 1, 2022

@swift-ci please build toolchain

@xedin xedin changed the title [Sema/SILGen] Allow let property wrapper [Sema/SILGen] Allow property wrappers on let declarations Dec 1, 2022
@amritpan amritpan force-pushed the allow-let-property-wrapper branch 2 times, most recently from 6cb7fe8 to 6139ffa Compare December 2, 2022 19:07
@amritpan amritpan force-pushed the allow-let-property-wrapper branch 5 times, most recently from 29359b4 to 8dca445 Compare December 17, 2022 04:27
xedin
xedin reviewed Dec 20, 2022
View reviewed changes
@amritpan amritpan force-pushed the allow-let-property-wrapper branch 3 times, most recently from bd2ab13 to a71ce80 Compare January 19, 2023 06:02
@amritpan amritpan marked this pull request as ready for review January 19, 2023 06:02
@amritpan amritpan requested review from xedin and removed request for hborla and slavapestov January 19, 2023 06:03
@xedin
Copy link
Copy Markdown
Contributor

xedin commented Jan 19, 2023

I will try to take a look tomorrow, thank you for following up on this!

@amritpan amritpan mentioned this pull request Jan 19, 2023
Closed
xedin
xedin approved these changes Jan 20, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@amritpan amritpan force-pushed the allow-let-property-wrapper branch from a71ce80 to 8420fa9 Compare January 28, 2023 00:07
ktoso
ktoso reviewed Feb 2, 2023
View reviewed changes
test/Concurrency/task_local.swift Outdated
Copy link
Copy Markdown
Contributor

@ktoso ktoso Feb 2, 2023

Choose a reason for hiding this comment

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

Thanks for including the test 👍

@amritpan amritpan force-pushed the allow-let-property-wrapper branch from 8420fa9 to ca1cf88 Compare May 15, 2023 18:16
@amritpan amritpan requested a review from tshortli as a code owner May 15, 2023 18:16
@amritpan amritpan force-pushed the allow-let-property-wrapper branch from ca1cf88 to 5cc2d74 Compare May 15, 2023 18:18
@amritpan amritpan force-pushed the allow-let-property-wrapper branch 2 times, most recently from a0712d9 to 39239b5 Compare May 23, 2023 18:10
Amritpan Kaur added 2 commits June 20, 2023 07:58
[TypeCheckStorage] Allow backing property to be either var or let.
ac88e77 
This ensures that a let declared property wrapper also
has a let declared backing property.
[Decl] Remove var-only property wrapper diagnostic.
a22c337
@amritpan amritpan force-pushed the allow-let-property-wrapper branch from 39239b5 to e557afe Compare June 20, 2023 17:11
Amritpan Kaur and others added 8 commits June 20, 2023 10:14
[SILGenLValue] Handle property wrapper structs in custom set method
7926713 
to generate new R and L values for assignment.
[SILGenLValue] Handle property wrapper classes in custom set method
f73e96d
@xedin
[SILGenLValue] Protect r-value set against situations when DC is no…
7ba0cca 
…t a declaration

Assignment or initialization of a field could happen in a closure,
which means that the check for `let` property wrapped property
cannot reply on `getAsDecl()` always producing a valid pointer.
[SILGenLValue] Unify set under NominalTypeElementComponent.
2e5bd5f
[TypeCheckStorage] Support isolated wrapped properties.
0315ab0
[ASTPrinter] Print var for let wrapped computed property.
bd920d2
[Tests] Add tests for let property wrappers.
26795bc
[TypeCheckStorage] Do not generate projectedValue setters
988b438 
for let wrapped properties.
@amritpan amritpan force-pushed the allow-let-property-wrapper branch from e557afe to 988b438 Compare June 20, 2023 17:14
@xedin
Copy link
Copy Markdown
Contributor

xedin commented Jun 20, 2023

@swift-ci please build toolchain

@jeudesprits
Copy link
Copy Markdown

jeudesprits commented Aug 17, 2023

What is the status of this proposal?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ktoso ktoso ktoso left review comments

@xedin xedin xedin approved these changes

@tshortli tshortli Awaiting requested review from tshortli

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@amritpan @xedin @jeudesprits @ktoso