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

Access to the AST for macro parameters #26

Open
TimWhiting opened this issue Jul 22, 2021 · 11 comments
Open

Access to the AST for macro parameters #26

TimWhiting opened this issue Jul 22, 2021 · 11 comments
Labels
enhancement New feature or request

Comments

@TimWhiting
Copy link
Contributor

TimWhiting commented Jul 22, 2021

Thought I'd separate this out from #6 (comment) since it is a different request.

The basic idea is to be able to manipulate and consume the AST of expressions as macro arguments, similarly to the proposed statement macros in #24 (comment), but in the declaration context.

(Copied from #6 (comment))

The advantage of AST inspection means that you can have fewer magic strings and utilize more of what the programmer has already written to ensure valid programs with raw unresolved identifiers. If you could inspect the AST of the macro arguments as well it could solve the issues with raw identifiers instead of strings in #23 (comment) as well . The nim programming language has a concept of untyped / typed / static arguments for their macros. https://nim-lang.org/docs/tut3.html. TLDR; untyped arguments just have to be parseable and are not semantically checked or resolved or expanded, and are passed to the macro as a syntax tree. Typed are similar but are resolved. Static are passed as their value.

In Dart that might look like:

class MyUntypedMacro implements ClassDeclarationMacro {
  MyUntypedMacro(UntypedExpression ast, TypedExpression typedAst, int value);
  void visitClassDeclaration(
      ClassDeclaration declaration, ClassDeclarationBuilder builder){
      // `ast` is the actual unresolved AST for (1 + 2)
      // `typedAst` is the actual resolved AST for SomeClass.someMethod
      // `value` is 10 
   }
}
@MyUntypedMacro((1 + 2), SomeClass.someMethod, 10);
class MyClass {}

Where the macro system encounters an UntypedExpression or TypedExpression in the macro constructor it would hand the unresolved or resolved ast when actually constructing the macro instance, whereas the exact value 10 gets passed into the constructor since it is neither an Untyped or Typed expression. This would be limited to expressions, possibly blocks? (though in Nim that is not the case). From the user's point of view, a TypedExpression or UntypedExpression argument of a macro is essentially typed as dynamic.

As a more concrete example, I implemented basic support for such macros in my fork:
Here is a functional widget macro using this approach:
https://github.com/TimWhiting/macro_prototype/blob/main/flutter_example/lib/macros/functional_widget2.dart

Macro Implementation
extension on analyzer.ParameterElement {
  TypeReference get typeRef =>
      AnalyzerTypeReference(type.element! as analyzer.TypeDefiningElement,
          originalReference: type);
}

class FunctionalWidget2 implements ClassDeclarationMacro {
  final ResolvedAST buildMethod;

  const FunctionalWidget2(this.buildMethod);

  @override
  void visitClassDeclaration(
      ClassDeclaration declaration, ClassDeclarationBuilder builder) {
    final build = buildMethod.ast;

    if (build is! ast.FunctionExpression) {
      throw ArgumentError(
          'Build Method is not a Function Expression ${build.runtimeType}');
    }
    final positionalParams =
        build.parameters!.parameterElements.where((p) => p!.isPositional);
    if (positionalParams.isEmpty ||
        positionalParams.first!.type.getDisplayString(withNullability: true) !=
            'BuildContext') {
      throw ArgumentError(
          'FunctionalWidget functions must have a BuildContext argument as the '
          'first positional argument');
    }
    final namedParams = {
      for (var p = 0; p < build.parameters!.parameters.length; p++)
        if (!build.parameters!.parameterElements[p]!.isPositional)
          build.parameters!.parameters[p]:
              build.parameters!.parameterElements[p]
    };

    var positionalFieldParams = positionalParams.skip(1);
    var fields = <Code>[
      for (var param in positionalFieldParams)
        Declaration('final ${param!.typeRef.toCode()} ${param.name};'),
      for (var param in namedParams.values)
        Declaration('final ${param!.typeRef.toCode()} ${param.name};'),
    ];

    var constructorArgs = <Code>[
      for (var param in positionalFieldParams)
        Fragment('this.${param!.name}, '),
      Fragment('{'),
      for (var param in namedParams.keys)
        Fragment(
            '${param.isRequired ? 'required ' : ''}this.${param.identifier!.name}, '),
      Fragment('Key? key, }'),
    ];

    var widgetName = declaration.name;
    var constructor = Declaration.fromParts(
        ['const $widgetName(', ...constructorArgs, ') : super(key: key);']);

    var method = Declaration.fromParts([
      '''
@override
Widget build(BuildContext context) ${build.body.toSource()}
'''
    ]);

    builder.addToClass(Declaration.fromParts([
      ...fields,
      constructor,
      method,
    ]));
  }
}

and the application:

@FunctionalWidget2((BuildContext context,
    {String? appTitle, String? homePageTitle}) {
  return MaterialApp(
      title: appTitle ?? 'Flutter Demo',
      theme: ThemeData(primarySwatch: Colors.blue),
      home: MyHomePage(title: homePageTitle ?? 'Flutter Demo Home Page'));
})
class MyApp2 extends StatelessWidget {}

Ideally I'd like to get rid of the class declaration and generate that as well using an additional UntypedAst parameter, but this is currently impossible since annotations have to be attached to a declaration

@FunctionalWidget2(MyApp2, (BuildContext context,
    {String? appTitle, String? homePageTitle}) {
  return MaterialApp(
      title: appTitle ?? 'Flutter Demo',
      theme: ThemeData(primarySwatch: Colors.blue),
      home: MyHomePage(title: homePageTitle ?? 'Flutter Demo Home Page'));
})
// Problem no element to attach the annotation to

In order to make this work the annotation would have to be on the library or be like the proposed statement macro (omitting the @ - which would be actually really nice).

FunctionalWidget2(MyApp2, (BuildContext context,
    {String? appTitle, String? homePageTitle}) {
  return MaterialApp(
      title: appTitle ?? 'Flutter Demo',
      theme: ThemeData(primarySwatch: Colors.blue),
      home: MyHomePage(title: homePageTitle ?? 'Flutter Demo Home Page'));
})

In general with this you can use bare identifiers / references instead of Strings in the macro instantiation, which makes it look a ton better. It looks more like a complete program, rather than a partial hacky program that magically turns Strings into real identifiers and code.

@jakemac53
Copy link
Owner

I had some similar thoughts, and I do generally like this idea 👍 . There are some things to work out with the approach, a couple initial thoughts:

  • We really are trying to avoid actually exposing any AST classes. This makes macros very brittle - for example just look at how many versions the analyzer has had. We don't want the language itself to be hampered by exposing this api. Is it enough to just expose these arguments as Code classes? We have considering adding some sort of "stringly typed" api to Code to allow you to traverse it without actually getting access to the ast classes also - no details at the moment though.
  • I think the Resolved versions of code are likely problematic since we can't resolve implementation code until all macros are done, or at least the first two phases. Do we really need this "resolved" version, or is an unresolved ast sufficient? Possibly we only expose resolved asts in the "definition" phase?

cc @munificent

@jakemac53
Copy link
Owner

Note that also with this approach, if we give you bare "Code" objects, then you can just use those directly in your macro when building up the actual code, and we should be able to map that back to the original source location. It is a bit weird to set breakpoints in annotations to be sure, but we could make that work I think.

@jakemac53
Copy link
Owner

@goderbauer @Hixie do you think this feature would assist at all with the other cases we have discussed where we really want to generate multiple constructs from a single one (stateful widget + state class for instance)?

I would be less concerned about for instance performing some more significant mutations on a class, (splitting it into two classes), if that class was passed more clearly as a construct to a macro, as opposed to just annotated with a macro?

@jakemac53 jakemac53 added the enhancement New feature or request label Jul 22, 2021
@TimWhiting
Copy link
Contributor Author

We really are trying to avoid actually exposing any AST classes. This makes macros very brittle - for example just look at how many versions the analyzer has had. We don't want the language itself to be hampered by exposing this api. Is it enough to just expose these arguments as Code classes? We have considering adding some sort of "stringly typed" api to Code to allow you to traverse it without actually getting access to the ast classes also - no details at the moment though.

Not quite sure what you mean by a stringly typed api, essentially I imagine it being very similar to the AST, just making sure you can't access / modify the compiler's AST yourself. Yes, the analyzer goes through lots of versions, but they reflect the changes in the language. Ideally the AST api would be separate from the analyzer.

I think the Resolved versions of code are likely problematic since we can't resolve implementation code until all macros are done, or at least the first two phases. Do we really need this "resolved" version, or is an unresolved ast sufficient? Possibly we only expose resolved asts in the "definition" phase?

A resolved AST would be good to validate types for definitions #21, so I agree that they make most sense in the definition phase. I use it in the example to get the type of the arguments, but hopefully a "stringly typed" argument type would be available in your proposal. I didn't see a way to get the types of the arguments to create fields unless I had a resolved AST.

@Hixie
Copy link
Collaborator

Hixie commented Jul 22, 2021

I have many (not all coherent or consistent) opinions here. I'm not sure they are helpful, sorry.

@jakemac53
Copy link
Owner

Not quite sure what you mean by a stringly typed api, essentially I imagine it being very similar to the AST, just making sure you can't access / modify the compiler's AST yourself.

What I mean is not a static api, think of accessing the AST through more of a map-like structure (with string keys) rather than an actual api. The usability would be poor, but we could possibly avoid breaking people as much. We haven't delved deeply into this, its a very early idea :).

Yes, the analyzer goes through lots of versions, but they reflect the changes in the language. Ideally the AST api would be separate from the analyzer.

The api would be different from the analyzer AST - my point is just that it would be every bit as brittle. Given this would be a core API shipped with the language, that is a very risky business. It ends up hampering us from doing any language feature that would be breaking for the AST api, because that would break existing macros, and that isn't a place we want to be in.

@TimWhiting
Copy link
Contributor Author

It ends up hampering us from doing any language feature that would be breaking for the AST api, because that would break existing macros, and that isn't a place we want to be in.

Isn't it already breaking for code generators? Or I guess code generators can specify a version of the analyzer which works on multiple compiler versions, whereas this would be tied to a compiler version since that is what would pass the macro the argument? Even if this is the case, it seems like users would just end up creating a package with a strongly typed API to transform the map-like structure into something they can easily navigate. Good use case for the views proposal. I guess a stringly typed api could change independently, and the package could support different ways of navigating the stringly typed api with some sort of version checking / error recovery to handle multiple versions of the compiler. So I guess I answered my question there.

It still seems like it would be just as breaking to change keys of the map-like structure as it would to change a class structure. Either way, macros might stop working, with a class structure it would be an macro compile time error, and with a map (accessing a key that changed) it would be a macro runtime error. Seems like the AST or macros need to follow language versioning unless macros only add and don't introspect. The analyzer already changes the language version when it updates the API for a newer compiler doesn't it?

@jakemac53
Copy link
Owner

Isn't it already breaking for code generators? Or I guess code generators can specify a version of the analyzer which works on multiple compiler versions, whereas this would be tied to a compiler version since that is what would pass the macro the argument?

Yes, but code generators exist outside of the sdk. In effect, it isn't the languages problem. And having analyzer constraints also does help. Macro authors could do very tight sdk constraints but it would be a large burden on those authors, while the vast majority of the time they actually would not be broken.

Even if this is the case, it seems like users would just end up creating a package with a strongly typed API to transform the map-like structure into something they can easily navigate.

Exactly, a package is a whole different ballgame. It can be separately versioned and controlled - and it isn't specified as part of the language/core libs.

It still seems like it would be just as breaking to change keys of the map-like structure as it would to change a class structure.

It won't fix all situations, to be sure. But it would eliminate the cases where something is statically breaking but not dynamically breaking. Again this isn't a fully fleshed out thing though, just something we have been thinking about.

I really don't think exposing full AST classes is on the table at all at this time. It is simply too damaging to our long term ability to modify the language. We are trying to figure out if we can give similar power (at least the important bits), without the same problems. Even if the api kind of sucks to work with.

@goderbauer
Copy link
Collaborator

do you think this feature would assist at all with the other cases we have discussed

Being able to pass a method to a macro would be useful for some of the use cases I looked at, see also #23 (comment).

With this proposal, do the methods have to be static? Or could you also pass in an instance method of a class? There are some use cases where writing something like this would be nice and concise:

class Foo {
  @CallWhenChanged(Foo._handleBarChange)
  int bar = 10;

  // Gets called whenever a new value is assigned to bar.
  void _handleBarChange() {
   // Do whatever...
  }
}

@jakemac53
Copy link
Owner

jakemac53 commented Jul 26, 2021

That is an interesting thought and it would help you to be able to not use Strings to represent expressions like that. I think that probably what we would actually want is to just allow you to write any expression, and maybe allow you to define what the resulting type of that expression should be.

So for instance maybe the parameter type for that example would be Expression<void Function()>. The expressions would probably be resolved in the scope of the class, so you would actually write @CallWhenChanged(_handleBarChange) (without the Foo). We would need to make sure you aren't then allowed to use that expression outside of the class (in a library declaration, or in static declarations on the class for that matter), but that is likely doable.

@jakemac53
Copy link
Owner

Note that my current prototype can't implement this - that would require a more real prototype as it would involve new language features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants