Consider whether to loosen "must match the original" in augmentation spec

[davidmorgan]

I noticed

• Augmenting constructors signature must match the original
• Type parameters must match the original with bounds and names
• Function signature must match the original

--in the context of macro (incremental) build performance these are all somewhat counter-productive: the macro must do work to find out about the signature enough to reproduce it exactly, including adding any needed imports; then the compiler must do work to check that it was reproduced correctly.

Would it maybe make sense to loosen the constructor+function signature requirements to be just the names + structure, like you would write when overriding a method and expecting the compiler to infer the types from super?

By avoiding mentioning types that could be correctly inferred the macro can be cheaper to compile and invalidated less often.

Re: type parameters, I can't think of any nice way to remove the need to mention types for type parameter bounds; but it does seem unfortunate that a macro that wants to do something unrelated to the type Foo:

@UsefulMacro()
class X<T extends Foo> {}
--- macro output
augment class X<T extends Foo> {
  void usefulMethod();
}

now has to rerun for changes to it. @jakemac53

[davidmorgan]

Looks like some checking is missing today :) ... dart-lang/sdk#55478

[jakemac53]

We could allow inference to fill in types potentially... but omitted types also don't play very nicely with macros, since initially they only see the syntax so they will just get "OmittedTypeAnnotation" instances which are not very useful, and can't be inferred until later on, plus the actual inference of those types when it does happen will involve further dependencies being added.

[davidmorgan]

Hmm I'm not sure I follow--isn't a macro more likely to be looking at the original declaration than the augmentation? So the types will be there. Can a macro even introspect an augmentation?

[jakemac53]

Hmm I'm not sure I follow--isn't a macro more likely to be looking at the original declaration than the augmentation? So the types will be there. Can a macro even introspect an augmentation?

It isn't specified, but definitely some things must involve some knowledge of the augmentation, such as the hasBody boolean for functions (if any augmentation has filled one in, it should return true).

Consider also a hand written augmentation which only applies a macro:

augment class A {
  @SomeMacro()
  augment foo(x);
}

Here at least the expectation might be that it sees exactly the declaration it was applied to and not the "original".

[lrhn]

I'd expect the macro to see the result of all augmentations, not the individual syntactic declarations. Or, if it's applied on an augmentation, then all augmentations up to and including that file and is subtrees (which is what it'll be augmenting if it's generated code is put into a part of that file).
There is never any reason to see individual augmentation declarations.

So for augment class A { @SomeMacro() foo(x); }, it should see foo as it would be after applying everything in the current subtree. That's what it is augmenting.

[lrhn]

We can definitely make augmenting declarations inherit some parts of its augmented declaration's signature of it's omitted.

• return type, augment foo(int x, int y)
• parameter types, augment foo(x, y)
• parameters, augment foo(, y)
• parameters entirely augment foo() or even augment foo, although the latter might make it hard to determine which kind of declaration it's intended to augment. But as long as it's not wrong, we can apply it to anything.
• type parameter bounds, augment foo<X, Y>()
• type parameters, augment foo<X,>()
  -all type parameters, augment foo().

In each case, the parameters or type parameters of the augmented do declaration are inherited to fill out the function signature.

I'd probably not allow a body to refer to a name that it omitted, but if you just want to add an annotation, @annotation augment foo; could work for any method or variable declaration.
Probably cannot omit get, set or ClassName. from constructors.

[davidmorgan]

Thanks; re: introspecting on augmentations, merging from the point of view of the macro SGTM.

Re: omitting, I think we decided it would be a good thing to encourage omitting types in overrides once there is an override keyword, so that you can do it "safely", by which we meant, knowing that something sensible will be inferred; with augment it's always "safe", so presumably the same argument applies :)

[jakemac53]

While it is safe that doesn't make it good - I do think I would prefer to see types on all overrides and augmentations. You can't easily infer the type by looking just at the text, you have to go to the original declaration, and the experience outside of a proper IDE is definitely improved by including the types, imo.

[davidmorgan]

Hmmm, I don't think I'm convinced by "looks good".

There will be many cases where generated augmentations simply don't care about the types.

Isn't it then clearer if they omit them?

augment foo(a, b, c) {
  return _cast(_cache[[a, b, c]] ??= augmented());
}

vs

import 'package:examples/foo.dart' as prefix0;
import 'package:examples/barbaz.dart' as prefix1;
augment prefix1.Baz foo(
    int a,
    prefix0.Foo<Bar<prefix1.Baz, prefix1.Bar>> b,
    prefix1.Bar<prefix1.Baz, prefix1.Bar> c) {
  return (_cache[[a, b, c]] ??= augmented()) as prefix1.Baz;
}

if an augmentation has been generated by a macro, I have to be in an IDE to see it anyway, and can easily find the original declaration and the types.

On the other hand if I'm writing augmentations by hand, I may simply be annoyed at having to go and update my augmentations for type changes when it has no effect. Or, I may choose that I want the annotations and happily go and update them :) which seems fine.

[jakemac53]

I think the implication of this issue though, is that we would never include types on any macro generated augmentations? If it helps significantly for performance reasons that could be compelling.

My responses below can be summed up mostly as "lets treat them the same as overrides". And, we don't require the types on overrides today. So, that is reason enough for me to be on board, even if I wish overrides didn't work that way :).

Hmmm, I don't think I'm convinced by "looks good".

While the prefixes make things a lot more gross today, we do have a path to removing most of those, and then it looks quite a lot less bad if you do. I have never once had a reviewer ask me to remove types from an override.

There will be many cases where generated augmentations simply don't care about the types.

The same argument could be made for overrides, they are essentially the same. I think in practice almost all overrides do duplicate the types, even if they don't have to. But, it would be consistent to not require them here.

[davidmorgan]

Yes, I think it would be strange if augmentations are stricter that overrides here, when augmentations are more targeted at a use case that doesn't need/want the types.