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

Are macro applications always constructed? #1890

Open
munificent opened this issue Oct 7, 2021 · 14 comments
Open

Are macro applications always constructed? #1890

munificent opened this issue Oct 7, 2021 · 14 comments
Assignees
Labels
static-metaprogramming Issues related to static metaprogramming

Comments

@munificent
Copy link
Member

The macro proposal currently says:

To apply a macro, a Dart compiler constructs an instance of the applied macro's class, and then invokes methods on it that implement the macro API.

But in most of the examples in the prototype, the macro application is a constant object like @jsonSerializable or @dataClass, and not a constant constructor call like @foo(1, 2). In applications where the macro is an object, is it still the case that the compiler constructs a new instance of the applied macro's class? Or does it just call methods directly on that existing constant object?

@munificent munificent added the static-metaprogramming Issues related to static metaprogramming label Oct 7, 2021
@jakemac53
Copy link
Contributor

Good question, we could be ambiguous about this (add a line saying that a previously created instance "may" be used) or something. That might lead to observable differences though between tools.

It might be best to just always create a new one, these should be cheap objects.

@munificent
Copy link
Member Author

But the metadata annotation refers to an actual object, how do you know you can construct it? What if it was constructed given arguments? What if its constructor is private?

I'm sure this is tractable, but it feels like we'll want to tweak something here.

@jakemac53
Copy link
Contributor

What about:

  • If a macro application is a direct reference to a constant object, then a reference to that instance should be used.
  • If a macro application is a constructor invocation, a new object is constructed for each application, as well as each phase in which the macro is applied.

@munificent
Copy link
Member Author

munificent commented Oct 12, 2021

  • If a macro application is a direct reference to a constant object, then a reference to that instance should be used.
  • If a macro application is a constructor invocation, a new object is constructed for each application, as well as each phase in which the macro is applied.

The "as well as each phase" part feels kind of arbitrary. Can we simply say that the macro application is expected to const-evaluate to an instance of a macro class (either because it's a constant reference to an instance, or because it's a const constructor call) and then the runtime just uses that instance directly?

That might raise some tricky questions about transferring that instance over to an isolate, but it seems semantically simplest.

@jakemac53
Copy link
Contributor

jakemac53 commented Oct 12, 2021

That might raise some tricky questions about transferring that instance over to an isolate, but it seems semantically simplest.

This is the problem - the macro expansion isolate doesn't even import the library where the macro application lives, so there is no constant object to refer to. So these instances can't be const. We could do runtime canonicalization or something else, but I doubt its worth the effort to do so.

@munificent
Copy link
Member Author

There's an even more fundamental problem:

Can we simply say that the macro application is expected to const-evaluate to an instance of a macro class (either because it's a constant reference to an instance, or because it's a const constructor call)

We can't always do const evaluation before macro application. We can do constant evaluation in the library where the macro is defined because we know that that library has no import cycles and can be fully compiled (and macro expanded and const eval'd) before we get to the library containing the application.

But you could imagine a foolish user trying to do:

import 'some_macro.dart';

const reusableMacroObj = SomeMacro("some very long string I don't want to repeat...");

@reusableMacroObj class A {}

@reusableMacroObj class B {}

But that definitely won't work. We can't reliably evaluate reusableMacroObj until after macros in this library have been applied. We probably need to say that the name in a macro application must resolve to an identifier in another library that has no import cycles. So, combined with my previous suggestion, the rules are something like:

  1. If a metadata annotation is an identifier:

    1. If the identifier resolves to a library that does not import the current library directly or indirectly,
    2. and the identifier resolves to a constant value whose runtime type implements one of the macro interfaces,
    3. then the annotation is a macro application invoked on the given constant object.
  2. Else, if a metadata annotation is a constructor invocation:

    1. If the name resolves to a library that does not import the current library directly or indirectly,
    2. and the name resolves to a class that implements one of the macro interfaces,
    3. then an instance of the class is constructed with the given meta-lifted arguments and the annotation is a macro application invoked on that instance.

the macro expansion isolate doesn't even import the library where the macro application lives

I think my updated rules address that. The macro expansion isolate does import the library where the macro is defined, and that's where the constant must live.

@lrhn
Copy link
Member

lrhn commented Oct 16, 2021

In applications where the macro is an object, is it still the case that the compiler constructs a new instance of the applied macro's class?

