Skip to content
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

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

elizarov
Copy link
Contributor

@elizarov elizarov commented Jan 24, 2022

See explicit-backing-fields.md for the full text.

Please discuss in issue #278.

lunakoly and others added 7 commits September 15, 2021 21:04
* 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.
proposals/explicit-backing-fields.md Outdated Show resolved Hide resolved
proposals/explicit-backing-fields.md Outdated Show resolved Hide resolved
proposals/explicit-backing-fields.md Outdated Show resolved Hide resolved
proposals/explicit-backing-fields.md Show resolved Hide resolved
elizarov and others added 3 commits January 27, 2022 10:27
Co-authored-by: ilya-g <ilya.gorbunov@jetbrains.com>
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.
@shaun-wild
Copy link

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?

Comment on lines 138 to 168
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.
Copy link
Member

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


There are the following additional semantic restrictions on the property declaration grammar:
Visibility of property can be any, except `private`.
Copy link
Member

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.
Copy link
Member

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"?

Copy link

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.
Copy link
Member

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?

Copy link

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

Comment on lines +370 to +425
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.
Copy link
Member

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


- **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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- **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

* [Expose different object](#expose-different-object)
* [`var` backing property](#var-backing-property)
* [`lateinit` backing property](#lateinit-backing-property)
* [Delegation](#Delegation)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* [Delegation](#Delegation)
* [Delegation](#delegation)

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)).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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)).

Copy link
Contributor

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.


#### Hide complex value storage logic

Sometimes we want to provide a public API for getting the instant value of a variable,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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".


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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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


### Accessors

Property, which has explicit backing field, can also specify getter or setter.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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.

* 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)
Copy link
Contributor

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.

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.

Copy link
Contributor

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? =)

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.

* 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)).
Copy link
Contributor

Choose a reason for hiding this comment

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

Why "or"?

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()
Copy link
Contributor

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, indeed. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants