-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[resource_identifiers] Rename to recorded_uses. #55808
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
Comments
Let's see if I understand this correctly ... The annotation marks an It works by the annotation having some metadata which is JSON-encoded and written to a some JSON file (before or after tree-shaking) where the build/link scripts can get the metadata and use it to build the requested external data. Probably includes the desired format too. The naming of annotations is not as standardized as most other things. Yet? So If I haven't understood things correctly, then ... something else! |
Sorry for scattering information all around @lrhn! We need some way of allowing users to tree-shake whole assets or parts of assets in link hooks. For the simplest cases we can omit the whole What we're trying to solve with "resource identifiers" (to be renamed) is more involved. The For example for JNIgen when trying to implement proguard rules (#55568). class JClassIdentifier implements /* to be renamed */ ResourceIdentifier {
final String originalJavaClassName;
const JClassIdentifier({
required this.originalJavaClassName;
});
}
@JClassIdentifier({
originalJavaClassName: "FooBar",
})
late final JClass foobar; The link hook would then take such info and ensure the
Correct, that's why I'm filing issues. I'm offloading a bunch of whiteboard discussions to GitHub issues to do some extra rounds of bikeshed.. err naming.
Well, the annotated declaration uses an asset or a part of an asset at runtime. But the mapping from the const argument values and metadata in the annotation to asset id and asset part id happens in the link hook. The example above is a verbatim mapping from metadata to asset part id, but that is not always the case. For example an argument So naming the annotation Hence, I'm leaning more towards gearing the annotation name to the fact that the calls to this static function are recorded after treeshaking. E.g. I think this shows that maybe we should have
|
Ok, so the meaning of the annotation is that someone wants to know whether the annotated (static only? Function only?) declaration may be used in the program, and if so, how (the arguments passed at the user point if it's a function, and the use is a call, and only if the argument is a constant that can be JSON encoded) based on only the uses left after tree-shaking. The compiler records the invocation of the annotated declaration, at least if it's a get or call. (Should probably support constructors, but if it only supports static functions, one can always forward. Does it record type arguments? Does it work with extension and extension type methods, which are not static declarations, but are resolved statically?) What someone uses the information for is up to them, this is a general feature that can be used for anything a user can come up with and make a tool for. The annotation can carry extra information, which is JSON encoded and stored with the use record. Again that can be completely general, and the consumer gets to decide what it means. If that's correct, then |
+1 for |
Thank you @lrhn, you make everything better! Naming proposal:
In the future we might want to add recording uses of other things (for example an annotation on a class, meaning that if any if the class survives tree shaking). But that still is valid to talk about recorded uses.
No, we don't have info about things that are gone after tree shaking. Maybe we should. (Implementation: We should visit the tree before and after tree-shaking.) This would enable link hook writers to write their hooks as throwing things out instead of retaining things. I'm not sure how important this is though.
No, we could if they are const on the call site, identified by the library uri and class name. We can add this later if we find a use case.
👍 We are on the same page! Extensions yes:
Extension types, most probably not. |
cc @rakudrama |
After some discussion with @mosuem we feel like resource identifier is not a great name. Resource is kind of a synonym for asset.
Some options:
Naming linked to the language constructs:(We might want other treeshaking info than just calls to static methods)@RecordReference(Object? metadata)
recorded_references.jsonpackage:record_references which lives in the SDK in pkg/record_references and is imported by dart2js and pkg/vm and published unlistedgen_kernel --record_references=recorded_references.json ...
@RecordReference(Object? metadata)
gen_kernel --record_treeshaking_info=treeshaking_info.json ...
Naming linked to assets:(The link hook take the tree shaking info and know what assets it is related to.)@AssetReference(String? assetId, Object? metadata)
asset_references.jsonPackage:asset_references (in the SDK, and unlisted)The downside of the last option is that it is linked to the assets concept but that the compiler itself is just recording references so that's a bit weird. With it being possible to implement
RecordReference
(#55568), thenAssetReference
can be a subtype ofRecordReference
.With all cases
package:native_assets_cli
would not expose the types of the package but extension types around the package, because we'd only expose a subset of all the fields of the resource indentifiers.The text was updated successfully, but these errors were encountered: