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

Augmented constants? Really? ;-) #3721

Closed
eernstg opened this issue Apr 24, 2024 · 8 comments
Closed

Augmented constants? Really? ;-) #3721

eernstg opened this issue Apr 24, 2024 · 8 comments
Labels
augmentation-libraries question Further information is requested

Comments

@eernstg
Copy link
Member

eernstg commented Apr 24, 2024

Thanks to @sgrekhov for bringing this up. Consider the following example:

// --- Library 'main.dart'.
import augment 'augment.dart';

const String c = "Original";

// --- Library augmentation 'augment.dart'.
augment library 'main.dart';

augment const String c = "Augment: $augmented";

Do we support augmented in constant variable declaration augmentations? I'd assume that we very much want to avoid this, because it brings even more complexity into the management of constant expression values involving macro phases and the like. On the other hand, it is a glaring inconsistency if we don't support it.

I'd recommend that we make it an error to augment a constant declaration. We may then revisit the topic later on if needed.

Alternatively, we should support addition of metadata to a constant variable declaration, but not an augmentation of the initializing expression. In that case we should probably change the syntax such that an augmenting constant variable declaration does not have an initializing expression.

@dart-lang/language-team, WDYT?

@eernstg eernstg added question Further information is requested augmentation-libraries labels Apr 24, 2024
@davidmorgan
Copy link
Contributor

Possibly, augmenting consts is no worse--from the compiler's point of view--than shadowing them?

// --- Library 'main.dart'.
import augment 'augment.dart';
import 'declares_a.dart';

const String c = a;

// --- Library augmentation 'augment.dart'.
augment library 'main.dart';

const String a = "some other value";

In any case, yes, definitely an important one to consider for macro metadata.

@lrhn
Copy link
Member

lrhn commented Apr 24, 2024

It's not impossible to support augmenting constant, but it's also weird.

We may have to rule that macros can't change the value of any constant that occur in a macro application annotation.

(We may want to have rules that macro generated names can't change the resolution of anything that was part of triggering or maybe executing a macro. If so, we probably also don't want macros to change the meaning of constants that were used.
We don't absolutely need to. We can allow macro generation to depend on different resolution and different constants than the final result, after all, only the final result has to be a valid Dart program. Nothing says what macros see using introspection on a partial program has to be correct or final. If the final result is valid anyway, apparently the change didn't matter.)

All in all, I think we can safely allow it, and we can also allow macros to change constants, as long as we can live with the fallout of the change.

(The model of an augmentation application being equivalent to renaming the parent to a fresh privat name that's not used anywhere else also applies here:

const String _$fresh$c = "Original";
const String c = "Augment: ${_$fresh$c}";

and if the resulting program would works, so should the augmentation. Anything that saw the pre-augmented constant may be out-of-date, but we get to decide whether that's a problem, and for whom.
This model only applies to augmenting bodies of otherwise complete and valid declarations. If the augmentee is still missing essential parts, it can't be its own declaration.)

@lrhn
Copy link
Member

lrhn commented Apr 24, 2024

It does bring up the question of what the syntax is for an augmenting declaration that doesn't change something.

final int x = 42;

@Banana()
augment final int x;

Is the second declaration allowed, and does it not change the initializer expression?
Can I write just:

@Banana()
augment final i;

and not specify a type? (Can I omit the final? Probably not, parsing would be horrendous.)

For const, where we don't ever allow omitting an initializer expression, this is even more pressing, but the same rules should apply everywhere.

(The spec is clear: You cannot augment a variable with a variable and not supply an initializer expression. The spec also claims that "Since the initializer is the only meaningful part of the augmenting declaration, an initializer must be provided.", which this example proves wrong - annotations matter too - so maybe we should allow it.)

@jakemac53
Copy link
Contributor

I do think that augmenting constants is potentially useful - although if it was sufficiently motivated I could see limiting it such that you can only fill in a missing initializer, and not replace an existing one.

In particular, I could imagine a macro which generates a large constant object which is derived from the program structure - for example as a dart:mirrors replacement. Or a constant encoder/decoder for a specific type, etc.

@jakemac53
Copy link
Contributor

(The spec is clear: You cannot augment a variable with a variable and not supply an initializer expression. The spec also claims that "Since the initializer is the only meaningful part of the augmenting declaration, an initializer must be provided.", which this example proves wrong - annotations matter too - so maybe we should allow it.)

Good point, we should allow it. You can also add doc comments.

@eernstg
Copy link
Member Author

eernstg commented May 1, 2024

@jakemac53 wrote:

Good point, we should allow it.

I'm not 100% sure what we should allow. ;-)

But certainly we can allow an augmenting declaration of a constant variable to add DartDoc comments and metadata. I'm just not so sure about the overall benefits of allowing an augmenting declaration to replace the value of the constant by some other value.

Filling in a missing initializing expression might be benign (at least, a macro expansion step that must know the value could report a compile-time error if it is not yet available).

In particular, I could imagine a macro which generates a large constant object which is derived from the program structure - for example as a dart:mirrors replacement. Or a constant encoder/decoder for a specific type, etc.

It sounds like we could implement the macro such that it generates the constant variable declaration as a whole, rather than insisting that there is a constant variable declaration (with no initializer, assuming that we introduce the syntax to do that) which is then augmented to have an initializing expression.

@davidmorgan wrote:

Possibly, augmenting consts is no worse--from the compiler's point of view--than shadowing them?

(I'll adjust the example to use parts, based on the assumption that we're about to do that for this feature in general)

// --- Library 'main.dart'.
import 'declares_a.dart';
part 'augment.dart';

const String c = a;

// --- Library augmentation part 'augment.dart'.
part of 'main.dart';

const String a = "some other value";

In this particular case we wouldn't actually reinterpret the meaning of any declared names, because the semantics of parts already implies that the declared names in main.dart and in augment.dart are visible during name resolution on the a that occurs in main.dart. When a is bound to a declaration in the top-level scope of main.dart (which is true because it is declared in a part of main.dart), it will shadow every declaration named a which is imported without a prefix, which means that it doesn't matter whether or not declares_a.dart exports a declaration named a.

However, it is possible to create a situation that gives rise to a re-binding of a name (e.g., when M1 and M2 are two sibling parts where M1 introduces a top-level declaration named n and M2 has an occurrence of n which is proto-bound to a declaration which is imported by M2 or any of its parents).

A proposal about re-binding errors is given near the end of this comment).

