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

Should we allow eliding final on field declarations in enums and inline classes? #3171

Open
munificent opened this issue Jun 28, 2023 · 2 comments

Comments

@munificent
Copy link
Member

The primary constructors proposal (#3023) says:

In the case where the constructor is constant, and in the case where the declaration is an inline class or an enum declaration, the modifier final on every instance variable is required. Hence, it can be omitted from the formal parameter in the primary constructor:

This mechanism follows an existing pattern, where const modifiers can be omitted in the case where the immediately syntactic context implies that this modifier must be present. For example, const [const C()] can be written as const [C()]. In the examples above, the parameter-and-variable declarations final int x and final int y are written as int x and int y, and this is allowed because it would be a compile-time error to omit final in an inline class, and in a class with a constant constructor. In other words, when we see inline on the class or const on the class name, we know that final is implied on all instance variables.

I'm a little worried about the implicit magic here, but I think the argument makes sense. The brevity is certainly nice.

However, I find the inconsistency with field declarations worrisome. If we really think it makes sense to elide final on parameters in primary constructors on inline classes, shouldn't also allow eliding final on field declarations in inline classes and enums too? The argument seems to equally well to them.

Why not let you write:

enum E {
  one('a'),
  two('b');
  String s; // <- No `final` needed.
  const E(this.s);
}

Whichever way we decide to go, I think primary constructors should probably be consistent with field declarations.

@lrhn
Copy link
Member

lrhn commented Jun 28, 2023

There is a difference, a level of indirection, betwen the parameters of the primary constructor and the fields they imply (and the actual constuctor and constructor parameters that they imply too).
When you look at the constructor, you're not looking at actual field declarations.
It makes sense, to me, to add a necessary final on the implied fields as part of the desugaring that adds fields.

So I'm not worried that

class const Point(int x, int y);

doesn't say final, but the fields are still final. The const told me that, and it's right there on the same line, and I know the parameters are not the fields, they just imply fields.

I'm more worried about reading

class Point {
  int x, y;
  const Point(this.x, this.y);
}

and not being able to see that x and y are final. There can be many more declarations and documentation blocks between the int x, y; and the constructor. And if someone removes the const from the constructor, the meaning changes without any warning.

That is allowing you to omit final from actual field declarations is more iffy, because looking at them, you'd imagine that you know that they mean. There is no local context telling you that this field is actually final.
I'd say that it'd probably be OK for enums, because being an enum is something you should have in mind when reading the class, but allowing it only for enums introduces a difference between an enum-shaped class declaration and an enum declaration, with a migration cliff between them.

Let's assume that we do want to allow omitting final for fields (at least in some places) where they are otherwise required.

I'll assume we don't allow var x;, you have to write a type, or write final, it can't be actively miselading.
And you can always write the final if you want to. (Which likely means we end up with a style guide or lint telling you to always/never omit the final, and at that point we'll allow a choice with no real benefit. Or have started a new style war.)

Where would omitting final from instance fields be allowed?

  • class const <primaryConstructor> declarations, for the parameters only.
  • enum declarations, all fields must be final.
  • inline class, where the single synthetic field must be final.
  • class const <primaryConstructor> declarations, for every field.
  • class Id ... declarations which has at least one const constructor.
  • class const Id ... declarations, which we can allow even without a primary constructor, which forces the class to behave as if it has a const constructor (all fields must be final). Even if all constructors are actually non-const.
  • mixin class const Id ... can only have trivial constructors, but would force all fields to be final and have constant initializers.
  • mixin const Id ... can't have constructors, but would again force all fields to be final and have constant initializers.

This feels like a sliding scale from obvious to obscure.

There is an underlying concept of "must allow being constant" which implies that all fields must be final, and final field initialziers must be constant.

Currently that only affects class declarations with at least one const constructor. We refer to it in mixin applications, to see whether the application can have const forwarding constructors, but currently it just doen't allow any field declarations, not even final ones with constant initialziers.

The clear cut-off points on the scale are:

  • Primary constructor parameters only, don't affect explicit field declarations. That's simple and localized.
  • Existing (plus inline class) declarations where all fields must be final, ones already under the "must allow being constant" requirements: enums, all classes with a const constructors, inline class single field.
  • Allowing us to mark other declaration as "must allow being constant", say by adding const after class (or even mixin), and then getting the effect for all fields - independently of there being a const constructor or not.

I worry that allowing classes-with-const-constructor to omit final is a foot-gun.
Removing the const from the Point constructor above, or having multiple constructors and removing the last (possibly private) const constructor, would change fields to be non-final.
That's a potentailly breaking change (adding setters to the API can make subclasses not implement the interface any more).
Also, it's probably not deliberate.

So if we were to do this, I'd only allow it if the declaration is marked as intending to be constant, which means being an enum or inline class, or being marked with const in the class declaration line, even without a primary constructor:

class const Foo { ... }

You can then remove the last constant constructor (and maybe get a hint that you did so), but the constraint that all fields must be final and field initilizers must be constant is still there, and x and y won't become mutable.
Then enum and inline class wouldn't need the marker to enable "fields must be final", but an inline class could still have it if it wants its primary constructor to be a const constructor.

And if we do go this way, I'd make enum generative constructors not have to say const too. Either the syntax is the same as for non-enum classes, or we go all the way and remove all the unnecessary keywords, not just final.

@eernstg
Copy link
Member

eernstg commented Jun 29, 2023

I'd refer to the localized nature of the class const Foo(int i) mechanism as the most crucial point: The keyword const is going to be visible for anyone who is reading 'int i'. Similarly for inline class Foo(int i).

We have prior art in the rule that allows void f() => 1; and makes void f() { return 1; } an error: In the former case, void is almost inevitably going to be visible when reading the returned expression.

As soon as we allow these syntactic elements to be separated by an unlimited number of tokens it is going to be possible to read int i and have no hint in sight that it means final int i.

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

No branches or pull requests

3 participants