Skip to content

Implicit coercion of Type to Identifier doesn't work #55424

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

Closed
goderbauer opened this issue Apr 10, 2024 · 14 comments
Closed

Implicit coercion of Type to Identifier doesn't work #55424

goderbauer opened this issue Apr 10, 2024 · 14 comments
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). feature-macros Implementation of the macros feature

Comments

@goderbauer
Copy link
Contributor

I am experimenting with macros and ran into the following issue:

My macro definiton:

macro class Foo implements ClassDeclarationsMacro {
  const Foo(this.originalClass);

  final Identifier originalClass;

  @override
  FutureOr<void> buildDeclarationsForClass(ClassDeclaration clazz, MemberDeclarationBuilder builder) async {
    // Intentionally left empty, not important for reproduction. 
  }
}

When applying this macro to a class, e.g.

@Foo(String)
class Bar { }

the analyze is unhappy with this:

error: The argument type 'Type' can't be assigned to the parameter type 'Identifier'.  (argument_type_not_assignable at [macros_playground] lib/main.dart:9)

My understanding is that the Type should be automatically be converted to Identifier.

@jakemac53
Copy link
Contributor

cc @scheglov @johnniwinther this feature would be nice for the stateful widget case (or in general when you want to generate a new class, and run macros on it which want to introspect on the original class).

It would be nice to prioritize it somewhat highly (but behind anything json macro related), to unblock experiments in this area for flutter.

@scheglov
Copy link
Contributor

What kind of Argument should it be?
image

@jakemac53
Copy link
Contributor

Oh, I thought we supported just Identifier, but it looks like not (and they aren't in the spec either).

For this case, a TypeAnnotationArgument would work too though. @goderbauer can you try that, using NamedTypeAnnotation as the field type?

Or, you could just do ExpressionCode, and then assert that you are given an expression with exactly one identifier (var identifier = code.parts.single as Identifier;, maybe with some error handling). But, the type annotation is probably better.

@scheglov
Copy link
Contributor

Well, the specification says:

Here the macro takes an Identifier argument, and introspects on it to know how to generate the desired serialization and deserialization classes.

@scheglov
Copy link
Contributor

There is a complication with identifiers. At least in the analyzer. This is related to the moment when we instantiate macros. We do this right after parsing a file, so all we have is unresolved AST. And without types built and resolved, we don't know if Foo in @MyMacro(Foo) is a type literal, or a function name.

We could disposeMacro() after the types phase, and re-instantiate with different arguments. Is this what you would expect?

Note, that people asked to arguments that are references to functions as well.

@jakemac53
Copy link
Contributor

The macro does not see any details of the Identifier, so what I would do is create an opaque Identifier, which gets its true information filled in after the types phase?

Technically though, it is possible that a regular declaration could be added in the declarations phase, which shadows the identifier for the type....

@jakemac53
Copy link
Contributor

And of course, it also isn't clear what the identifier should look like in the augmentation code from the first phase (what should the compiler say when asked to resolve the identifier to a URI by the augmentation library code?)

@goderbauer
Copy link
Contributor Author

For this case, a TypeAnnotationArgument would work too though. @goderbauer can you try that, using NamedTypeAnnotation as the field type?

I tried both NamedTypeAnnotation and ExpressionCode as the type for the field in the macro without changing anything at the instantiation side (still @Foo(String)). I continue to get the same error.

@scheglov
Copy link
Contributor

What make me worry is using TypeAnnotationArgument for something that might be not a type. If it is just an opaque identifier, then it would be clean to use such kind of Argument. OTOH, if it is definitely a named type like List<int>, we should use TypeAnnotationArgument.

What to do if (after the types phase) it turned out to be a type literal, is an open question. AFAIK in the analyzer we don't rewrite final x = int; from SimpleIdentifier to TypeLiteral(SimpleIdentifier), but that does not mean that this is a good example to follow :-)

@goderbauer Yes, currently the analyzer complains about any use of an identifier as an argument. We are trying to decide what should be done with such identifiers.

@devoncarew devoncarew added the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label Apr 10, 2024
@davidmorgan
Copy link
Contributor

This should be obsolete with dart-lang/language#3728 which will replace coercion with introspection.

@goderbauer
Copy link
Contributor Author

@davidmorgan For my own understanding: Does this mean instead of

@Foo(String)
class Bar { }

we should be doing

@Foo("String")
class Bar { }

and then resolve the string to the String Identifier in the macro manually?

@jakemac53
Copy link
Contributor

There isn't actually a concrete proposal yet, but you should still be able to have an API like @Foo(String) its just how you actually get the identifier for String will be different.

@goderbauer
Copy link
Contributor Author

Thanks for the clarification. Is dart-lang/language#3728 the tracking issue for that or is there a better one I can link to?

@jakemac53
Copy link
Contributor

dart-lang/language#3847 is the one you want :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). feature-macros Implementation of the macros feature
Projects
Development

No branches or pull requests

5 participants