@lrhn
Copy link
Member

lrhn commented May 1, 2024

I think we can safely allow constant initializers to be augmented.

There is nothing particularly special about constant initializers, other than the expression needing to be a constant expression, but that is also what would potentially allow augmented to work in an augmenting initializer expression.

One pattern I would expect to see used, is the accumulating list:

const allTheFoos = [];

@Foo("something")
class Banana {}

@Foo("other")
class Apricot {}

where the Foo macro adds augmentations of allTheFoos like:

augment const allTheFoos = [...augmented, FooEntry<Banana>("something")];
augment const allTheFoos = [...augmented, FooEntry<Apricot>("other")];

which would "just work", and it can work with maps too.

That's definitely something people doing dependency injection would love.

The next question is whether we can avoid having augmented constants. If people can get the same effect anyway with some more verbose workarounds, then we might as well give it to them directly.

If they can't change the binding of a constant variable, will they just make that constant variable call a const contructor and augment that constructor instead? (Maybe, if it can work.)

const List<FooEntry> allTheFoos = const _AllTheFoos();
extension type _AllTheFoos(List<FooEntry> _) {
  const _AllTheFoos() : _ = [];
}
augment extension type _AllTheFoos(List<FooEntry> _) {
  augment const _AllTheFoos() : augment this._ = const [...augmented, FooEntry<Banana>("something")];
}
augment extension type _AllTheFoos(List<FooEntry> _) {
  augment const _AllTheFoos() : augment this._ = const [...augmented, FooEntry<Apricot>("other")];
}

If we can augment initializer list entries, then this ... probably still doesn't work, because augmented is only potentially constant, not definitely constant. But it's close enough that I can't rule out that I'll find a way around it.

If it's useful, well-defined and not technically any more complicated than augmenting non-constant variables, I don't see a reason to forbid it.

If changning the meaning of a constant is a problem for macros, macros can choose how to deal with that. The language itself has not problems.
There is no guarantee that constant values can even exist before macros have run, so macros are not changning the meaning of a constant, it's just invalidating our preliminary guess at what the constant would mean in a complete and valid Dart program. It's up to the macro tool to decide if that's a problem or not. It's free to reject a macro run that invalidates the preliminary value guess, if it thinks it's confusing or error prone, or it can just let it pass, and see if the program ends up valid anyway.

@eernstg
Copy link
Member Author

eernstg commented May 10, 2024

Closing: OK, I'll stop worrying about this. If it does create a huge amount of complexity then we can return to the issue.

@eernstg eernstg closed this as completed May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
augmentation-libraries question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants