Re-think augmenting constructor bodies

[jakemac53]

We realized today that in particular augmented expressions are extremely weird for constructors. You do not want to re-run the original initializer list for instance. We could say that you just always invoke it with zero parameters, and it runs the original body with whatever the current scope is, but that also feels very weird.

We could also say that augmented is banned in augmenting constructor bodies, since the semantics are just too difficult to specify.

We could also just say the original constructor body is implicitly called, and specify the ordering in which that happens.

Or, we can say you can only add a constructor body where none existed previously, but cannot replace an existing one.

[polina-c]

To instrument disposables using macros, I need to augment constructors: flutter/flutter#137435
So, will appreciate prioritization for this.

For now, experimenting with empty-body constructors.

[polina-c]

After fix for error reporting I started seeing the error. It is "Unsupported operation: Augmenting existing constructor bodies is not allowed.".

It blocks the prototyping for me. Any chance at least empty constructors can be supported soon?

I use 0.1.0-main.5.

[polina-c]

@jakemac53

[rrousselGit]

The Freezed macro tries to generate an augmented constructor at the moment. I think that's a good scenario to test against.

IMO we should treat augmented constructors similar to a : super(..) call ; kind of like : this().
Maybe we could use the augmented keyword in place of the usual super()/this()?

class Base {
  Base({bool? flag});
}

class Foo extends Base {
  Foo({this.a}) 
    : assert(a >= 0),
      super(flag: true);

  int? a;
}

augment class Foo {
  augment Foo({int? a})
    :  _newField = ...,
      assert(...),
      augmented(a: a * 2);

  final int _newField;
}

[jakemac53]

It looks like the analyzer does work here, but the CFE does not. It looks like it must be reporting hasBody: true for constructors with no body? cc @johnniwinther

[lrhn]

IMO we should treat augmented constructors similar to a : super(..) call ; kind of like : this().

The problem with that is that invoking the augmenting constructor means re-evaluating the parameter list, which may contain initializing formals. And should also include evaluating the initializer list in that patientrettet scope, an initializer list which has already been evaluated before we got to executing bodies. And the already executed initializer list could have created a closure closing over the original parameter variables. The augmented body may even assume that an instance variable has been initialized with a closure that closes over a parameter, and that it's the same variable that it has available in the body.
If we rebind the parameter variables in an augmented(args) call, but do not re-execute the initializer list, the resulting state may just be inconsistent. We can't execute initializer lists at this point.

My suggestion is to have augmented, no arguments, execute the augmented body in the same body scope as the augmenting body. It's the only way to be sure. (See also my overly permissive approach to augmenting constructors, whose link I'll give when I get back to a real computer.)

[jakemac53]

#3737 should also resolve the specification with regards to this issue, which should in turn unblock the implementations.

[johnniwinther]

@jakemac53 The CFE doesn't support constructor augmentation yet (only replacing an external constructor with a concrete, as used for patching), so you will see weirdness here.

cc @chloestefantsova

[polina-c]

@johnniwinther , what is 'external constructor'?

[johnniwinther]

An external constructor is a constructor with the external modifier. For instance the List.filled in dart:core is declared as

external factory List.filled(int length, E fill, {bool growable = false});

Dart backends provide implementation for such declarations by the "patching mechanism" which is similar to augmentations. For instance dart2js have

@patch
class List<E> {
  @patch
  factory List.filled(int length, E fill, {bool growable = false}) {
    var result =
        growable ? JSArray<E>.growable(length) : JSArray<E>.fixed(length);
    if (length != 0 && fill != null) {
      for (int i = 0; i < result.length; i++) {
        // Unchecked assignment equivalent to `result[i] = fill`;
        // `fill` is checked statically at call site.
        JS('', '#[#] = #', result, i, fill);
      }
    }
    return result;
  }

where @patch works similar to the augment modifier.

The patching mechanism predates the augmentation libraries feature but because of their similarity I've built the support for augmentations on the patching implementation. In time making patching just a restricted variant of augmentation.

Therefore the current state of augmentations reflect that; I've have not added support for augmenting constructors yet, but it has inherited the functionality already present in patching.