Open ds84182 opened 7 years ago
We will definitely not make the initializer list have access to the real object.
Only giving access to final variables is a little too specific - the next request will likely be to give read-access to non-final fields too (and not unreasonably so).
What we could do instead is to let initializer list assignments also introduce a final variable, visible in the rest of the list, that contains the same value as the one just assigned to the field.
It's potentially confusing - a function expression might capture that variable, and it will be different from the actual field. It's not unprecedented, it is what we do for intiializing formals (this.x
) already.
There are practical problems to solve too. The variable might have the same name as an existing variable:
class C {
final x, y;
C(x, y) : x = x * 2, y = x + y; // What does the final `x` mean?
}
As for the work-around, you can do that without a factory constructor too, which reduces both the textual overhead and the problem of not being extensible:
class GenericProducerWorkaround<T> extends DelegatingStream<T> {
final StreamController<T> _controller;
GenericProducerWorkaround._(this._controller) : super(_controller.stream);
GenericProducerWorkaround() : this._(new StreamController());
}
All in all, it's a possible enhancement, but not one I think will be a high priority any time soon. There are many restrictions on what you can do in an initializer list, this is just one of them. If anything, the entire feature might need a redesign rather than a series of incremental changes.
References to other initialized final fields provides concise declarations of relationships. We are converting from Typescript where we use an object/functional reactive style. For example we transform immutable data (IList
from dartz) sourced from a BehaviorSubject (RxDart) through Stream
mapping functions:
class Obj1 {
final _personsSubject = new BehaviorSubject<IList<Person>>(seedValue: nil());
final Stream<IList<Person>> persons = _personsSubject.stream; // error: Only static members can be accessed in initializers
final Stream<IList<String>> personNames = persons.map((ps)=>ps.map((p)=>p.name)); // error: ^same^
final Stream<IList<String>> personNamesSorted = personNames.map((ns)=>ns.sort(toStringOrder)); // error: ^same^
}
The above won't compile because Only static members can be accessed in initializers
.
Changing from final objects to getter functions:
Stream<IList<String>> get personNames => persons.map((ps)=>ps.map((p)=>p.name));
doesn't provide the same runtime semantics. Each invocation of personNames
returns a new Stream
object instead of sharing the same final object. (Passing a new Stream
on each invocation to an Angular AsyncPipe
currently results in an infinite loop.)
A faux immutable pattern that is immutable external to the object falsely suggests that it can/should be changed internally:
var _personNames = _persons.map((ps)=>ps.map((p)=>p.name));
Stream<IList<String>> get personNames => _personNames;
Keeping it immutable with nested named constructors is unwieldily:
class Obj3 {
final BehaviorSubject<IList<Person>> _personsSubject;
final Stream<IList<Person>> persons;
final Stream<IList<String>> personNames;
final Stream<IList<String>> personNamesSorted;
Obj3() : this._1(new BehaviorSubject<IList<Person>>(seedValue: nil()));
Obj3._1(personsSubject) : this._2(personsSubject, personsSubject.stream);
Obj3._2(personsSubject, persons) : this._3(personsSubject, persons, persons.map((ps)=>ps.map((p)=>p.name)));
Obj3._3(this._personsSubject, this.persons, this.personNames) : personNamesSorted = personNames.map((ns)=>ns.sort(toStringOrder));
}
It's a little cleaner with a factory constructor:
class Obj4 {
final BehaviorSubject<IList<Person>> _personsSubject;
final Stream<IList<Person>> persons;
final Stream<IList<String>> personNames;
final Stream<IList<String>> personNamesSorted;
Obj4._(this._personsSubject, this.persons, this.personNames, this.personNamesSorted);
factory Obj4() {
final personsSubject = new BehaviorSubject<IList<Person>>(seedValue: nil());
final persons = personsSubject.stream;
final personNames = persons.map((ps)=>ps.map((p)=>p.name));
final personNamesSorted = personNames.map((ns)=>ns.sort(toStringOrder));
return new Obj4._(personsSubject, persons, personNames, personNamesSorted);
}
}
Having developers use a syntactic pattern to declare relationships between Dart final value initializers doesn't follow other contemporary languages. Java's final
, Scala/Kotlin val
, even Typescript readonly
allow non-forward references in initializers. For lazy val
Scala even allows forward references - aren't all Dart 2 initializers lazy?
In our code conversion to Dart, this is one of our biggest annoyances since we define almost all data structures as immutable and use stream transformations to shape the data as it flows from the central repository to the U/I. So how about reconsidering this enhancement request's priority?
For any given constructor, we are currently adding a local variable for every formal parameter (including a final local variable for each initializing final parameter) to the scope:
Each initializing formal in the formal parameter list introduces a final local
variable into the formal parameter initializer scope, but not into the formal
parameter scope; every other formal parameter introduces a local variable
into both the formal parameter scope and the formal parameter initializer
scope.
If we also add a final local variable to the formal parameter initializer scope for each initialized instance variable, making it a compile-time error to access such a variable before the end of the initializer element for the corresponding instance variable, and initializing each of them with the value used to initialize that instance variable, we'd get something that provides access to the values that you wish to use without breaking the rule that initializer lists cannot access this
(except in the very specific manner which is initialization of one instance variable by means of the execution of an initializer element).
So the following would now have a compile-time errors as indicated:
class C {
final x, y, z;
C(x1, y1):
x = x1 * y, // Error, accessing `y` before end of initializer `y = ...`.
y = y1 + y, // Error, same reason.
z = x + 1; // OK
}
And the following would work as requested:
class Obj4 {
final BehaviorSubject<IList<Person>> _personsSubject;
final Stream<IList<Person>> persons;
final Stream<IList<String>> personNames;
final Stream<IList<String>> personNamesSorted;
Obj4():
_personsSubject = new BehaviorSubject<IList<Person>>(seedValue: nil()),
persons = _personsSubject.stream,
personNames = persons.map((ps) => ps.map((p) => p.name)),
personNamesSorted = personNames.map((ns) => ns.sort(toStringOrder));
}
This would probably not be very hard to implement, and we have preserved the protection of this
(such that object construction remains a near-functional process: we are creating instances and "installing" them into instance variables of this
, but we can't leak this
to other code, we can't implicitly/explicitly run instance methods on this
, etc.).
One real problem with this approach remains, though, because it aggravates an issue which is already known: If any piece of code captures one of these specialized local variables then the code capturing the variable may be very error-prone, because it does not reflect future updates to the corresponding instance variable:
class A {
var x, y;
A(this.x) : y = (() => x);
}
main() {
A a = new A(2);
a.x = 3;
Expect.equals(a.x, 3);
Expect.equals(a.y(), 2);
}
(This source code is from this test.)
The point is that the function literal captures x
. But that's not the instance variable, it is the special local variable which was introduced in order to provide access to the initializing formal this.x
. So when we invoke the function object stored in y
we get the value of that local variable, and not the value of the instance variable x
. It matters because the local variable kept its original value when the instance variable was updated.
Another issue to think about is that we may well have a huge number of name clashes:
class A {
int x;
A(int x): this.x = x ?? 42;
}
It might be a massively breaking change to introduce the rules mentioned above, because the x
on the right hand side of x
is now a reference to the new local variable providing access to the initial value of the instance variable x
, and it is an error to use that before the end of that initializer.
Alternatively, it would surely be massively confusing if we were to avoid all these conflicts by only introducing the new local variables in cases where there is no such name clash.
So we might be able to do something about this pretty easily, but it might not be so easy to introduce the feature into the universe of existing Dart code.
@eernstg - Your proposal to "add a final local variable to the formal parameter initializer scope for each initialized instance variable" sounds promising. As you show with your Obj4
example, it would clean up our nested named constructor workaround.
I see your point about aggravating the issue shown by the initializing_formal_capture_test.dart
test. The proposal would definitely increase the number of values available in the initializer scope that could be abused. My first thought is - don't capture the value like that. Next thought - if you do capture values, you better understand initializer scope. Final thought - only add initialized final instance values into the initializer scope. The apparent "surprise" happens when the class property is mutated.
Name clashes - yes, the formal parameters shouldn't be shadowed by the initialized values. A name alias could be introduced on either the formal parameter or the initialized value, but I don't think there's a precedent for that. Another possibility (I think I've seen somewhere else) is to allow local variable creation into the initialization scope. Your name clash example could look like:
class A {
int x, y;
A(int x) :
final _tmpX = x ?? 42,
x = _tmpX,
y = _tmpX + 2
;
}
Since the RHS sees the formals first, the y
initializer wouldn't be able to access the initialized x
value. Introducing local _tmpX
variable works around the name clash. All values introduced into the initializer scope should be final so that the above capture can't be further abused to mutate values.
This enhancement request is titled "Access to initialized final fields inside initializer expressions". If the general case is untenable, a solution that focuses on final
properties is still valuable.
@rich-j, maybe the most robust solution would actually be your idea: allow an initializer list element to be a local variable declaration. Then we wouldn't need to introduce those implicitly generated local variables to "shadow" the instance variables at all (they are tricky and confusing, anyway), it would certainly be at least as expressive and flexible as the implicitly generated "shadow" variables (for example, developers can create as many of these 'initializer list locals' as they want), and there is no breakage (in particular: no name clashes in existing code).
The only issue I can see is that it would often be a little bit more verbose:
class A {
int x, y;
// Using 'initializer list locals'.
A.n1(int x): final _tmpX = x ?? 42, x = _tmpX, y = _tmpX + 2;
// Using implicitly induced "shadows" of instance variables.
A.n2(int x0): x = x0 ?? 42, y = x + 2;
}
Quoting lrhn
We will definitely not make the initializer list have access to the real object.
Why exactly? Speaking bluntly, it is annoying and has gotten in my way more than once, like just a few minutes ago. I ended up creating a mutable variable in the class needing the value and then passed the value (a function reference) to it after the initializer list.
What problem does it solve? Especially when other languages are fine with it.
The problem this solves is to avoid giving code access to the object before it has been fully initialized.
Java may be "fine" with this, but that choice leads to stackoverflow questions and subtle bugs.
C++ takes a different approach, and calls in a constructor will not see subclass virtual overrides of virtual methods called by the constructor. That too leads to subtle bugs.
Dart splits constructor calls into two passes: field initialization and the constructor bodies, where the field initialization performed by the initializer list is complete before any constructor body is run. This ensures that no code sees a this
reference which doesn't have all its fields initialized, not even constructor bodies.
If we get non-nullable fields, it becomes even more important to not show fields that have not been initialized yet, because they won't just be null
then, they will truly have no value.
At times, it would be convenient to be able to do more computation in the initializer list, and that's something we can work on without giving up on the clean separation between initialization and this
-access.
So, Dart is more restrictive here, but also less likely to allow something with a subtle bug caused by calling a virtual method before the object has been completely initialized.
That's great that you all are trying to protect the users of Dart from these bugs, but I question how relevant that is in the real world. The question you linked is an academic one, and does not seem to correspond to an actual issue the person had.
In the last 10 years I have worked on large Java projects in the enterprise as well as Javascript projects and other languages. I can't remember a single time where having the real object available in the constructor (or initializer list. I'm using the two interchangeably in this context) caused a bug in dev, int, stage, or production as a story made its way through the tiers.
I do understand that it has the opportunity to cause a bug, but I'm arguing that it rarely plays out in the real world, and guarding against it causes more inconvenience for the developer by requiring him or her to do workarounds, which could lead to other unforeseen bugs.
If you try to use StreamView or Delegating* classes (from the async package) as a superclass you can hit some REALLY awkward syntax:
The implementation would involve having initialized finals as a variable in initializer scope AFTER the initialization. This would make
C(int x) : z = y * 2, y = x * 2;
(assuming y and z are declared final) ill-formed, since the initializer for z tries to access y, which is uninitialized.