Skip to content

[Field Promotion] Promotion of fields on final classes #3050

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

Open
leafpetersen opened this issue May 8, 2023 · 4 comments
Open

[Field Promotion] Promotion of fields on final classes #3050

leafpetersen opened this issue May 8, 2023 · 4 comments
Labels
class-modifiers Issues related to "base", "final", "interface", and "mixin" modifiers on classes and mixins. field-promotion Issues related to addressing the lack of field promotion

Comments

@leafpetersen
Copy link
Member

leafpetersen commented May 8, 2023

With the new class modifiers feature, we have the ability express classes for which all subtypes are restricted to the same library as the defining class. This provides a path to defining a sound version of field promotion in which final fields on instances of final (or sealed) classes could be subject to promotion. This issue is for discussing the tradeoffs involved.

Reasons not to do this

Several objections have been raised to this proposal, summarized here.

Removing the final modifier becomes a breaking change

Currently, it is not a breaking change to remove the final modifier from a class. Allowing promotion on fields of final classes would change this - if the class had any public final fields, removing final from the class breaks downstream clients. For sealed, it is already very breaking to remove the modifier: however since sealed classes are abstract, they are mostly only useful through their non-sealed subclasses which just pushes the issue off.

Changing a final field to a getter (and vice versa) becomes a breaking change.

Currently in Dart it is not a breaking change to switch between a final field and a getter. This field/getter symmetry is valued by many Dart programmers. Allowing promotion of final fields would break this, since changing a final field on a final class would break any clients using promotion. Moving in the other direction (getter to field) is also in principle breaking, since in more limited cases it can be breaking to do promotion where it was not previously done.

Note that this does not just apply to fields defined in final classes. For any public class except one marked interface, moving between a field and a getter would be breaking:

  • For a final class, doing so changes promotion behavior directly
  • For a base or simple class, there may be unknown final classes which inherit from the class and expose the member to promotion

Options for addressing the concerns above

Local promotion only

We could allow promotion only for uses of final fields on final classes within the same library as the definition of the class. We'd also need to restrict this to fields which are themselves declared in the same library as the class (rather than inherited) to avoid implicitly promoting a field from another library.

Note that it's not clear that we would want to restrict ourselves to final classes in this case: there's no real reason not to do this for all classes. [Edit: as pointed out below, this doesn't work of course because of overrides]

Explicit opt in at the field declaration

Use some kind of syntax at the definition site of the field (e.g. something like stable getters) to mark fields which are intended to be promoted

cc @dart-lang/language-team @mraleph

@leafpetersen leafpetersen added the field-promotion Issues related to addressing the lack of field promotion label May 8, 2023
@lrhn
Copy link
Member

lrhn commented May 8, 2023

(Local promotion only:)

Note that it's not clear that we would want to restrict ourselves to final classes in this case: there's no real reason not to do this for all classes.

We need to be sure that we are actually accessing the final field, so all subclasses must be declared inside the same library (so the class must be sealed, final or private, and all subclasses must also be sealed, final or private), and all concrete subclasses must all have promotable fields too.

Our existing local final field promotion handles the "private" part of this. We could extend it to cover final as well, since it prevents external subclassing just as efficiently.

Or more generally: If we have a library-contained subclass hierarchy, and all concrete subclasses in this hierarchy have final fields for a particular name, then we can safely consider that getter stable inside that library, and promote its value.
(But not outside the library, because it's a property of the current implementation, so not something that should be considered stable across an abstraction boundary. Changing to a getter would be a breaking change that isn't detected immediately, and can't be fixed at the same time. I'd be willing to extend promotion to the same package, but the class hierarchy still needs to be restricted to a single library, because if a type is subtypeable outside the library, we can't know that it doesn't also leave the package - we don't have "package-private libraries" yet.)

But we can now add final to the ways to ensure that a subclass hierarchy is library-contained, where we previously only had "private".
(Except for platform libraries, for now, since final can be ignored.)

However, including "private" in the allowed subclasses is probably too tricky. We need to prevent type aliases for the private class being visible outside of the library too, so maybe "sealed or final" for the class, and all subtypes declared in the same library, and all of those having only final fields for that getter.

What we have today (soon) is:

  • Private final fields locally promotable, if all locally implemented members with that name are final variables, and no non-throwing implementation can occur in any subtype declared in other libraries.

What we can add is:

  • Public final fields locally promotable, if all locally implemented members in subtypes with that name are final variables, and no subtypes can be declared in other libraries.

@munificent
Copy link
Member

munificent commented May 10, 2023

Your summary of the objections covers my exact concerns with allowing promotion on all final fields very well.

if the class had any public final fields, removing final from the class breaks downstream clients. For sealed, it is already very breaking to remove the modifier

Exactly right. And, in fact, a large part of the motivation for adding both final and sealed for classes was specifically to enable users to distinguish these two cases. The final modifier does not enable exhaustiveness checking so that removing that modifier isn't a breaking. Conversely, sealed is its own modifier specifically so that users (hopefully!) understand that removing sealed is breaking change.

That suggests a strawman:

Sealed fields

A field can be marked sealed. That opts the field into promotability. In order for that promotion to be sound, the field must have a fixed unchanging value, so marking a field sealed:

  • Implicitly makes the field final too with no setter.
  • Makes it an error for a subclass to override the field. (We could consider loosening this to Erik's stable getter proposal and allow overrides that always yield the field's value.)
  • Makes it an error for an implementing subtype to implement it with anything but a sealed field.

By the last two bullet points, I mean basically that every implementation of a sealed field must reliably resolve to an actual sealed field. I'm not sure if I've thought through these restrictions correctly. Based on how class modifiers went, I'm fairly certain I have not thought it through well and there are many holes. :)

But the basic idea is that we give users a different modifier they can use on fields that gives them promotability but in turn makes the API more rigid with regards to breaking API changes and overrides.

I don't know if this is actually a good idea or not. Asking users to choose promotability versus evolvability for each field declaration might be a big lift. I don't know what the best practices would be around when to use sealed versus final.

@leafpetersen leafpetersen added the class-modifiers Issues related to "base", "final", "interface", and "mixin" modifiers on classes and mixins. label Jul 18, 2023
@Reprevise
Copy link

I'd much rather have a modifier like closed which could be applied to fields and classes. I'm not that big on the idea of having to mark each field as sealed for promotion, considering the fact that most of the classes I'd write would be closed (thinking about Flutter widgets). Marking a class as closed would probably have to disable extends/implements unless the language will not allow you to override the fields.

@TekExplorer
Copy link

thats what final is for.

i think that it would be sane to just have final classes allow for local field promotion since you can easily tell that nothing else overrides it.

it also will not be breaking to remove final, or add a re-opened subclass, because you, the owner of the library, are the only one affected.

outside of the library, its treated as the getter it sees. (since its not like anything outside of the class can know its a field anyway - that's what the stable getters proposal is for.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
class-modifiers Issues related to "base", "final", "interface", and "mixin" modifiers on classes and mixins. field-promotion Issues related to addressing the lack of field promotion
Projects
None yet
Development

No branches or pull requests

5 participants