If the code runs in a different isolate (which it does because it runs, and the original program hasn't been compiled yet, so it has no isolate), you need to create an instance of the annotation in that isolate. You cannot use a value which belongs to a different isolate. The two objects are different because they close over different global scopes.

It stretches our notion of identity if we claim the same object, even if it's constant, can exist in two different isolates at the same time. We could probably specify things in a way which allows that notion, but we haven't so far.
(In other words, saying "the same object" about objects in different code which isn't running in the same isolate, makes no sense at all to me. I literally do not know what it means. Can a macro annotation refer to a normal const declaration in the generatee program?)

We can serialize and deserialize a constant object through a SendPort, and canonicalize it with a similar constant object at the receiving end. That means we can talk about the (or an) equivalent object in a different isolate, without claiming it's the same object.

@jakemac53
Copy link
Contributor

What if we enabled an as clause for macro classes, with the following constraints:

  • Only macro classes with an unnamed constructor with zero arguments can have an as clause.
  • If there is an as clause, there is an implicit canonical instance of that macro and the provided name refers to it.

So basically you could do:

macro class DataClass implements ClassDeclarationsMacro as dataClass {...}

And then refer to it as @dataClass.

For macros that take arguments, you could still actually create aliases for certain argument combinations, by just creating a new macro that extends the other and provides specific arguments:

macro class SomeMacroWithArgs extends SomeMacro as someMacroWithArgs {
  SomeMacroWithArgs() : super(some, arguments, here);
}

I worry a bit about the latter case, if that was a common pattern it could potentially cause an explosion of the number of macros that exist, which could come with a significant compilation time cost.

@munificent
Copy link
Member Author

It feels a little heavyweight to add new syntax (especially given how hard we've otherwise tried to avoid new syntax for macros). I believe my previous comment would address this. Instead of:

macro class DataClass implements ClassDeclarationsMacro as dataClass {...}

You could do:

macro class DataClass implements ClassDeclarationsMacro {...}

const dataClass = DataClass();

@jakemac53
Copy link
Contributor

const dataClass = DataClass();

Yes I think conceptually this could work but I also think people will end up wanting to do that in any library, but won't be able to.

It also brings up weird questions - can you only do that in the library where DataClass is defined, or in any library that defines any macro?

If the former, then you end up still needing to create a whole new macro, just to be able to pass the custom arguments (although this isn't worse than my proposal above):

macro class CoolDataClass extends DataClass {
  CoolDataClass(...) : super(...);
}

const coolDataClass = CoolDataClass(...);

If we support the latter it encourages people making dummy macros that are never used just to be able to define these constants.

Other thoughts

  • We could also just allow any constant that isn't in the current library cycle. We might end up having to create a whole separate macro program just to use those though, if we don't always have enough info to reconstruct the original arguments and pass those to the macro isolate.

  • I am also concerned a bit about the feasibility of allowing references to any constants in the macro libraries. Today we can bootstrap a macro isolate without doing any resolution at all. I am not sure we could find all these constants without doing resolution, but possibly we could? If we do something more explicit (like the as proposal, but it could also be an annotation or something that doesn't add new syntax), I would be more comfortable.

@jakemac53
Copy link
Contributor

At least for now, I am going to require explicit constructor invocations for macro annotations. I think that greatly simplifies the model, and it isn't too much of a burden.

We can always add the functionality for using constant references later on.

I will leave this open though as its a good discussion, but move it to the backlog for later releases.

@lrhn
Copy link
Member

lrhn commented Jul 11, 2023

Should we just require macro imports to be done using macro import "...", and only names coming from such can trigger macro invocation.
(And maybe those imports would not beer part of the program that is compiled.)

Or is that what we're already doing?

@jakemac53
Copy link
Contributor

jakemac53 commented Jul 11, 2023

Unfortunately that wouldn't be a good solution (to this specific problem) imo, especially for modular compiles but really for any compilation.

Basically today, the macro expansion programs we create only need to import macro definitions, which is a good thing. If we start allowing other random user code we will end up with likely far more of these programs and we want as few as possible (they are separate apps that have to be compiled etc). We also want their import scopes to be as small as possible, and random users are less likely to understand that than macro authors (they are likely to jam these constants into a library with lots of other unrelated imports).

@jakemac53
Copy link
Contributor

(I do think we will want either macro imports or macro libraries for other reasons though, to ensure the code from them is only present at compile time)

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Jul 13, 2023
dart-lang/language#1890 (comment)
dart-lang/language#3205

Change-Id: I20a181a01eab4fef9a8bf6e568745eb2b8e86d6d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/312881
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
osa1 pushed a commit to osa1/sdk that referenced this issue Jul 17, 2023
dart-lang/language#1890 (comment)
dart-lang/language#3205

Change-Id: I20a181a01eab4fef9a8bf6e568745eb2b8e86d6d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/312881
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
static-metaprogramming Issues related to static metaprogramming
Projects
None yet
Development

No branches or pull requests

3 participants