-
Notifications
You must be signed in to change notification settings - Fork 368
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
Updated explicit backing fields proposal #289
base: master
Are you sure you want to change the base?
Conversation
* Use-cases are orothogonalized (similar) use-cases are grouped together. * Additional use-cases are explained to cover all the major pieces of the proposed design. * Design is worked out in more details, with grammar and various restrictions more explicitly spelled out. * This list of future enhancements is narrowed down to the ones that are feasible in the near future. * Table of Contents is added for reference.
Co-authored-by: ilya-g <[email protected]>
The variable should be var to have setter
* Fix subtype -> supertype. * Clarify the advantages of the new syntax of the backing property pattern. * Clarify repeated allocations in Expose read-only view use-case. * Fix Access field from outside of getter and setter use-cases, don't require explicit field declaration. * Retrieve property delegate reference use-case added.
This is awesome, I think it would make more sense the invert the type definitions though, the internal property keeps the internal type definition, then the public API is set using the get syntax: val names: MutableList<String>= mutableListOf()
get(): List<String> Thoughts? |
e078435
to
47348ab
Compare
proposals/explicit-backing-fields.md
Outdated
referenced within the class. Obviously, there is no point in doing so, since the backing property usually provides | ||
all the same functionality, as a primary property and even more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't use the word "obviously" in such a context. This is not a conclusion within a set of strict constraints, but the rationale behind a free choice we decide to make
proposals/explicit-backing-fields.md
Outdated
|
||
There are the following additional semantic restrictions on the property declaration grammar: | ||
Visibility of property can be any, except `private`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: let's say "must be more permissive than", because this statement would stay correct w.r.t. possible future enhancements
|
||
#### Lateinit | ||
If property with explicit backing field has initializer, it's prohibited to declare accessors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more candidate for the future enhancements list? Or a way "to prevent feature abuse"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be a candidate for the future enhancements, but I think it's something that shouldn't be implemented because property declaration with explicit backing field + initializer + getter is something too much puzzling.
5. Support combining mutable explicit backing field and non-mutable property. | ||
Despite its popularity (see [`var` backing property](#var-backing-property)), this use case is not supported in this proposal, because it is impossible for one property to behave as mutable + nullable and | ||
at the same time non-mutable and non-nullable. | ||
6. Add API in Reflection to retrieve information about explicit backing field. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this KEEP also suggests bringing in the init
field, it can probably raise a similar concern?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can. But this init field is more like an implementation detail, and I don't think there can be a huge demand for inspecting it using Reflection, so I decided to not add it to the list
Property with explicit backing field and initializer is represented by additional | ||
auxiliary synthetic field which is needed for storing the value from property initializer. | ||
Auxiliary field is given the name `propertyName$init` and is `private` regardless of property visibility. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be worth mentioning if allowing everything (accessors + access to both the fields as field
and init
within them) is considered a future enhancement or something we'd like to avoid for design reasons
proposals/explicit-backing-fields.md
Outdated
|
||
- **Type**: Design Proposal | ||
- **Authors**: Nikolay Lunyak, Roman Elizarov, Roman Efremov | ||
- **Contributors**: Mikhail Zarechenskiy, Marat Akhin, Alejandro Serrano Mena, Anastasia Nekrasova, Svetlana Isakova, Kirill Rakhman, Dmitry Petrov, Roman Elizarov, Ben Leggiero, Matej Drobnič, Mikhail Glukhikh, Nikolay Lunyak |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- **Contributors**: Mikhail Zarechenskiy, Marat Akhin, Alejandro Serrano Mena, Anastasia Nekrasova, Svetlana Isakova, Kirill Rakhman, Dmitry Petrov, Roman Elizarov, Ben Leggiero, Matej Drobnič, Mikhail Glukhikh, Nikolay Lunyak | |
- **Contributors**: Mikhail Zarechenskiy, Marat Akhin, Alejandro Serrano Mena, Anastasia Nekrasova, Svetlana Isakova, Kirill Rakhman, Dmitry Petrov, Ben Leggiero, Matej Drobnič, Mikhail Glukhikh |
AFAIK, we usually do not put the authors to the contributors as it makes little sense for such double attribution
proposals/explicit-backing-fields.md
Outdated
* [Expose different object](#expose-different-object) | ||
* [`var` backing property](#var-backing-property) | ||
* [`lateinit` backing property](#lateinit-backing-property) | ||
* [Delegation](#Delegation) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* [Delegation](#Delegation) | |
* [Delegation](#delegation) |
proposals/explicit-backing-fields.md
Outdated
This proposal caters to a variety of use-cases that are currently met via a | ||
[backing property pattern](https://kotlinlang.org/docs/properties.html#backing-properties). | ||
|
||
Some of use cases listed below above are not supported to be rewritten with a new feature (starting from [var backing property](#var-backing-property)). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of use cases listed below above are not supported to be rewritten with a new feature (starting from [var backing property](#var-backing-property)). | |
Some of use cases listed below are not supported to be rewritten with a new feature (starting from [var backing property](#var-backing-property)). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe make an additional section header level aka "Use cases targeted by the explicit backing field feature" / "Use cases not supported by the explicit backing field feature"? That would make it easier for somebody who skips through the document to understand what's happening.
proposals/explicit-backing-fields.md
Outdated
|
||
#### Hide complex value storage logic | ||
|
||
Sometimes we want to provide a public API for getting the instant value of a variable, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sometimes we want to provide a public API for getting the instant value of a variable, | |
Sometimes we want to provide a public API for getting the immediate value of a variable, |
A better term to use here would be "immediate" instead of "instant".
proposals/explicit-backing-fields.md
Outdated
|
||
By analogy with the already existing term ["implicit backing fields"](https://kotlinlang.org/docs/properties.html#backing-properties), it is proposed to introduce a syntax | ||
for declaring "explicit backing fields" of properties. | ||
Syntax is consisted of a keyword `field`, optional type definition and initialization expression |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Syntax is consisted of a keyword `field`, optional type definition and initialization expression | |
Syntax consists of a keyword `field`, optional type definition and initialization expression |
proposals/explicit-backing-fields.md
Outdated
|
||
### Accessors | ||
|
||
Property, which has explicit backing field, can also specify getter or setter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Property, which has explicit backing field, can also specify getter or setter. | |
Property, which has explicit backing field, can also specify getter and/or setter. |
proposals/explicit-backing-fields.md
Outdated
* must be final (otherwise calling a field instead of a getter would be undesirable behavior) | ||
* can't be `const` | ||
* can't be `expect` or `external` | ||
* can't be `inline` (but its accessors can) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FTR: an inline
property means its accessors are inline
.
Why do we want to have inline
accessors for EBF properties? This means that the EBF leaks into the public ABI, has to be accessible from the outside and I'm not sure I like this.
Atm, inline
properties cannot have backing fields, and I do not understand why we want to relax the restriction here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, it's better for inline
to be prohibited at all for properties with EBFs.
The case that originally confused me is:
private val _foo = mutableListOf(1, 2, 3)
internal inline val foo: List<Int> get() = _foo // OK
inline val foo2: List<Int> get() = _foo // ERR, "Public-API inline function cannot access non-public-API"
So I thought that we shouldn't prohibit inline
for explicit backing fields in general, and instead only obey the existing rule "Public-API inline function cannot access non-public-API".
But now I checked at what the internal foo
property in code above compiles into (extra static method), and I think it doesn't make any sense to support for EBFs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Smth I originally forgot -- do we say anywhere that EBF are prohibited for extension properties? =)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's an good remark, thank you. I've added it.
proposals/explicit-backing-fields.md
Outdated
* can't be `const` | ||
* can't be `expect` or `external` | ||
* can't be `inline` (but its accessors can) | ||
* or explicit backing field itself can't be `lateinit` (added to [Future enhancements](#future-enhancements)). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why "or"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The full phrase I meant is "Property with explicit backing field or explicit backing field itself can't be lateinit
".
I think the confusion comes from that I put the words “Property with an explicit backing field” at the beginning so as not to repeat them in every bullet-point.
I've rewritten it with "neither...nor" and extracted it into a separate bullet list. Hope, it looks clearer now.
It's allowed to have both property initializer and explicit backing field specified. | ||
|
||
```kotlin | ||
val city: StateFlow<String> = field.asStateFlow() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about cases this case:
val city: StateFlow<String> get() = field.asStateFlow()
field = MutableStateFlow("")
Will it be supported? Now it's not really clear for me from the KEEP text
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. It is described in "Accessors" chapter:
Property, which has explicit backing field, can also specify getter and/or setter.
val counter: Int
field = atomic(0)
get() = field.value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, indeed. Thanks
339ea47
to
1880e68
Compare
See explicit-backing-fields.md for the full text.
Please discuss in issue #278.