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

Exploration: supported languages as "singleton values": macros #53

Merged
merged 8 commits into from
Jul 13, 2023

Conversation

helixbass
Copy link
Owner

In this PR:

To test:
You should see the fixed_map!() macro fail if you try to omit any keys, pass any spurious key/values or not pass corresponding values of the expected type
You should see the by_supported_language!() macro fail if you omit any supported languages or pass any unknown keys

Based on supported-languages-fixed-size

proc-macro = true

[dependencies]
Inflector = "0.11.4"
Copy link
Owner Author

Choose a reason for hiding this comment

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

A capitalized crate name?

Almost seems ironic since what this crate is doing is case conversion

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ha, maybe it's a weird joke.

let len = variants.len();
quote! {
#[derive(Default)]
pub struct #collection_type_name<T>([T; #len]);
Copy link
Owner Author

Choose a reason for hiding this comment

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

Per #52 (comment), add a newtype wrapper around the fixed-size collection type (eg BySupportedLanguage) for the purpose of exposing "HashMap-ish" iteration methods

I actually made the changes corresponding to those comments "statically" before starting the macro stuff so if you'd rather see a pre-macroization version of them you could look at the diff as of f672cca

}
}

fn get_iter_implementations(
Copy link
Owner Author

Choose a reason for hiding this comment

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

Per above comment this is implementing "HashMap-style" .iter()/.values() on eg BySupportedLanguage

) -> proc_macro2::TokenStream {
let len = variants.len();
quote! {
impl<T> Deref for #collection_type_name<T> {
Copy link
Owner Author

Choose a reason for hiding this comment

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

Added a Deref implementation along with the newtype wrapper (I think it's currently only getting used to call the underlying .len())

) -> proc_macro2::TokenStream {
let macro_name = format_ident!("{}", collection_type_name.to_string().to_snake_case());
quote! {
#[macro_export]
Copy link
Owner Author

Choose a reason for hiding this comment

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

Ok so this is sort of funny, the way that I was able to get a "generated" eg by_supported_language!() macro (that needed to be basically implemented as a proc macro, but per #52 (comment) I don't think you could easily just "generate and use a new proc macro"?) was to generate it as a declarative macro which basically delegates to an existing "static" by_fixed_map!() proc macro but has the things specific to "this fixed map" (ie the variant names and collection type name eg BySupportedLanguage) "pre-curried" as arguments to it (by hardcoding them into the declarative macro body basically)

It's sort of funny that I believe those pre-curried arguments just get passed along from one to the other "as token trees", so they can eg reference non-existing identifiers

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ha yes, that's kind of ridiculous, but cool that it works.

value_mapping_content.parse::<Token![=>]>()?;
let value: Expr = value_mapping_content.parse()?;
value_mapping.insert(key, value);
if value_mapping_content.is_empty() {
Copy link
Owner Author

Choose a reason for hiding this comment

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

On one of these macros I think I required a trailing comma but there are certainly ways to avoid that eg I copied this pattern from the syn::ExprArray Parse implementation

} = parse_macro_input!(input as ByFixedMapArgs);

if value_mapping.len() != variants.len() {
panic!("Incorrect variants");
Copy link
Owner Author

Choose a reason for hiding this comment

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

These could be more helpful and tell you eg which variant(s) were missing or what invalid variant names it encountered

]
});
impl SupportedLanguage {
pub fn language(&self) -> Language {
Copy link
Owner Author

Choose a reason for hiding this comment

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

Per #52 (comment), move these onto methods on SupportedLanguage

"lua",
];
static SUPPORTED_LANGUAGE_LANGUAGES: Lazy<BySupportedLanguage<Language>> = Lazy::new(|| {
by_supported_language!(
Copy link
Owner Author

Choose a reason for hiding this comment

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

I think I sort of like this approach of exposing a secondary macro for allowing you to create new "fixed-size collections" (as opposed to eg specifying all of the tree_sitter::Language's and "names for ignore select" in the body of the main fixed_map!() invocation and then either generating the same thing as this under the hood or possible generating "fat SupportedLanguage structs" (like how it used to have language/name_for_ignore_select as struct fields rather than these separate top-level data structures))?

I guess the downsides could be (a) in terms of code readability if one thought it were clearer to read "one big declaration" of all of the info/"fields" for all of the supported languages and (b) in terms of runtime if for some reason it were more performant ("cache locality"...?) (and if this were then actually a practical performance issue, I'm really just theorizing) to actually have those fields co-located as part of their relevant "SupportedLanguage struct" (vs being co-located with other instances of "that same field" like this)?

} else {
for name_for_ignore_select in SUPPORTED_LANGUAGE_NAMES_FOR_IGNORE_SELECT.iter() {
Copy link
Owner Author

Choose a reason for hiding this comment

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

The top-level SUPPORTED_LANGUAGE_LANGUAGES/SUPPORTED_LANGUAGE_NAMES_FOR_IGNORE_SELECT can really be considered "private"/"implementation details" (and I did in fact make them not pub) since you can achieve eg here iterating over them by iterating over the main ALL_SUPPORTED_LANGUAGES and then accessing the corresponding relevant "field" via the helper methods (eg here .name_for_ignore_select())

@helixbass helixbass mentioned this pull request Jul 12, 2023
proc-macro = true

[dependencies]
Inflector = "0.11.4"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ha, maybe it's a weird joke.

"name" => {
name = Some(input.parse::<Ident>()?);
}
"variants" => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Whoa, interesting. I guess I haven't seen this kind of stuff in a proc-macro before.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ya I was sort of "winging it" off of picturing the macro! { key => value, other_key => other_value } "invocation style"

let collection_type_name = format_ident!("By{name}");
let all_variants_collection_name = format_ident!(
"ALL_{}",
name.to_string().to_plural().to_screaming_snake_case()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I love the name "screaming snake case", and it seems like most people haven't even heard of it 😄

next_index: usize,
}

impl<'collection, T> #iter_struct_name<'collection, T> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I never really thing about how Iterator is implemented on structs that exist just for that purpose.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Right I haven't read that many implementations of them (eg of whatever the types are returned by Vec::into_iter() etc) but I think they just tend to be little "state machines"?

) -> proc_macro2::TokenStream {
let macro_name = format_ident!("{}", collection_type_name.to_string().to_snake_case());
quote! {
#[macro_export]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ha yes, that's kind of ridiculous, but cool that it works.

let key: Ident = value_mapping_content.parse()?;
value_mapping_content.parse::<Token![=>]>()?;
let value: Expr = value_mapping_content.parse()?;
value_mapping.insert(key, value);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Worth checking that it's not already present? (Unless that's handled elsewhere.)

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ah ya that makes sense, updated

TreeSitterQuery => tree_sitter_query::language(),
Json => tree_sitter_json::language(),
Css => tree_sitter_css::language(),
Lua => tree_sitter_lua::language(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

How would you feature-gate these (if you wanted to eventually), since you can't use the normal annotation in a macro. (Although I suppose you could make the macro understand that annotation?)

Copy link
Owner Author

Choose a reason for hiding this comment

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

Although I suppose you could make the macro understand that annotation?

Ya per I think a comment on a subsequent PR (where I did cpp -> c++), if I parse into a baked-in syn type eg syn::ExprArray it parses annotations for me

But... this isn't real Rust syntax so couldn't use that directly

Maybe that would be an excuse to mimic how eg syn implements .parse::<ExprArray>() but have the "item" parsing be different or whatever

Base automatically changed from supported-languages-fixed-size to master July 13, 2023 21:23
@helixbass helixbass merged commit 7873e30 into master Jul 13, 2023
2 checks passed
@helixbass helixbass deleted the supported-languages-fixed-size-macro branch July 13, 2023 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants