dart-lang / language

Design of the Dart language
Other
2.65k stars 202 forks source link

Augmenting external declarations, and external augmentations #3934

Closed lrhn closed 2 months ago

lrhn commented 3 months ago

A declaration being external is a way to be non-abstract, but not specify the implementation in the program source. The implementation is provided by the compiler, in some tool-chain specific way. Examples include FFI and JS-interop, and our own platform patching. In many ways external is more like a body modifier, like async*, than a prefix operator, it just replaces the entire body. (But as a prefix, it works with variables too, so that's a reason it is where it is.)

There should be no problem having an augmenting declaration replace the body of an external method.

external void doFoo();
augment void doFoo() {
  print("Before");
  augmented();
  print("After");
}

The augmenting declaration isn't external since its body is provided in the source. Being external is not a property of the logical member, it's a property of the individual syntactic declarations (so not inherited by augmentations, or visible in the final declaration signature in any way.)

We can't flatten that augmentation chain into a single body, but it's not clear we want to do that in all cases anyway. Each augmented call needs its own parameter variables, and having more than one augmented call, even if only one is executed on each call, may cause code size blow-up.

Question: Can an augmenting declaration be external?

String get browserName;

@JS('navigator.userAgent')
augment external String get browserName;

That should work too. The external implementation probably cannot call augmented, but having the implementation replaced/provided for the first time by an external body shouldn't be a problem.

On the other hand, if we don't want the complication, and disallow external augmenting declarations, you can easily rewrite it as:

@JS('navigator.userAgent')
external String get _browserName;
augment String get browserName => _browserName;

If it's a problem, we should probably disallow initially, then we can consider whether to allow later if there turns out to be user demand.

(Current implementation does not allow the augment external combination. It does allow augmenting an external declaration with a non-external body, but dart2js ignores the augmentation. Don't know if it's just not implemented.)

In any case, it's a combination we should be aware of and decide on the behavior of.

jakemac53 commented 3 months ago

We do discuss augmenting external members a fair bit in the current spec, and it works as you suggest generally (although we don't allow augmenting external variables with variables).

It doesn't mention external augmentations, that seems pretty weird to me. You can always add a new method which is external though. I think we shouldn't allow this until we have a reason to.

lrhn commented 3 months ago

That matches what I see. Including not being able to augment the body of an external variable, which makes sense when we say that external int x; is really shorthand for a getter and a setter, not an actual variable. It has no initializer, and it won't ever run one.

Not allowing an external augmenting declaration is probably fine. External declations are a little "not really Dart", so I think people will accept that they need an indirection from the Dart function to the not-really-Dart function.

The current spec (or intended spec, if we can't find the words) seems to be fine and workable. ✓

jakemac53 commented 2 months ago

Should we close this issue, is there anything left to resolve? cc @lrhn

lrhn commented 2 months ago

Looks closable to me!