Targeted Attribute Injection - #1656
Conversation
bca5742 to
73c9e78
Compare
|
I know I'm taking a bit of time to get to this, sorry about that. This hasn't fallen off my radar, just haven't gotten the time to review it yet |
alexcrichton
left a comment
There was a problem hiding this comment.
Thanks for the PR again, and apologies for the delay. This all seems reasonable to me but I'd primarily like to integrate the string-selector style syntax with the rest of the macro (as opposed to building something brand new). Could it also be integrated with the unused-attributes warning/error that wit-bindgen emits to catch typos?
|
|
||
| /// Every selector key that matches type `id`: its identity keys plus its | ||
| /// owning interface and package. | ||
| fn type_selector_keys(&self, id: TypeId) -> Vec<String> { |
There was a problem hiding this comment.
I'd like to ideally have a relatively central method of specifying strings which identify/map to WIT types. To that extent this is at least somewhat present as full_wit_type_name at the root of the crate and how it affects with. Can that be integrated and/or updated to handle this new use case as well? I woudl prefer to not add another method of identifying types/etc with strings.
There was a problem hiding this comment.
Changed this to a thin wit_type_selectors wrapper around full_wit_type_name, which it calls rather than reimplements, so with is untouched. Pushing the coarser forms into with doesn't seem right though: a remapping is 1:1, so a package key would have to map every type in the package to the same Rust path, whereas attributes fan out by design.
…itional_member_attributes
73c9e78 to
acab6ef
Compare
acab6ef to
3f4ae8f
Compare
|
@alexcrichton for the unused-attributes warning I believe I've added that here: https://github.com/bytecodealliance/wit-bindgen/pull/1656/changes#diff-e256bf47e7fd893d97fe738d2d6b4a47ed74ad649bc94a403398a4ea509ca38bR1620 but I can fold it into a single diagnostic here: https://github.com/bytecodealliance/wit-bindgen/pull/1656/changes#diff-e256bf47e7fd893d97fe738d2d6b4a47ed74ad649bc94a403398a4ea509ca38bR1600 |
Targeted version of
additional_derives. Instead of hitting every generated type,you attach attributes to specific types and members. This is an attempt at the path-specific /
with-style derive idea from #1386, #1467, and #812 (also #291, #554).Two options:
additional_type_attributesfor records/variants/enums, andadditional_member_attributesfor fields and enum/variant cases. Each maps a selectorto a list of attributes, and the selectors are the same keys
withuses (bare name,package, interface, or fully-qualified, with
@versionwhen versioned):Any attribute works, not just derives. An injected
#[derive(...)]folds into thetype's generated derive so it dedups against the built-ins and
additional_derivesinstead of colliding.
Went with two options rather than one nested map to match prost's
type_attribute/field_attribute, keep the CLI flags simple, and avoid a bare namebeing ambiguous between a type and a member. A selector that matches nothing is an
error, same as
withwith unused keys.Some limits: only records/variants/enums (broad selectors just skip
flags/resources/aliases), attributes land on every ownership form so an owned-only
derive like
Deserializewon't compile on a borrowed form, and it's Rust-only fornow following #678/#1199.
Not sure on the naming.
additional_*sits next toadditional_derives, but it actsmore like
with. Happy to switch totype_attributes/member_attributesor whateveryou prefer.
Tests in
crates/rust/tests/codegen.rs.