Skip to content

TargetKind for resource identifiers annotations #55937

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

Open
Tracked by #55555
mosuem opened this issue Jun 5, 2024 · 3 comments
Open
Tracked by #55555

TargetKind for resource identifiers annotations #55937

mosuem opened this issue Jun 5, 2024 · 3 comments
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...).

Comments

@mosuem
Copy link
Member

mosuem commented Jun 5, 2024

Based on the comment on a CL.

We are introducing an annotation @ResourceIdentifier to record usage of methods and their arguments and annotations at compile time, to be used in a link hook for tree shaking assets. The targets of this annotation are for now:

  • Static methods (including methods in extensions)
  • Classes with a const constructor, to target annotations

Possible future targets might be

  • Top level methods
  • Instance methods

It is unclear if instance methods would be possible to track, as the use of dynamic prevents knowing if they are referenced at compile time.

Currently, the placement of the annotation is checked in pkg/front_end. Would it make sense to introduce a custom TargetKind, or a set of TargetKinds, to be placed on the annotation definition itself?

cc @bwilkerson

@bwilkerson
Copy link
Member

Would it make sense to introduce a custom TargetKind, or a set of TargetKinds, to be placed on the annotation definition itself?

I think it would make sense. It's better for users if we can surface this kind of diagnostic in the IDE rather than waiting until running a compiler.

It sounds like you'd currently need to add a target kind for static methods within classes, enums, mixins, extensions and extension types. In the future you might need one instance methods, but that's unknown at the moment. I think everything else is already covered, with one minor caveat.

Classes with a const constructor ...

I don't think we want to use TargetKind for that kind of check. It makes sense to use TargetKind.classType, but the restriction that the class must have at least one const constructor is, IMO, outside of how we want to use TargetKind and would need additional validation in the analyzer. That isn't hard to do, just something to keep in mind.

It is unclear if instance methods would be possible to track, as the use of dynamic prevents knowing if they are referenced at compile time.

Yes, dynamic is an issue, but probably for a very small percentage of users. It might be reasonable to support instance methods with a caveat that they don't work for dynamic targets, but I also don't know how useful it would be for users to have the ability to annotate instance methods.

@lrhn
Copy link
Member

lrhn commented Jun 12, 2024

Should we extend, or replace, the TargetKind annotation to be able to define more fine-grained predicates on declarations?
Say, being able to select only "static getters of classes" or "const constructors of extension types"?

@bwilkerson
Copy link
Member

Maybe. I considered that initially, but was expecting (hoping?) that we wouldn't need that kind of complexity. The plan was to introduce subsets of categories when the finer granularity was needed. For example, if TargetKind.method is too broad, we could add TargetKind.staticMethod. It's possible that we are reaching a point where that isn't sufficient.

I'm reluctant to try to make a fully general mechanism because of the complexity that comes with that, including the need to produce diagnostics when an invalid combination of modifiers is used.

@Markzipan Markzipan added the area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). label Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...).
Projects
None yet
Development

No branches or pull requests

4 participants