Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
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

Typed metadata #73

Closed
wants to merge 6 commits into from
Closed

Typed metadata #73

wants to merge 6 commits into from

Conversation

basro
Copy link

@basro basro commented May 11, 2020

Add typed metadata that has to be imported to be used.

Rendered version

In the proposal I describe a way in which it could be implemented. I'm probably not the best person to design that so take it with a pinch of salt.
The important part is being able to import and declare metadata as a type.

@Aurel300
Copy link
Member

So in your proposal a metadata is just a marker type? I guess this improves the situation a little bit, by making sure that the metadata is at least declared. The macro-based type following doesn't seem too elegant to me (even if it was hidden behind the stdlib). The problem is that a library that wants to make use of a metadata still has to search through a lot of data, e.g. in a onAfterTyping callback. It would be great (for performance) if the application of a typed metadata reduced this need for extensive search.

@Gama11
Copy link
Member

Gama11 commented May 11, 2020

The typed metadata topic is something that comes up all the time (there are even some notes based on those discussions here), so I think it's almost certainly something that's going to happen at some point.

However, this proposal seems too bare-bones to me. IMO it's not nearly typed enough:

  • it doesn't include any mechanism to specify what parts of the AST the meta can be used on - we have metadata that can only be used on types / type parameters / parameters / expressions, or any combination of these
  • it doesn't include a way to type the arguments of the metadata, which is also very important for code completion

There were also ideas to base typed metadata on functions (particularly module-level ones in the future I imagine), which would play well with the second part (specifying the arguments) in particular.

@basro
Copy link
Author

basro commented May 11, 2020

I was not aware of the previous discussion on the topic, it's nice to know that it's probably going to happen.

Regarding the missing features both @Gama11 and @Aurel300 bring up, the answer in this proposal is metadata on the metadata type. In the Opening possibilities section I mention a @.haxe.meta.BuildField metadata, in the same fashion one could add @.haxe.meta.AllowedAST(fields, expr) and @.haxe.meta.AfterTyping(call.this.Macro.func)

An example metadata type would be:

@.MetadataType
@.AllowedAST(fields)
@.BuildField(my.lib.BuildMacro.buildField)
abstract MyMeta(Any) {}

Not too sure about the metadata arguments, probably metadata too but waiting for the module level functions might be best.

@RealyUniqueName
Copy link
Member

I didn't get what's the reason for typed meta to be represented as an abstract?
How does a code in the abstract interact with typed meta mechanism?

@Aurel300
Copy link
Member

@RealyUniqueName I think the idea is that the type does nothing, it is just a marker type allowing it to be imported. The code in the abstract is not related to the metadata, at least not in the proposal at the moment. An empty abstract would generate nothing in the generated code, which is good. Another possible choice might be an empty extern class.

@basro
Copy link
Author

basro commented May 12, 2020

@RealyUniqueName: It's as Aurel300 said. I needed a type so my options were to introduce a new type kind or use the ones that already exist. I decided to go for the "easier to implement" option. It is mentioned in the unresolved questions section that a new type might make sense.

@skial skial mentioned this pull request May 13, 2020
1 task
@nadako
Copy link
Member

nadako commented May 26, 2020

I'm against this. While I completely understand the problem and fully agree with motivation, I don't like both the new syntax and that metadata are defined with types.

Regarding the syntax, IMO having @: vs @ separation is already bad enough and I don't think we should add @. to this mix.

Regarding meta-as-type-declarations, I think it does make sense in .NET or Java, where it is possible to get an instance of such type at run-time and access its fields, however Haxe metadata is closer to Python/TypeScript decorators IMO and ideally, in the long term it should be kind of decorators that process decorated AST nodes at compile-time. Also, not fan of uppercase-first-letter naming. :)


I also have some rough ideas on how to approach this, but I don't have time to come up with a proper evolution proposal currently. The basic idea is that metadata should be defined as static (or now module-level) functions, with its signature declaring possible arguments for both IDEs and compiler/macros. Then you can import these functions normally and use with normal @ syntax. Something along these lines;

@metadata function access(type:TypePath);
@metadata function expose(?name:String);

and then

import Metas.expose; // also, built-in compiler metadata globally imported by default

@expose("hello") function greet() {}

This still requires a lot of design, especially regarding nice type-safe macro API, but I like this approach and I think it can be achieved if we put some effort into it. Moreover, as a next step, someday we could even make these functions actual macro functions that would work more or less as Python/TS decorators, but at compile-time.

@basro
Copy link
Author

basro commented May 26, 2020

I must say I like your idea better too @nadako. Once I became aware that this was an already discussed idea I knew that my proposal had no wings ;P

Regarding rejecting the @. syntax, does that mean that typed metadata would be a backwards compatibility breaking change?

@nadako
Copy link
Member

nadako commented May 26, 2020

does that mean that typed metadata would be a backwards compatibility breaking change

I didn't think this through yet, but I presume it can be done in a backward-compatible way:

  • when "typing" @something, check if we have @metadata function something() imported in current context: if we do - proceed with the new thing, if we don't - fallback to old untyped metadata
  • emit a warning for the old untyped metadata: it could be a typo for the new one (provide levinstein hint too!), or just plain old metadata that needs to be upgraded to the new system.

@RealyUniqueName
Copy link
Member

I think we still need some distinction in meta naming to catch typos.

import Metas.expose;

//is "exposer" a typo or an intended name of a different meta?
@exposer("hello") function greet() {} 

@nadako
Copy link
Member

nadako commented May 27, 2020

  • emit a warning for the old untyped metadata: it could be a typo for the new one (provide levinstein hint too!), or just plain old metadata that needs to be upgraded to the new system.

In my "plan", that should deal with it ^^ I think it should be enough if we are to eventually remove untyped meta.

@Simn Simn added the rejected label Jun 18, 2020
@Simn
Copy link
Member

Simn commented Jun 19, 2020

We have decided to reject this proposal in our haxe-evolution meeting yesterday.

While we want to have typed metadata, this approach in particular isn't ideal. As @nadako has already pointed out, there should be better solutions to this which we will discuss in the future.

@Simn Simn closed this Jun 19, 2020
@skial skial mentioned this pull request Jun 19, 2020
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants