diff --git a/Cargo.lock b/Cargo.lock index 5946747d4f9..f69b69c3c90 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3489,7 +3489,7 @@ dependencies = [ "proc-macro2", "proc-macro2-diagnostics", "quote", - "syn 1.0.109", + "syn 2.0.63", ] [[package]] @@ -3629,7 +3629,7 @@ version = "3.103.0" dependencies = [ "proc-macro2", "quote", - "syn 1.0.109", + "syn 2.0.63", ] [[package]] @@ -3639,7 +3639,7 @@ dependencies = [ "proc-macro2", "proc-macro2-diagnostics", "semver 1.0.23", - "syn 1.0.109", + "syn 2.0.63", ] [[package]] diff --git a/changelog.d/1235.changed.md b/changelog.d/1235.changed.md new file mode 100644 index 00000000000..a6ca0f8b8eb --- /dev/null +++ b/changelog.d/1235.changed.md @@ -0,0 +1 @@ +Update syn to version 2. \ No newline at end of file diff --git a/mirrord/config/derive/Cargo.toml b/mirrord/config/derive/Cargo.toml index 7add6a42763..31afa578b1b 100644 --- a/mirrord/config/derive/Cargo.toml +++ b/mirrord/config/derive/Cargo.toml @@ -24,4 +24,4 @@ proc-macro = true proc-macro2 = "1" proc-macro2-diagnostics = "0.10" quote = "1" -syn = { version = "1", features = ["full", "extra-traits"] } +syn = { version = "2", features = ["full", "extra-traits"] } diff --git a/mirrord/config/derive/src/config/flag.rs b/mirrord/config/derive/src/config/flag.rs index f08dc27d760..e3f4d15b706 100644 --- a/mirrord/config/derive/src/config/flag.rs +++ b/mirrord/config/derive/src/config/flag.rs @@ -1,7 +1,10 @@ use proc_macro2::{Span, TokenStream}; use proc_macro2_diagnostics::{Diagnostic, SpanDiagnosticExt}; use quote::{quote, ToTokens}; -use syn::{spanned::Spanned, Attribute, Ident, Lit, Meta, NestedMeta}; +use syn::{ + punctuated::Punctuated, spanned::Spanned, Attribute, Expr, ExprLit, Ident, Lit, Meta, + MetaNameValue, Token, +}; #[derive(Debug, Eq, PartialEq)] pub enum ConfigFlagsType { @@ -17,8 +20,16 @@ pub enum ConfigFlagsType { pub struct ConfigFlags { pub doc: Vec, + /// Derive [`struct@Ident`]s that we want the config type to have. + /// + /// `#[config(map_to = "Bear", derive = "Metallic")` will produce a type + /// `#[derive(Metallic)] struct Bear {}` pub derive: Vec, pub generator: Option, + + /// Generates a mapped type that implements config stuff (?). + /// + /// `#[config(map_to = "Bear")` will produce a type `struct Bear {}` pub map_to: Option, pub default: Option, @@ -30,120 +41,147 @@ pub struct ConfigFlags { pub deprecated: Option, } +/// Retrieves the [`enum@Lit`] that is inside a [`MetaNameValue`]. +/// +/// The [`enum@Lit`] can be: +/// 1. The `CatFile` in `map_to = "CatFile"`; +/// 2. The `JsonSchema` in `derive = "JsonSchema"`; +/// 3. The `CatUserConfig` in `generator = "CatUserConfig"`; +fn lit_in_meta_name_value(meta: &MetaNameValue) -> Option { + if let MetaNameValue { + path: _, + eq_token: _, + value: Expr::Lit(ExprLit { lit, .. }), + } = meta + { + Some(lit.clone()) + } else { + None + } +} + impl ConfigFlags { + /// Digs into the [`syn::MetaList`] of the list of [`Attribute`] to fill up our + ///[`ConfigFlags`]. pub fn new(attrs: &[Attribute], mode: ConfigFlagsType) -> Result { let mut flags = ConfigFlags { doc: attrs .iter() - .filter(|attr| attr.path.is_ident("doc")) + .filter(|attr| attr.path().is_ident("doc")) .cloned() .collect(), ..Default::default() }; - for meta in attrs + for meta_list in attrs .iter() - .filter(|attr| attr.path.is_ident("config")) - .filter_map(|attr| attr.parse_meta().ok()) + .filter(|attr| attr.path().is_ident("config")) + .filter_map(|attr| attr.meta.require_list().ok()) { - if let Meta::List(list) = meta { - for meta in list.nested { - match meta { - NestedMeta::Meta(Meta::Path(path)) - if mode == ConfigFlagsType::Field && path.is_ident("nested") => - { - flags.nested = true - } - NestedMeta::Meta(Meta::Path(path)) - if mode == ConfigFlagsType::Field && path.is_ident("toggleable") => - { - flags.toggleable = true - } - NestedMeta::Meta(Meta::Path(path)) - if mode == ConfigFlagsType::Field && path.is_ident("unstable") => - { - flags.unstable = true - } - NestedMeta::Meta(Meta::NameValue(meta)) - if mode == ConfigFlagsType::Field && meta.path.is_ident("env") => - { - flags.env = Some(EnvFlag(meta.lit)) - } - NestedMeta::Meta(Meta::Path(path)) - if mode == ConfigFlagsType::Field && path.is_ident("default") => - { - flags.default = Some(DefaultFlag::Flag) - } - NestedMeta::Meta(Meta::NameValue(meta)) - if mode == ConfigFlagsType::Field && meta.path.is_ident("default") => - { - flags.default = Some(DefaultFlag::Value(meta.lit)) - } - NestedMeta::Meta(Meta::NameValue(meta)) - if mode == ConfigFlagsType::Field && meta.path.is_ident("rename") => - { - flags.rename = Some(meta.lit) - } - NestedMeta::Meta(Meta::NameValue(meta)) - if mode == ConfigFlagsType::Field - && meta.path.is_ident("deprecated") => - { - flags.deprecated = Some(meta.lit) - } - NestedMeta::Meta(Meta::NameValue(meta)) - if mode == ConfigFlagsType::Container - && meta.path.is_ident("map_to") => - { - match meta.lit { - Lit::Str(val) => { - flags.map_to = Some(Ident::new(&val.value(), Span::call_site())) - } - _ => { - return Err(meta - .lit - .span() - .error("map_to should be a string literal as value")) - } + // The more flexible way of parsing nested `Meta` in a `MetaList` is to use + // `parse_nested_meta`, but it's also more troublesome, as it parses the meta + // attribute in a weird way for our usecase. + // Consider something like + // `#[config(map_to = "CatFile", derive = "JsonSchema")]`, it would parse this + // as `Path {map_to, CatFile}` and the rest of the input would be + // `, derive = "JsonSchema"`, which is a bigger hassle to parse. + // + // Also, error handling becomes more problematic with `parse_nested_meta`. + let nested = + meta_list.parse_args_with(Punctuated::::parse_terminated)?; + + // Let's look into the nested `Meta` to possibly set multiple flags. + for meta in nested { + match meta { + Meta::Path(path) + if mode == ConfigFlagsType::Field && path.is_ident("nested") => + { + flags.nested = true; + } + Meta::Path(path) + if mode == ConfigFlagsType::Field && path.is_ident("toggleable") => + { + flags.toggleable = true; + } + Meta::Path(path) + if mode == ConfigFlagsType::Field && path.is_ident("unstable") => + { + flags.unstable = true; + } + Meta::NameValue(meta) + if mode == ConfigFlagsType::Field && meta.path.is_ident("env") => + { + flags.env = lit_in_meta_name_value(&meta).map(EnvFlag); + } + Meta::Path(path) + if mode == ConfigFlagsType::Field && path.is_ident("default") => + { + flags.default = Some(DefaultFlag::Flag); + } + Meta::NameValue(meta) + if mode == ConfigFlagsType::Field && meta.path.is_ident("default") => + { + flags.default = lit_in_meta_name_value(&meta).map(DefaultFlag::Value); + } + Meta::NameValue(meta) + if mode == ConfigFlagsType::Field && meta.path.is_ident("rename") => + { + flags.rename = lit_in_meta_name_value(&meta); + } + Meta::NameValue(meta) + if mode == ConfigFlagsType::Field && meta.path.is_ident("deprecated") => + { + flags.deprecated = lit_in_meta_name_value(&meta); + } + Meta::NameValue(meta) + if mode == ConfigFlagsType::Container && meta.path.is_ident("map_to") => + { + lit_in_meta_name_value(&meta).map(|lit| match lit { + Lit::Str(val) => { + flags.map_to = Some(Ident::new(&val.value(), Span::call_site())); + Ok(()) } - } - NestedMeta::Meta(Meta::NameValue(meta)) - if mode == ConfigFlagsType::Container - && meta.path.is_ident("derive") => - { - match meta.lit { - Lit::Str(val) => { - flags.derive.extend( - val.value() - .split(',') - .map(|part| Ident::new(part.trim(), Span::call_site())), - ); - } - _ => { - return Err(meta - .lit - .span() - .error("derive should be a string literal as value")) - } + _ => Err(lit + .span() + .error("map_to should be a string literal as value")), + }); + } + Meta::NameValue(meta) + if mode == ConfigFlagsType::Container && meta.path.is_ident("derive") => + { + lit_in_meta_name_value(&meta).map(|lit| match lit { + Lit::Str(val) => { + flags.derive.extend( + val.value() + .split(',') + .map(|part| Ident::new(part.trim(), Span::call_site())), + ); + Ok(()) } - } - NestedMeta::Meta(Meta::NameValue(meta)) - if mode == ConfigFlagsType::Container - && meta.path.is_ident("generator") => - { - match meta.lit { - Lit::Str(val) => { - flags.generator = - Some(Ident::new(&val.value(), Span::call_site())) - } - _ => { - return Err(meta - .lit - .span() - .error("derive should be a string literal as value")) - } + _ => Err(lit + .span() + .error("derive should be a string literal as value")), + }); + } + Meta::NameValue(meta) + if mode == ConfigFlagsType::Container + && meta.path.is_ident("generator") => + { + lit_in_meta_name_value(&meta).map(|lit| match lit { + Lit::Str(val) => { + flags.generator = Some(Ident::new(&val.value(), Span::call_site())); + Ok(()) } - } - _ => return Err(meta.span().error("unsupported config attribute flag")), + _ => Err(lit + .span() + .error("derive should be a string literal as value")), + }); + } + _ => { + return Err(meta.path().span().error( + "unsupported config + attribute flag", + )) } } } diff --git a/mirrord/layer/macro/Cargo.toml b/mirrord/layer/macro/Cargo.toml index 04fc2dbc2e4..66f4750804b 100644 --- a/mirrord/layer/macro/Cargo.toml +++ b/mirrord/layer/macro/Cargo.toml @@ -22,4 +22,4 @@ proc-macro = true [dependencies] proc-macro2 = "1" quote = "1" -syn = { version = "1", features = ["full"] } +syn = { version = "2", features = ["full"] } diff --git a/mirrord/layer/macro/src/lib.rs b/mirrord/layer/macro/src/lib.rs index 8f88202762c..205001cd391 100644 --- a/mirrord/layer/macro/src/lib.rs +++ b/mirrord/layer/macro/src/lib.rs @@ -2,7 +2,7 @@ use proc_macro2::Span; use quote::quote; -use syn::{parse::Parser, punctuated::Punctuated, token::Comma, Block, Ident, ItemFn}; +use syn::{parse::Parser, punctuated::Punctuated, token::Comma, Block, Ident, ItemFn, Type}; /// `#[hook_fn]` annotates the C ffi functions (mirrord's `_detour`s), and is used to generate the /// following boilerplate (using `close_detour` as an example): @@ -49,7 +49,8 @@ pub fn hook_fn( let unsafety = signature.unsafety; let abi = signature.abi; - let fn_args = signature + // Function arguments without taking into account variadics! + let mut fn_args = signature .inputs .into_iter() .map(|fn_arg| match fn_arg { @@ -58,6 +59,16 @@ pub fn hook_fn( }) .collect::>(); + // If we have `VaListImpl` args, then we push it to the end of the `fn_args` as + // just `...`. + if signature.variadic.is_some() { + let fixed_arg = quote! { + ... + }; + + fn_args.push(Box::new(Type::Verbatim(fixed_arg))); + } + let return_type = signature.output; // `unsafe extern "C" fn(i32) -> i32` @@ -124,7 +135,7 @@ pub fn hook_guard_fn( let unsafety = signature.unsafety; let abi = signature.abi; - let fn_args = signature + let mut fn_args = signature .inputs .clone() .into_iter() @@ -134,6 +145,16 @@ pub fn hook_guard_fn( }) .collect::>(); + // If we have `VaListImpl` args, then we push it to the end of the `fn_args` as + // just `...`. + if signature.variadic.is_some() { + let fixed_arg = quote! { + ... + }; + + fn_args.push(Box::new(Type::Verbatim(fixed_arg))); + } + let fn_arg_names: Punctuated<_, Comma> = signature .inputs .into_iter() diff --git a/mirrord/macros/Cargo.toml b/mirrord/macros/Cargo.toml index ee4200fac53..618ecd7ed25 100644 --- a/mirrord/macros/Cargo.toml +++ b/mirrord/macros/Cargo.toml @@ -22,5 +22,5 @@ proc-macro = true [dependencies] proc-macro2 = "1" proc-macro2-diagnostics = "0.10" -syn = { version = "1", features = ["full", "extra-traits"] } -semver.workspace = true \ No newline at end of file +syn = { version = "2", features = ["full", "extra-traits"] } +semver.workspace = true