Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[codegen] Make #[available] not accept expressions #180

Merged
merged 1 commit into from
Dec 15, 2022
Merged

Conversation

cmyr
Copy link
Member

@cmyr cmyr commented Dec 15, 2022

Further work on making us schema-ready. Previously this attribute accepted any expression that evaluated to something which implemented the 'Comparable' trait, for the appropriate version type. In practice, this was so that we could provide both literal integers as well as named constants, such as MajorMinor::VERSION_1_1.

The schema will not support the concept of an arbitrary expression, however, and so this needs a more constrained representation.

With this patch, we no longer support the use of named constants in this attribute; instead the argument must be either a single integer literal, or a comma-separated major/minor pair of literal integers, which we will convert to the appropriate type when comparing.

Further work on making us schema-ready. Previously this attribute
accepted any expression that evaluated to something which implemented
the 'Comparable' trait, for the appropriate version type. In practice,
this was so that we could provide both literal integers as well as named
constants, such as `MajorMinor::VERSION_1_1`.

The schema will not support the concept of an arbitrary expression,
however, and so this needs a more constrained representation.

With this patch, we no longer support the use of named constants in this
attribute; instead the argument must be either a single integer literal,
or a comma-separated major/minor pair of literal integers, which we will
convert to the appropriate type when comparing.
@@ -235,6 +235,12 @@ pub(crate) struct FieldReadArgs {
pub(crate) inputs: Vec<syn::Ident>,
}

#[derive(Clone, Debug)]
pub(crate) struct Available {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I didn't find the meaning of available intuitive. Maybe it's worth the cost of a long name? - for example available_from_version(2) I would immediately get

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

happy to bikeshed that name, I agree it isn't great. If we agree on a change will do a separate PR. :)

@cmyr cmyr merged commit 4ee01be into main Dec 15, 2022
@cmyr cmyr deleted the available-tuple branch December 15, 2022 20:23
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants