From 6dae9e9d455ba5e3bf18c452789d07ff6cfaf392 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sat, 26 Aug 2023 22:11:55 +1000 Subject: [PATCH 01/29] Add new crate `gix-macros` proc-macro crates, it provides `momo` for de-monomorphization for now. Signed-off-by: Jiahao XU --- Cargo.lock | 9 + Cargo.toml | 1 + gix-macros/Cargo.toml | 22 ++ gix-macros/LICENSE-APACHE | 1 + gix-macros/LICENSE-MIT | 1 + gix-macros/src/lib.rs | 450 ++++++++++++++++++++++++++++++++++++++ gix-macros/tests/test.rs | 286 ++++++++++++++++++++++++ 7 files changed, 770 insertions(+) create mode 100644 gix-macros/Cargo.toml create mode 120000 gix-macros/LICENSE-APACHE create mode 120000 gix-macros/LICENSE-MIT create mode 100644 gix-macros/src/lib.rs create mode 100644 gix-macros/tests/test.rs diff --git a/Cargo.lock b/Cargo.lock index a06040137e0..441417f4cdb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1795,6 +1795,15 @@ dependencies = [ "thiserror", ] +[[package]] +name = "gix-macros" +version = "0.0.0" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.29", +] + [[package]] name = "gix-mailmap" version = "0.17.0" diff --git a/Cargo.toml b/Cargo.toml index c79cf19e1d6..ec5728c3551 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -244,6 +244,7 @@ members = [ "gix-packetline", "gix-packetline-blocking", "gix-mailmap", + "gix-macros", "gix-note", "gix-negotiate", "gix-fetchhead", diff --git a/gix-macros/Cargo.toml b/gix-macros/Cargo.toml new file mode 100644 index 00000000000..d208d929c1f --- /dev/null +++ b/gix-macros/Cargo.toml @@ -0,0 +1,22 @@ +[package] +name = "gix-macros" +version = "0.0.0" +edition = "2021" +description = "Proc-macro utilities for gix" +authors = [ + "Jiahao XU ", + "Andre Bogus ", + "Sebastian Thiel ", +] +repository = "https://github.com/Byron/gitoxide" +license = "MIT OR Apache-2.0" +include = ["src/**/*", "LICENSE-*", "CHANGELOG.md"] +rust-version = "1.65" + +[lib] +proc_macro = true + +[dependencies] +syn = { version = "2.0", features = ["full", "fold"] } +quote = "1.0" +proc-macro2 = "1.0" diff --git a/gix-macros/LICENSE-APACHE b/gix-macros/LICENSE-APACHE new file mode 120000 index 00000000000..965b606f331 --- /dev/null +++ b/gix-macros/LICENSE-APACHE @@ -0,0 +1 @@ +../LICENSE-APACHE \ No newline at end of file diff --git a/gix-macros/LICENSE-MIT b/gix-macros/LICENSE-MIT new file mode 120000 index 00000000000..76219eb72e8 --- /dev/null +++ b/gix-macros/LICENSE-MIT @@ -0,0 +1 @@ +../LICENSE-MIT \ No newline at end of file diff --git a/gix-macros/src/lib.rs b/gix-macros/src/lib.rs new file mode 100644 index 00000000000..8688ccabaf1 --- /dev/null +++ b/gix-macros/src/lib.rs @@ -0,0 +1,450 @@ +use std::collections::{HashMap, HashSet}; + +use proc_macro::TokenStream; +use quote::quote; +use syn::{fold::Fold, punctuated::Punctuated, spanned::Spanned, *}; + +#[derive(Copy, Clone)] +// All conversions we support. Check references to this type for an idea how to add more. +enum Conversion<'a> { + Into(&'a Type), + TryInto(&'a Type), + AsRef(&'a Type), + AsMut(&'a Type), +} + +impl<'a> Conversion<'a> { + fn target_type(&self) -> Type { + match *self { + Conversion::Into(ty) => ty.clone(), + Conversion::TryInto(ty) => ty.clone(), + Conversion::AsRef(ty) => parse_quote!(&#ty), + Conversion::AsMut(ty) => parse_quote!(&mut #ty), + } + } + + fn conversion_expr(&self, i: Ident) -> Expr { + match *self { + Conversion::Into(_) => parse_quote!(#i.into()), + Conversion::TryInto(_) => parse_quote!(#i.try_into()?), + Conversion::AsRef(_) => parse_quote!(#i.as_ref()), + Conversion::AsMut(_) => parse_quote!(#i.as_mut()), + } + } +} + +fn parse_bounded_type(ty: &Type) -> Option { + if let Type::Path(TypePath { qself: None, ref path }) = ty { + if path.segments.len() == 1 { + return Some(path.segments[0].ident.clone()); + } + } + None +} + +fn parse_bounds(bounds: &Punctuated) -> Option { + if bounds.len() != 1 { + return None; + } + if let TypeParamBound::Trait(ref tb) = bounds.first().unwrap() { + if let Some(seg) = tb.path.segments.iter().last() { + if let PathArguments::AngleBracketed(ref gen_args) = seg.arguments { + if let GenericArgument::Type(ref arg_ty) = gen_args.args.first().unwrap() { + if seg.ident == "Into" { + return Some(Conversion::Into(arg_ty)); + } else if seg.ident == "TryInto" { + return Some(Conversion::TryInto(arg_ty)); + } else if seg.ident == "AsRef" { + return Some(Conversion::AsRef(arg_ty)); + } else if seg.ident == "AsMut" { + return Some(Conversion::AsMut(arg_ty)); + } + } + } + } + } + None +} + +// create a map from generic type to Conversion +fn parse_generics(decl: &Signature) -> (HashMap>, Generics) { + let mut ty_conversions = HashMap::new(); + let mut params = Punctuated::new(); + for gp in decl.generics.params.iter() { + if let GenericParam::Type(ref tp) = gp { + if let Some(conversion) = parse_bounds(&tp.bounds) { + ty_conversions.insert(tp.ident.clone(), conversion); + continue; + } + } + params.push(gp.clone()); + } + let where_clause = if let Some(ref wc) = decl.generics.where_clause { + let mut idents_to_remove = HashSet::new(); + let mut predicates = Punctuated::new(); + for wp in wc.predicates.iter() { + if let WherePredicate::Type(ref pt) = wp { + if let Some(ident) = parse_bounded_type(&pt.bounded_ty) { + if let Some(conversion) = parse_bounds(&pt.bounds) { + idents_to_remove.insert(ident.clone()); + ty_conversions.insert(ident, conversion); + continue; + } + } + } + predicates.push(wp.clone()); + } + params = params + .into_iter() + .filter(|param| { + if let GenericParam::Type(type_param) = param { + !idents_to_remove.contains(&type_param.ident) + } else { + true + } + }) + .collect(); + Some(WhereClause { + predicates, + ..wc.clone() + }) + } else { + None + }; + ( + ty_conversions, + Generics { + params, + where_clause, + ..decl.generics.clone() + }, + ) +} + +fn pat_to_ident(pat: &Pat) -> Ident { + if let Pat::Ident(ref pat_ident) = *pat { + return pat_ident.ident.clone(); + } + unimplemented!("No non-ident patterns for now!"); +} + +fn pat_to_expr(pat: &Pat) -> Expr { + let ident = pat_to_ident(pat); + parse_quote!(#ident) +} + +fn convert<'a>( + inputs: &'a Punctuated, + ty_conversions: &HashMap>, +) -> ( + Punctuated, + Conversions, + Punctuated, + bool, +) { + let mut argtypes = Punctuated::new(); + let mut conversions = Conversions { + intos: Vec::new(), + try_intos: Vec::new(), + as_refs: Vec::new(), + as_muts: Vec::new(), + }; + let mut argexprs = Punctuated::new(); + let mut has_self = false; + inputs.iter().for_each(|input| match input { + FnArg::Receiver(..) => { + has_self = true; + argtypes.push(input.clone()); + } + FnArg::Typed(PatType { + ref pat, + ref ty, + ref colon_token, + .. + }) => match **ty { + Type::ImplTrait(TypeImplTrait { ref bounds, .. }) => { + if let Some(conv) = parse_bounds(bounds) { + argtypes.push(FnArg::Typed(PatType { + attrs: Vec::new(), + pat: pat.clone(), + colon_token: *colon_token, + ty: Box::new(conv.target_type()), + })); + let ident = pat_to_ident(pat); + conversions.add(ident.clone(), conv); + argexprs.push(conv.conversion_expr(ident)); + } else { + argtypes.push(input.clone()); + argexprs.push(pat_to_expr(pat)); + } + } + Type::Path(..) => { + if let Some(conv) = parse_bounded_type(ty).and_then(|ident| ty_conversions.get(&ident)) { + argtypes.push(FnArg::Typed(PatType { + attrs: Vec::new(), + pat: pat.clone(), + colon_token: *colon_token, + ty: Box::new(conv.target_type()), + })); + let ident = pat_to_ident(pat); + conversions.add(ident.clone(), *conv); + argexprs.push(conv.conversion_expr(ident)); + } else { + argtypes.push(input.clone()); + argexprs.push(pat_to_expr(pat)); + } + } + _ => { + argtypes.push(input.clone()); + argexprs.push(pat_to_expr(pat)); + } + }, + }); + (argtypes, conversions, argexprs, has_self) +} + +struct Conversions { + intos: Vec, + try_intos: Vec, + as_refs: Vec, + as_muts: Vec, +} + +impl Conversions { + fn add(&mut self, ident: Ident, conv: Conversion) { + match conv { + Conversion::Into(_) => self.intos.push(ident), + Conversion::TryInto(_) => self.try_intos.push(ident), + Conversion::AsRef(_) => self.as_refs.push(ident), + Conversion::AsMut(_) => self.as_muts.push(ident), + } + } +} + +fn has_conversion(idents: &[Ident], expr: &Expr) -> bool { + if let Expr::Path(ExprPath { ref path, .. }) = *expr { + if path.segments.len() == 1 { + let seg = path.segments.iter().last().unwrap(); + return idents.iter().any(|i| i == &seg.ident); + } + } + false +} + +#[allow(clippy::collapsible_if)] +impl Fold for Conversions { + fn fold_expr(&mut self, expr: Expr) -> Expr { + //TODO: Also catch `Expr::Call` with suitable paths & args + match expr { + Expr::MethodCall(mc) if mc.args.is_empty() => match &*mc.method.to_string() { + "into" if has_conversion(&self.intos, &mc.receiver) => *mc.receiver, + "try_into" if has_conversion(&self.try_intos, &mc.receiver) => *mc.receiver, + + "as_ref" if has_conversion(&self.as_refs, &mc.receiver) => *mc.receiver, + "as_mut" if has_conversion(&self.as_muts, &mc.receiver) => *mc.receiver, + + _ => syn::fold::fold_expr(self, Expr::MethodCall(mc)), + }, + Expr::Call(call) if call.args.len() == 1 => match &*call.func { + Expr::Path(ExprPath { + path: Path { segments, .. }, + .. + }) if segments.len() == 2 => match (&*segments[0].ident.to_string(), &*segments[1].ident.to_string()) { + ("Into", "into") if has_conversion(&self.intos, &call.args[0]) => call.args[0].clone(), + ("TryInto", "try_into") if has_conversion(&self.try_intos, &call.args[0]) => call.args[0].clone(), + + ("AsRef", "as_ref") if matches!(&call.args[0], Expr::Reference(ExprReference { expr, mutability: None, .. }) if has_conversion(&self.as_refs, expr)) => { + if let Expr::Reference(ExprReference { expr, .. }) = &call.args[0] { + (**expr).clone() + } else { + panic!("expr must be Reference") + } + } + ("AsMut", "as_mut") if matches!(&call.args[0], Expr::Reference(ExprReference { expr, mutability: Some(_), .. }) if has_conversion(&self.as_muts, expr)) => { + if let Expr::Reference(ExprReference { expr, .. }) = &call.args[0] { + (**expr).clone() + } else { + panic!("expr must be Reference") + } + } + + _ => syn::fold::fold_expr(self, Expr::Call(call)), + }, + _ => syn::fold::fold_expr(self, Expr::Call(call)), + }, + Expr::Try(expr_try) => match &*expr_try.expr { + Expr::MethodCall(mc) + if mc.args.is_empty() + && mc.method == "try_into" + && has_conversion(&self.try_intos, &mc.receiver) => + { + (*mc.receiver).clone() + } + Expr::Call(call) if call.args.len() == 1 => match &*call.func { + Expr::Path(ExprPath { + path: Path { segments, .. }, + .. + }) if segments.len() == 2 + && segments[0].ident == "TryInto" + && segments[1].ident == "try_into" + && has_conversion(&self.try_intos, &call.args[0]) => + { + call.args[0].clone() + } + _ => syn::fold::fold_expr(self, Expr::Try(expr_try)), + }, + _ => syn::fold::fold_expr(self, Expr::Try(expr_try)), + }, + _ => syn::fold::fold_expr(self, expr), + } + } +} + +fn contains_self_type_path(path: &Path) -> bool { + path.segments.iter().any(|segment| { + segment.ident == "Self" + || match &segment.arguments { + PathArguments::AngleBracketed(AngleBracketedGenericArguments { args, .. }) => { + args.iter().any(|generic_arg| match generic_arg { + GenericArgument::Type(ty) => contains_self_type(ty), + GenericArgument::Const(expr) => contains_self_type_expr(expr), + _ => false, + }) + } + PathArguments::Parenthesized(ParenthesizedGenericArguments { inputs, output, .. }) => { + inputs.iter().any(contains_self_type) + || matches!(output, ReturnType::Type(_, ty) if contains_self_type(ty)) + } + _ => false, + } + }) +} + +fn contains_self_type_expr(expr: &Expr) -> bool { + match expr { + Expr::Path(ExprPath { qself: Some(_), .. }) => true, + Expr::Path(ExprPath { path, .. }) => contains_self_type_path(path), + _ => false, + } +} + +fn contains_self_type(input: &Type) -> bool { + match input { + Type::Array(TypeArray { elem, len, .. }) => { + // Call `matches!` first so that we can do tail call here + // as an optimization. + contains_self_type_expr(len) || contains_self_type(elem) + } + Type::Group(TypeGroup { elem, .. }) => contains_self_type(elem), + Type::Paren(TypeParen { elem, .. }) => contains_self_type(elem), + Type::Ptr(TypePtr { elem, .. }) => contains_self_type(elem), + Type::Reference(TypeReference { elem, .. }) => contains_self_type(elem), + Type::Slice(TypeSlice { elem, .. }) => contains_self_type(elem), + + Type::Tuple(TypeTuple { elems, .. }) => elems.iter().any(contains_self_type), + + Type::Path(TypePath { qself: Some(_), .. }) => true, + Type::Path(TypePath { path, .. }) => contains_self_type_path(path), + + _ => false, + } +} + +fn has_self_type(input: &FnArg) -> bool { + match input { + FnArg::Receiver(_) => true, + FnArg::Typed(PatType { ty, .. }) => contains_self_type(ty), + } +} + +/// Generate lightweight monomorphized wrapper around main implementation. +/// May be applied to functions and methods. +#[proc_macro_attribute] +pub fn momo(_attrs: TokenStream, input: TokenStream) -> TokenStream { + //TODO: alternatively parse ImplItem::Method + momo_inner(input.into()).into() +} + +fn momo_inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream { + let fn_item: Item = match syn::parse2(code.clone()) { + Ok(input) => input, + Err(err) => return err.to_compile_error(), + }; + + if let Item::Fn(item_fn) = fn_item { + let (ty_conversions, generics) = parse_generics(&item_fn.sig); + let (argtypes, mut conversions, argexprs, has_self) = convert(&item_fn.sig.inputs, &ty_conversions); + + let uses_self = has_self + || item_fn.sig.inputs.iter().any(has_self_type) + || matches!(&item_fn.sig.output, ReturnType::Type(_, ty) if contains_self_type(ty)); + + let inner_ident = Ident::new( + // Use long qualifier to avoid name colision. + &format!("_{}_inner_generated_by_gix_macro_momo", item_fn.sig.ident), + proc_macro2::Span::call_site(), + ); + + let new_inner_item = Item::Fn(ItemFn { + // Remove doc comment since they will increase compile-time and + // also generates duplicate warning/error messages for the doc, + // especially if it contains doc-tests. + attrs: { + let mut attrs = item_fn.attrs.clone(); + attrs.retain(|attr| { + let segments = &attr.path().segments; + !(segments.len() == 1 && segments[0].ident == "doc") + }); + attrs + }, + vis: Visibility::Inherited, + sig: Signature { + ident: inner_ident.clone(), + generics, + inputs: argtypes, + ..item_fn.sig.clone() + }, + block: Box::new(conversions.fold_block(*item_fn.block)), + }); + + if uses_self { + // We can use `self` or `Self` inside function defined within + // the impl-fn, so instead declare two separate functions. + // + // Since it's an impl block, it's unlikely to have name conflict, + // though this won't work for impl-trait. + // + // This approach also make sure we can call the right function + // using `Self` qualifier. + let new_item = Item::Fn(ItemFn { + attrs: item_fn.attrs, + vis: item_fn.vis, + sig: item_fn.sig, + block: if has_self { + parse_quote!({ self.#inner_ident(#argexprs) }) + } else { + parse_quote!({ Self::#inner_ident(#argexprs) }) + }, + }); + quote!(#new_item #[allow(unused_mut)] #new_inner_item) + } else { + // Put the new inner function within the function block + // to avoid duplicate function name and support associated + // function that doesn't use `self` or `Self`. + let new_item = Item::Fn(ItemFn { + attrs: item_fn.attrs, + vis: item_fn.vis, + sig: item_fn.sig, + block: parse_quote!({ + #[allow(unused_mut)] + #new_inner_item + + #inner_ident(#argexprs) + }), + }); + quote!(#new_item) + } + } else { + Error::new(fn_item.span(), "expect a function").to_compile_error() + } +} diff --git a/gix-macros/tests/test.rs b/gix-macros/tests/test.rs new file mode 100644 index 00000000000..10006bf1be4 --- /dev/null +++ b/gix-macros/tests/test.rs @@ -0,0 +1,286 @@ +use gix_macros::momo; +use std::pin::Pin; + +struct Options; + +#[allow(dead_code)] +fn test_open_opts_inner(_dir: impl Into, _options: Options) -> Result<(), ()> { + Ok(()) +} + +/// See if doc are kept +#[allow(dead_code)] +#[momo] +fn test_open_opts(directory: impl Into, options: Options) -> Result<(), ()> { + test_open_opts_inner(directory, options) +} + +#[allow(dead_code)] +#[momo] +fn test_with_try(t: impl TryInto, _options: Options) -> Result<(), ()> { + let t = t.try_into()?; + t.strip_prefix('1').ok_or(())?; + Ok(()) +} + +#[momo] +fn test_fn( + a: impl Into, + b: impl AsRef, + mut c: impl AsMut, + d: impl TryInto, +) -> Result { + let mut s = a.into(); + s += b.as_ref(); + s += c.as_mut(); + s += &d.try_into()?; + + Ok(s) +} + +#[momo] +fn test_fn_call_style( + a: impl Into, + b: impl AsRef, + mut c: impl AsMut, + d: impl TryInto, +) -> Result { + let mut s = Into::into(a); + s += AsRef::as_ref(&b); + s += AsMut::as_mut(&mut c); + s += &TryInto::try_into(d)?; + + Ok(s) +} + +#[momo] +fn test_fn_where(a: A, b: B, mut c: C, d: D) -> Result +where + A: Into, + B: AsRef, + C: AsMut, + D: TryInto, +{ + let mut s = a.into(); + s += b.as_ref(); + s += c.as_mut(); + s += &d.try_into()?; + + Ok(s) +} + +struct TestStruct; + +impl TestStruct { + #[momo] + fn test_method( + self, + a: impl Into, + b: impl AsRef, + mut c: impl AsMut, + d: impl TryInto, + ) -> Result { + let mut s = a.into(); + s += b.as_ref(); + s += c.as_mut(); + s += &d.try_into()?; + + Ok(s) + } + + #[allow(clippy::needless_arbitrary_self_type)] + #[momo] + fn test_method2( + self: Self, + a: impl Into, + b: impl AsRef, + mut c: impl AsMut, + d: impl TryInto, + ) -> Result { + let mut s = a.into(); + s += b.as_ref(); + s += c.as_mut(); + s += &d.try_into()?; + + Ok(s) + } + + #[momo] + fn test_fn( + a: impl Into, + b: impl AsRef, + mut c: impl AsMut, + d: impl TryInto, + _e: (), + ) -> Result { + let mut s = a.into(); + s += b.as_ref(); + s += c.as_mut(); + s += &d.try_into()?; + + Ok(s) + } + + #[momo] + fn test_fn2( + _this: Self, + a: impl Into, + b: impl AsRef, + mut c: impl AsMut, + d: impl TryInto, + _e: (), + _f: (), + ) -> Result { + let mut s = a.into(); + s += b.as_ref(); + s += c.as_mut(); + s += &d.try_into()?; + + Ok(s) + } + + #[momo] + fn test_fn3( + _this: Pin<&mut Self>, + a: impl Into, + b: impl AsRef, + mut c: impl AsMut, + d: impl TryInto, + _e: (), + _f: (), + ) -> Result { + let mut s = a.into(); + s += b.as_ref(); + s += c.as_mut(); + s += &d.try_into()?; + + Ok(s) + } + + #[allow(unused)] + #[momo] + fn test_fn_ret( + _this: Pin<&mut Self>, + a: impl Into, + b: impl AsRef, + mut c: impl AsMut, + d: impl TryInto, + _e: (), + _f: (), + ) -> Result { + let mut s = a.into(); + s += b.as_ref(); + s += c.as_mut(); + s += &d.try_into()?; + + drop(s); + + Ok(Self) + } +} + +struct S(bool); +impl TryInto for S { + type Error = (); + + fn try_into(self) -> Result { + if self.0 { + Ok(String::from("!2345")) + } else { + Err(()) + } + } +} + +#[test] +fn test_basic_fn() { + assert_eq!( + test_fn("12345", "12345", String::from("12345"), S(true)).unwrap(), + "123451234512345!2345" + ); + + test_fn("12345", "12345", String::from("12345"), S(false)).unwrap_err(); + + assert_eq!( + test_fn_call_style("12345", "12345", String::from("12345"), S(true)).unwrap(), + "123451234512345!2345" + ); + + test_fn_call_style("12345", "12345", String::from("12345"), S(false)).unwrap_err(); + + assert_eq!( + test_fn_where("12345", "12345", String::from("12345"), S(true)).unwrap(), + "123451234512345!2345" + ); + + test_fn_where("12345", "12345", String::from("12345"), S(false)).unwrap_err(); +} + +#[test] +fn test_struct_method() { + // Test test_method + assert_eq!( + TestStruct + .test_method("12345", "12345", String::from("12345"), S(true)) + .unwrap(), + "123451234512345!2345" + ); + + TestStruct + .test_method("12345", "12345", String::from("12345"), S(false)) + .unwrap_err(); + + // Test test_method2 + assert_eq!( + TestStruct + .test_method2("12345", "12345", String::from("12345"), S(true)) + .unwrap(), + "123451234512345!2345" + ); + + TestStruct + .test_method2("12345", "12345", String::from("12345"), S(false)) + .unwrap_err(); +} + +#[test] +fn test_struct_fn() { + assert_eq!( + TestStruct::test_fn("12345", "12345", String::from("12345"), S(true), ()).unwrap(), + "123451234512345!2345" + ); + + TestStruct::test_fn("12345", "12345", String::from("12345"), S(false), ()).unwrap_err(); + + assert_eq!( + TestStruct::test_fn2(TestStruct, "12345", "12345", String::from("12345"), S(true), (), ()).unwrap(), + "123451234512345!2345" + ); + + TestStruct::test_fn2(TestStruct, "12345", "12345", String::from("12345"), S(false), (), ()).unwrap_err(); + + assert_eq!( + TestStruct::test_fn3( + Pin::new(&mut TestStruct), + "12345", + "12345", + String::from("12345"), + S(true), + (), + () + ) + .unwrap(), + "123451234512345!2345" + ); + + TestStruct::test_fn3( + Pin::new(&mut TestStruct), + "12345", + "12345", + String::from("12345"), + S(false), + (), + (), + ) + .unwrap_err(); +} From 58fbb08461064d96dd9816e2fb6911cf76b6badc Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sat, 26 Aug 2023 22:15:20 +1000 Subject: [PATCH 02/29] Apply `momo` to mod `gix::discover` to: - `ThreadSafeRepository::discover_opts` - `ThreadSafeRepository::discover_with_environment_overrides_opts` Signed-off-by: Jiahao XU --- gix/Cargo.toml | 1 + gix/src/discover.rs | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/gix/Cargo.toml b/gix/Cargo.toml index 7c3fe382cde..11af7e57621 100644 --- a/gix/Cargo.toml +++ b/gix/Cargo.toml @@ -126,6 +126,7 @@ fast-sha1 = [ "gix-features/fast-sha1" ] [dependencies] +gix-macros = { version = "^0.0.0", path = "../gix-macros" } gix-utils = { version = "^0.1.5", path = "../gix-utils" } gix-fs = { version = "^0.5.0", path = "../gix-fs" } gix-ref = { version = "^0.35.0", path = "../gix-ref" } diff --git a/gix/src/discover.rs b/gix/src/discover.rs index da43092c318..a5a3b8a4ad4 100644 --- a/gix/src/discover.rs +++ b/gix/src/discover.rs @@ -2,6 +2,7 @@ use std::path::Path; pub use gix_discover::*; +use gix_macros::momo; use crate::{bstr::BString, ThreadSafeRepository}; @@ -31,6 +32,7 @@ impl ThreadSafeRepository { /// if the directory that is discovered can indeed be trusted (or else they'd have to implement the discovery themselves /// and be sure that no attacker ever gets access to a directory structure. The cost of this is a permission check, which /// seems acceptable). + #[momo] pub fn discover_opts( directory: impl AsRef, options: upwards::Options<'_>, @@ -61,6 +63,8 @@ impl ThreadSafeRepository { /// /// Finally, use the `trust_map` to determine which of our own repository options to use /// based on the trust level of the effective repository directory. + #[momo] + #[allow(unused_mut)] pub fn discover_with_environment_overrides_opts( directory: impl AsRef, mut options: upwards::Options<'_>, From 46a9dfe12dedc1cbf997ea408d1f1d2c5d673ba5 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sat, 26 Aug 2023 22:17:28 +1000 Subject: [PATCH 03/29] Apply `momo` to mod `gix::init` to: - `ThreadSafeRepository::init` - `ThreadSafeRepository::init_opts` Signed-off-by: Jiahao XU --- gix/src/init.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gix/src/init.rs b/gix/src/init.rs index bffd5fc5b8f..9b9cb5a61b0 100644 --- a/gix/src/init.rs +++ b/gix/src/init.rs @@ -1,6 +1,7 @@ #![allow(clippy::result_large_err)] use std::{borrow::Cow, convert::TryInto, path::Path}; +use gix_macros::momo; use gix_ref::{ store::WriteReflog, transaction::{PreviousValue, RefEdit}, @@ -40,6 +41,7 @@ impl ThreadSafeRepository { /// /// Fails without action if there is already a `.git` repository inside of `directory`, but /// won't mind if the `directory` otherwise is non-empty. + #[momo] pub fn init( directory: impl AsRef, kind: crate::create::Kind, @@ -56,6 +58,8 @@ impl ThreadSafeRepository { /// /// Instead of naming the default branch `master`, we name it `main` unless configured explicitly using the `init.defaultBranch` /// configuration key. + #[momo] + #[allow(unused_mut)] pub fn init_opts( directory: impl AsRef, kind: crate::create::Kind, From d8355267fd64dbcf22a01a11cb29d93e75f0fb4c Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sat, 26 Aug 2023 22:22:02 +1000 Subject: [PATCH 04/29] Apply `momo` to `gix::object::tree` to: - `Tree<'_>::lookup_entry_by_path` - `Tree<'_>::peel_to_entry_by_path` Signed-off-by: Jiahao XU --- gix/src/object/tree/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gix/src/object/tree/mod.rs b/gix/src/object/tree/mod.rs index 62a7d02e1e8..888621bf109 100644 --- a/gix/src/object/tree/mod.rs +++ b/gix/src/object/tree/mod.rs @@ -1,4 +1,5 @@ use gix_hash::ObjectId; +use gix_macros::momo; pub use gix_object::tree::EntryMode; use gix_object::{bstr::BStr, TreeRefIter}; use gix_odb::FindExt; @@ -132,6 +133,7 @@ impl<'repo> Tree<'repo> { /// /// If any path component contains illformed UTF-8 and thus can't be converted to bytes on platforms which can't do so natively, /// the returned component will be empty which makes the lookup fail. + #[momo] pub fn lookup_entry_by_path( &self, relative_path: impl AsRef, @@ -154,6 +156,7 @@ impl<'repo> Tree<'repo> { /// /// If any path component contains illformed UTF-8 and thus can't be converted to bytes on platforms which can't do so natively, /// the returned component will be empty which makes the lookup fail. + #[momo] pub fn peel_to_entry_by_path( &mut self, relative_path: impl AsRef, From 3ce014499a86e4e8bb57ffe7caa540792c1c0a47 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sat, 26 Aug 2023 22:23:04 +1000 Subject: [PATCH 05/29] Apply `momo` to mod `gix::open::repository` to: - `ThreadSafeRepository::open_opts` - `ThreadSafeRepository::open_with_environment_overrides` Signed-off-by: Jiahao XU --- gix/src/open/repository.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gix/src/open/repository.rs b/gix/src/open/repository.rs index f533770c5b2..df096a4e3dd 100644 --- a/gix/src/open/repository.rs +++ b/gix/src/open/repository.rs @@ -2,6 +2,7 @@ use std::{borrow::Cow, path::PathBuf}; use gix_features::threading::OwnShared; +use gix_macros::momo; use super::{Error, Options}; use crate::{ @@ -58,6 +59,8 @@ impl ThreadSafeRepository { /// /// Note that opening a repository for implementing custom hooks is also handle specifically in /// [`open_with_environment_overrides()`][Self::open_with_environment_overrides()]. + #[momo] + #[allow(unused_mut)] pub fn open_opts(path: impl Into, mut options: Options) -> Result { let _span = gix_trace::coarse!("ThreadSafeRepository::open()"); let (path, kind) = { @@ -109,6 +112,7 @@ impl ThreadSafeRepository { // GIT_PROXY_SSL_CERT, GIT_PROXY_SSL_KEY, GIT_PROXY_SSL_CERT_PASSWORD_PROTECTED. // GIT_PROXY_SSL_CAINFO, GIT_SSL_CIPHER_LIST, GIT_HTTP_MAX_REQUESTS, GIT_CURL_FTP_NO_EPSV, #[doc(alias = "open_from_env", alias = "git2")] + #[momo] pub fn open_with_environment_overrides( fallback_directory: impl Into, trust_map: gix_sec::trust::Mapping, From 767ec2dcfd1fadaca93390770604494d03f88ab3 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sat, 26 Aug 2023 22:24:43 +1000 Subject: [PATCH 06/29] Apply `momo` to `gix::pathspec` to: - `Pathspec<'_>::pattern_matching_relative_path` - `Pathspec<'_>::is_included` Signed-off-by: Jiahao XU --- gix/src/pathspec.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/gix/src/pathspec.rs b/gix/src/pathspec.rs index f5af7b3228e..01445601a4f 100644 --- a/gix/src/pathspec.rs +++ b/gix/src/pathspec.rs @@ -1,4 +1,5 @@ //! Pathspec plumbing and abstractions +use gix_macros::momo; use gix_odb::FindExt; pub use gix_pathspec::*; @@ -92,6 +93,8 @@ impl<'repo> Pathspec<'repo> { alias = "matches_path", alias = "git2" )] + #[momo] + #[allow(clippy::needless_lifetimes)] pub fn pattern_matching_relative_path<'a>( &mut self, relative_path: impl Into<&'a BStr>, @@ -111,6 +114,8 @@ impl<'repo> Pathspec<'repo> { /// The simplified version of [`pattern_matching_relative_path()`](Self::pattern_matching_relative_path()) which returns /// `true` if `relative_path` is included in the set of positive pathspecs, while not being excluded. + #[momo] + #[allow(clippy::needless_lifetimes)] pub fn is_included<'a>(&mut self, relative_path: impl Into<&'a BStr>, is_dir: Option) -> bool { self.pattern_matching_relative_path(relative_path, is_dir) .map_or(false, |m| !m.is_excluded()) From 3c205abbdc0a80090b9f0f5681ce0949497e770f Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sat, 26 Aug 2023 22:25:49 +1000 Subject: [PATCH 07/29] Apply `momo` to `gix::reference` to: - `set_target_id` - `Platform<'_>::prefixed` Signed-off-by: Jiahao XU --- gix/src/reference/edits.rs | 5 +++-- gix/src/reference/iter.rs | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/gix/src/reference/edits.rs b/gix/src/reference/edits.rs index c6510c2e00b..20834077001 100644 --- a/gix/src/reference/edits.rs +++ b/gix/src/reference/edits.rs @@ -1,8 +1,8 @@ /// pub mod set_target_id { - use gix_ref::{transaction::PreviousValue, Target}; - use crate::{bstr::BString, Reference}; + use gix_macros::momo; + use gix_ref::{transaction::PreviousValue, Target}; mod error { use gix_ref::FullName; @@ -28,6 +28,7 @@ pub mod set_target_id { /// If multiple reference should be changed, use [`Repository::edit_references()`][crate::Repository::edit_references()] /// or the lower level reference database instead. #[allow(clippy::result_large_err)] + #[momo] pub fn set_target_id( &mut self, id: impl Into, diff --git a/gix/src/reference/iter.rs b/gix/src/reference/iter.rs index a2b022f64df..983a3803fc0 100644 --- a/gix/src/reference/iter.rs +++ b/gix/src/reference/iter.rs @@ -1,6 +1,7 @@ //! use std::path::Path; +use gix_macros::momo; use gix_odb::pack::Find; use gix_ref::file::ReferenceExt; @@ -42,6 +43,7 @@ impl<'r> Platform<'r> { /// These are of the form `refs/heads` or `refs/remotes/origin`, and must not contain relative paths components like `.` or `..`. // TODO: Create a custom `Path` type that enforces the requirements of git naturally, this type is surprising possibly on windows // and when not using a trailing '/' to signal directories. + #[momo] pub fn prefixed(&self, prefix: impl AsRef) -> Result, init::Error> { Ok(Iter::new(self.repo, self.platform.prefixed(prefix)?)) } From ea5c2dbabe9d3c1eb1ab5f15a578ec9f9c36a5d8 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sat, 26 Aug 2023 22:28:01 +1000 Subject: [PATCH 08/29] Apply `momo` to mod `remote::connection::fetch::receive_pack` to `receive` Signed-off-by: Jiahao XU --- gix/src/remote/connection/fetch/receive_pack.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/gix/src/remote/connection/fetch/receive_pack.rs b/gix/src/remote/connection/fetch/receive_pack.rs index 089fe295721..1df1a7174f3 100644 --- a/gix/src/remote/connection/fetch/receive_pack.rs +++ b/gix/src/remote/connection/fetch/receive_pack.rs @@ -73,6 +73,7 @@ where /// - `gitoxide.userAgent` is read to obtain the application user agent for git servers and for HTTP servers as well. /// #[gix_protocol::maybe_async::maybe_async] + #[allow(clippy::drop_non_drop)] pub async fn receive

(mut self, mut progress: P, should_interrupt: &AtomicBool) -> Result where P: Progress, From 5a505377199730354c2a6b6b7b060184558bb9c4 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sat, 26 Aug 2023 22:30:57 +1000 Subject: [PATCH 09/29] Apply `momo` to mod `gix::repository` to: - `Repository::transport_options` - `Repository::find_object` - `Repository::try_find_object` - `Repository::find_head` - `Repository::try_find_head` - `Repository::tag` - `Repository::tag_reference` - `Repository::try_find_remote` - `Repository::try_find_remote_without_url_rewrite` - `Repository::rev_parse` - `Repository::worktree_stream` Signed-off-by: Jiahao XU --- Cargo.lock | 1 + gix/src/repository/config/transport.rs | 8 ++++++-- gix/src/repository/object.rs | 16 +++++++++------- gix/src/repository/reference.rs | 6 ++++-- gix/src/repository/remote.rs | 4 ++++ gix/src/repository/revision.rs | 3 +++ gix/src/repository/worktree.rs | 2 +- 7 files changed, 28 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 441417f4cdb..e73076b78bc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1208,6 +1208,7 @@ dependencies = [ "gix-ignore 0.6.0", "gix-index 0.22.0", "gix-lock 8.0.0", + "gix-macros", "gix-mailmap", "gix-negotiate", "gix-object 0.35.0", diff --git a/gix/src/repository/config/transport.rs b/gix/src/repository/config/transport.rs index 81107076c31..d43fffaf560 100644 --- a/gix/src/repository/config/transport.rs +++ b/gix/src/repository/config/transport.rs @@ -1,6 +1,8 @@ #![allow(clippy::result_large_err)] use std::any::Any; +use gix_macros::momo; + use crate::bstr::BStr; impl crate::Repository { @@ -21,12 +23,14 @@ impl crate::Repository { )), allow(unused_variables) )] + #[allow(clippy::needless_lifetimes)] + #[momo] pub fn transport_options<'a>( &self, url: impl Into<&'a BStr>, remote_name: Option<&BStr>, ) -> Result>, crate::config::transport::Error> { - let url = gix_url::parse(url.into())?; + let url = gix_url::parse(url)?; use gix_url::Scheme::*; match &url.scheme { @@ -270,7 +274,7 @@ impl crate::Repository { .proxy .as_deref() .filter(|url| !url.is_empty()) - .map(|url| gix_url::parse(url.into())) + .map(|url| gix_url::parse(<&BStr>::from(url))) .transpose()? .filter(|url| url.user().is_some()) .map(|url| -> Result<_, config::transport::http::Error> { diff --git a/gix/src/repository/object.rs b/gix/src/repository/object.rs index bab7d1b5c50..2dafe6b4245 100644 --- a/gix/src/repository/object.rs +++ b/gix/src/repository/object.rs @@ -2,6 +2,7 @@ use std::{convert::TryInto, ops::DerefMut}; use gix_hash::ObjectId; +use gix_macros::momo; use gix_odb::{Find, FindExt, Header, HeaderExt, Write}; use gix_ref::{ transaction::{LogChange, PreviousValue, RefLog}, @@ -21,8 +22,8 @@ impl crate::Repository { /// /// In order to get the kind of the object, is must be fully decoded from storage if it is packed with deltas. /// Loose object could be partially decoded, even though that's not implemented. + #[momo] pub fn find_object(&self, id: impl Into) -> Result, object::find::existing::Error> { - let id = id.into(); if id == gix_hash::ObjectId::empty_tree(self.object_hash()) { return Ok(Object { id, @@ -40,8 +41,8 @@ impl crate::Repository { /// /// Note that despite being cheaper than [`Self::find_object()`], there is still some effort traversing delta-chains. #[doc(alias = "read_header", alias = "git2")] + #[momo] pub fn find_header(&self, id: impl Into) -> Result { - let id = id.into(); if id == gix_hash::ObjectId::empty_tree(self.object_hash()) { return Ok(gix_odb::find::Header::Loose { kind: gix_object::Kind::Tree, @@ -54,11 +55,11 @@ impl crate::Repository { /// Obtain information about an object without fully decoding it, or `None` if the object doesn't exist. /// /// Note that despite being cheaper than [`Self::try_find_object()`], there is still some effort traversing delta-chains. + #[momo] pub fn try_find_header( &self, id: impl Into, ) -> Result, object::find::Error> { - let id = id.into(); if id == gix_hash::ObjectId::empty_tree(self.object_hash()) { return Ok(Some(gix_odb::find::Header::Loose { kind: gix_object::Kind::Tree, @@ -69,8 +70,8 @@ impl crate::Repository { } /// Try to find the object with `id` or return `None` if it wasn't found. + #[momo] pub fn try_find_object(&self, id: impl Into) -> Result>, object::find::Error> { - let id = id.into(); if id == gix_hash::ObjectId::empty_tree(self.object_hash()) { return Ok(Some(Object { id, @@ -163,6 +164,7 @@ impl crate::Repository { /// /// It will be created with `constraint` which is most commonly to [only create it][PreviousValue::MustNotExist] /// or to [force overwriting a possibly existing tag](PreviousValue::Any). + #[momo] pub fn tag( &self, name: impl AsRef, @@ -173,11 +175,11 @@ impl crate::Repository { constraint: PreviousValue, ) -> Result, tag::Error> { let tag = gix_object::Tag { - target: target.as_ref().into(), + target: target.into(), target_kind, - name: name.as_ref().into(), + name: name.into(), tagger: tagger.map(|t| t.to_owned()), - message: message.as_ref().into(), + message: message.into(), pgp_signature: None, }; let tag_id = self.write_object(&tag)?; diff --git a/gix/src/repository/reference.rs b/gix/src/repository/reference.rs index 0428699e533..6c8274e2f68 100644 --- a/gix/src/repository/reference.rs +++ b/gix/src/repository/reference.rs @@ -1,6 +1,7 @@ use std::convert::TryInto; use gix_hash::ObjectId; +use gix_macros::momo; use gix_ref::{ transaction::{Change, LogChange, PreviousValue, RefEdit, RefLog}, FullName, PartialNameRef, Target, @@ -14,20 +15,21 @@ impl crate::Repository { /// /// It will be created with `constraint` which is most commonly to [only create it][PreviousValue::MustNotExist] /// or to [force overwriting a possibly existing tag](PreviousValue::Any). + #[momo] pub fn tag_reference( &self, name: impl AsRef, target: impl Into, constraint: PreviousValue, ) -> Result, reference::edit::Error> { - let id = target.into(); + let id = target; let mut edits = self.edit_reference(RefEdit { change: Change::Update { log: Default::default(), expected: constraint, new: Target::Peeled(id), }, - name: format!("refs/tags/{}", name.as_ref()).try_into()?, + name: format!("refs/tags/{name}").try_into()?, deref: false, })?; assert_eq!(edits.len(), 1, "reference splits should ever happen"); diff --git a/gix/src/repository/remote.rs b/gix/src/repository/remote.rs index 74ebbaea027..50f0e733dd1 100644 --- a/gix/src/repository/remote.rs +++ b/gix/src/repository/remote.rs @@ -1,6 +1,8 @@ #![allow(clippy::result_large_err)] use std::convert::TryInto; +use gix_macros::momo; + use crate::{bstr::BStr, config, remote, remote::find, Remote}; impl crate::Repository { @@ -61,6 +63,7 @@ impl crate::Repository { /// as negations/excludes are applied after includes. /// /// We will only include information if we deem it [trustworthy][crate::open::Options::filter_config_section()]. + #[momo] pub fn try_find_remote<'a>(&self, name_or_url: impl Into<&'a BStr>) -> Option, find::Error>> { self.try_find_remote_inner(name_or_url, true) } @@ -68,6 +71,7 @@ impl crate::Repository { /// Similar to [`try_find_remote()`][Self::try_find_remote()], but removes a failure mode if rewritten URLs turn out to be invalid /// as it skips rewriting them. /// Use this in conjunction with [`Remote::rewrite_urls()`] to non-destructively apply the rules and keep the failed urls unchanged. + #[momo] pub fn try_find_remote_without_url_rewrite<'a>( &self, name_or_url: impl Into<&'a BStr>, diff --git a/gix/src/repository/revision.rs b/gix/src/repository/revision.rs index 4eedf0867e2..27aeb38e194 100644 --- a/gix/src/repository/revision.rs +++ b/gix/src/repository/revision.rs @@ -1,4 +1,5 @@ use crate::{bstr::BStr, revision, Id}; +use gix_macros::momo; /// Methods for resolving revisions by spec or working with the commit graph. impl crate::Repository { @@ -9,6 +10,8 @@ impl crate::Repository { /// - `@` actually stands for `HEAD`, whereas `git` resolves it to the object pointed to by `HEAD` without making the /// `HEAD` ref available for lookups. #[doc(alias = "revparse", alias = "git2")] + #[allow(clippy::needless_lifetimes)] + #[momo] pub fn rev_parse<'a>(&self, spec: impl Into<&'a BStr>) -> Result, revision::spec::parse::Error> { revision::Spec::from_bstr( spec, diff --git a/gix/src/repository/worktree.rs b/gix/src/repository/worktree.rs index 17db1c0ebda..4773c6cd8e1 100644 --- a/gix/src/repository/worktree.rs +++ b/gix/src/repository/worktree.rs @@ -57,12 +57,12 @@ impl crate::Repository { /// The entries will look exactly like they would if one would check them out, with filters applied. /// The `export-ignore` attribute is used to skip blobs or directories to which it applies. #[cfg(feature = "worktree-stream")] + #[gix_macros::momo] pub fn worktree_stream( &self, id: impl Into, ) -> Result<(gix_worktree_stream::Stream, gix_index::File), crate::repository::worktree_stream::Error> { use gix_odb::{FindExt, HeaderExt}; - let id = id.into(); let header = self.objects.header(id)?; if !header.kind().is_tree() { return Err(crate::repository::worktree_stream::Error::NotATree { From ff210d82573cebfdf4edbfb39beaef08979c058f Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 27 Aug 2023 09:28:52 +1000 Subject: [PATCH 10/29] Fix clippy lints in `gix/src/repository/remote.rs` Signed-off-by: Jiahao XU --- gix/src/repository/remote.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gix/src/repository/remote.rs b/gix/src/repository/remote.rs index 50f0e733dd1..2110affbc4e 100644 --- a/gix/src/repository/remote.rs +++ b/gix/src/repository/remote.rs @@ -64,6 +64,7 @@ impl crate::Repository { /// /// We will only include information if we deem it [trustworthy][crate::open::Options::filter_config_section()]. #[momo] + #[allow(clippy::needless_lifetimes)] pub fn try_find_remote<'a>(&self, name_or_url: impl Into<&'a BStr>) -> Option, find::Error>> { self.try_find_remote_inner(name_or_url, true) } @@ -72,6 +73,7 @@ impl crate::Repository { /// as it skips rewriting them. /// Use this in conjunction with [`Remote::rewrite_urls()`] to non-destructively apply the rules and keep the failed urls unchanged. #[momo] + #[allow(clippy::needless_lifetimes)] pub fn try_find_remote_without_url_rewrite<'a>( &self, name_or_url: impl Into<&'a BStr>, From e7602257662cd9ddb4cfe41ef26cdf28cc007be7 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 28 Aug 2023 22:05:27 +1000 Subject: [PATCH 11/29] Manually de-`momo` `Repository::try_find_remote_{without_url_rewrite}` Since the `try_find_remote_inner` is already a separate function, there's no need to use `momo`. Signed-off-by: Jiahao XU --- gix/src/repository/remote.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/gix/src/repository/remote.rs b/gix/src/repository/remote.rs index 2110affbc4e..ebd0f52f3a1 100644 --- a/gix/src/repository/remote.rs +++ b/gix/src/repository/remote.rs @@ -1,8 +1,6 @@ #![allow(clippy::result_large_err)] use std::convert::TryInto; -use gix_macros::momo; - use crate::{bstr::BStr, config, remote, remote::find, Remote}; impl crate::Repository { @@ -63,22 +61,18 @@ impl crate::Repository { /// as negations/excludes are applied after includes. /// /// We will only include information if we deem it [trustworthy][crate::open::Options::filter_config_section()]. - #[momo] - #[allow(clippy::needless_lifetimes)] pub fn try_find_remote<'a>(&self, name_or_url: impl Into<&'a BStr>) -> Option, find::Error>> { - self.try_find_remote_inner(name_or_url, true) + self.try_find_remote_inner(name_or_url.into(), true) } /// Similar to [`try_find_remote()`][Self::try_find_remote()], but removes a failure mode if rewritten URLs turn out to be invalid /// as it skips rewriting them. /// Use this in conjunction with [`Remote::rewrite_urls()`] to non-destructively apply the rules and keep the failed urls unchanged. - #[momo] - #[allow(clippy::needless_lifetimes)] pub fn try_find_remote_without_url_rewrite<'a>( &self, name_or_url: impl Into<&'a BStr>, ) -> Option, find::Error>> { - self.try_find_remote_inner(name_or_url, false) + self.try_find_remote_inner(name_or_url.into(), false) } fn try_find_remote_inner<'a>( From 95a16264b0a6f8c7d8e2acded3a4c9c170c2729b Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 29 Aug 2023 21:10:18 +1000 Subject: [PATCH 12/29] Remove `TryInto` support from `gix_macros::momo` Part of the reason is that `impl TryInto` isn't as common as `AsRef`, `AsMut` or `Into`, but also because its error handling logic is much harder to parse and replace in the inner function. Signed-off-by: Jiahao XU --- gix-macros/src/lib.rs | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/gix-macros/src/lib.rs b/gix-macros/src/lib.rs index 8688ccabaf1..b485f60e625 100644 --- a/gix-macros/src/lib.rs +++ b/gix-macros/src/lib.rs @@ -8,7 +8,6 @@ use syn::{fold::Fold, punctuated::Punctuated, spanned::Spanned, *}; // All conversions we support. Check references to this type for an idea how to add more. enum Conversion<'a> { Into(&'a Type), - TryInto(&'a Type), AsRef(&'a Type), AsMut(&'a Type), } @@ -17,7 +16,6 @@ impl<'a> Conversion<'a> { fn target_type(&self) -> Type { match *self { Conversion::Into(ty) => ty.clone(), - Conversion::TryInto(ty) => ty.clone(), Conversion::AsRef(ty) => parse_quote!(&#ty), Conversion::AsMut(ty) => parse_quote!(&mut #ty), } @@ -26,7 +24,6 @@ impl<'a> Conversion<'a> { fn conversion_expr(&self, i: Ident) -> Expr { match *self { Conversion::Into(_) => parse_quote!(#i.into()), - Conversion::TryInto(_) => parse_quote!(#i.try_into()?), Conversion::AsRef(_) => parse_quote!(#i.as_ref()), Conversion::AsMut(_) => parse_quote!(#i.as_mut()), } @@ -52,8 +49,6 @@ fn parse_bounds(bounds: &Punctuated) -> Option( let mut argtypes = Punctuated::new(); let mut conversions = Conversions { intos: Vec::new(), - try_intos: Vec::new(), as_refs: Vec::new(), as_muts: Vec::new(), }; @@ -205,7 +199,6 @@ fn convert<'a>( struct Conversions { intos: Vec, - try_intos: Vec, as_refs: Vec, as_muts: Vec, } @@ -214,7 +207,6 @@ impl Conversions { fn add(&mut self, ident: Ident, conv: Conversion) { match conv { Conversion::Into(_) => self.intos.push(ident), - Conversion::TryInto(_) => self.try_intos.push(ident), Conversion::AsRef(_) => self.as_refs.push(ident), Conversion::AsMut(_) => self.as_muts.push(ident), } @@ -238,7 +230,6 @@ impl Fold for Conversions { match expr { Expr::MethodCall(mc) if mc.args.is_empty() => match &*mc.method.to_string() { "into" if has_conversion(&self.intos, &mc.receiver) => *mc.receiver, - "try_into" if has_conversion(&self.try_intos, &mc.receiver) => *mc.receiver, "as_ref" if has_conversion(&self.as_refs, &mc.receiver) => *mc.receiver, "as_mut" if has_conversion(&self.as_muts, &mc.receiver) => *mc.receiver, @@ -251,7 +242,6 @@ impl Fold for Conversions { .. }) if segments.len() == 2 => match (&*segments[0].ident.to_string(), &*segments[1].ident.to_string()) { ("Into", "into") if has_conversion(&self.intos, &call.args[0]) => call.args[0].clone(), - ("TryInto", "try_into") if has_conversion(&self.try_intos, &call.args[0]) => call.args[0].clone(), ("AsRef", "as_ref") if matches!(&call.args[0], Expr::Reference(ExprReference { expr, mutability: None, .. }) if has_conversion(&self.as_refs, expr)) => { if let Expr::Reference(ExprReference { expr, .. }) = &call.args[0] { @@ -272,29 +262,6 @@ impl Fold for Conversions { }, _ => syn::fold::fold_expr(self, Expr::Call(call)), }, - Expr::Try(expr_try) => match &*expr_try.expr { - Expr::MethodCall(mc) - if mc.args.is_empty() - && mc.method == "try_into" - && has_conversion(&self.try_intos, &mc.receiver) => - { - (*mc.receiver).clone() - } - Expr::Call(call) if call.args.len() == 1 => match &*call.func { - Expr::Path(ExprPath { - path: Path { segments, .. }, - .. - }) if segments.len() == 2 - && segments[0].ident == "TryInto" - && segments[1].ident == "try_into" - && has_conversion(&self.try_intos, &call.args[0]) => - { - call.args[0].clone() - } - _ => syn::fold::fold_expr(self, Expr::Try(expr_try)), - }, - _ => syn::fold::fold_expr(self, Expr::Try(expr_try)), - }, _ => syn::fold::fold_expr(self, expr), } } From c72eaa05697a3e34adaa3ee90584dce4b5c00120 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 29 Aug 2023 21:38:33 +1000 Subject: [PATCH 13/29] Dramatically simplify `gix_macros::momo` And make applying support of `gix_macros::momo` truly painless by making it work out-of-the-box. Signed-off-by: Jiahao XU --- gix-macros/src/lib.rs | 167 ++----------------------- gix/src/repository/config/transport.rs | 3 +- gix/src/repository/object.rs | 10 +- gix/src/repository/reference.rs | 4 +- gix/src/repository/worktree.rs | 3 +- 5 files changed, 24 insertions(+), 163 deletions(-) diff --git a/gix-macros/src/lib.rs b/gix-macros/src/lib.rs index b485f60e625..f2503e9b332 100644 --- a/gix-macros/src/lib.rs +++ b/gix-macros/src/lib.rs @@ -1,8 +1,8 @@ -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use proc_macro::TokenStream; use quote::quote; -use syn::{fold::Fold, punctuated::Punctuated, spanned::Spanned, *}; +use syn::{punctuated::Punctuated, spanned::Spanned, *}; #[derive(Copy, Clone)] // All conversions we support. Check references to this type for an idea how to add more. @@ -13,14 +13,6 @@ enum Conversion<'a> { } impl<'a> Conversion<'a> { - fn target_type(&self) -> Type { - match *self { - Conversion::Into(ty) => ty.clone(), - Conversion::AsRef(ty) => parse_quote!(&#ty), - Conversion::AsMut(ty) => parse_quote!(&mut #ty), - } - } - fn conversion_expr(&self, i: Ident) -> Expr { match *self { Conversion::Into(_) => parse_quote!(#i.into()), @@ -62,9 +54,8 @@ fn parse_bounds(bounds: &Punctuated) -> Option (HashMap>, Generics) { +fn parse_generics(decl: &Signature) -> HashMap> { let mut ty_conversions = HashMap::new(); - let mut params = Punctuated::new(); for gp in decl.generics.params.iter() { if let GenericParam::Type(ref tp) = gp { if let Some(conversion) = parse_bounds(&tp.bounds) { @@ -72,48 +63,20 @@ fn parse_generics(decl: &Signature) -> (HashMap>, Generics continue; } } - params.push(gp.clone()); } - let where_clause = if let Some(ref wc) = decl.generics.where_clause { - let mut idents_to_remove = HashSet::new(); - let mut predicates = Punctuated::new(); + if let Some(ref wc) = decl.generics.where_clause { for wp in wc.predicates.iter() { if let WherePredicate::Type(ref pt) = wp { if let Some(ident) = parse_bounded_type(&pt.bounded_ty) { if let Some(conversion) = parse_bounds(&pt.bounds) { - idents_to_remove.insert(ident.clone()); ty_conversions.insert(ident, conversion); continue; } } } - predicates.push(wp.clone()); } - params = params - .into_iter() - .filter(|param| { - if let GenericParam::Type(type_param) = param { - !idents_to_remove.contains(&type_param.ident) - } else { - true - } - }) - .collect(); - Some(WhereClause { - predicates, - ..wc.clone() - }) - } else { - None - }; - ( - ty_conversions, - Generics { - params, - where_clause, - ..decl.generics.clone() - }, - ) + } + ty_conversions } fn pat_to_ident(pat: &Pat) -> Ident { @@ -131,140 +94,36 @@ fn pat_to_expr(pat: &Pat) -> Expr { fn convert<'a>( inputs: &'a Punctuated, ty_conversions: &HashMap>, -) -> ( - Punctuated, - Conversions, - Punctuated, - bool, -) { - let mut argtypes = Punctuated::new(); - let mut conversions = Conversions { - intos: Vec::new(), - as_refs: Vec::new(), - as_muts: Vec::new(), - }; +) -> (Punctuated, bool) { let mut argexprs = Punctuated::new(); let mut has_self = false; inputs.iter().for_each(|input| match input { FnArg::Receiver(..) => { has_self = true; - argtypes.push(input.clone()); } - FnArg::Typed(PatType { - ref pat, - ref ty, - ref colon_token, - .. - }) => match **ty { + FnArg::Typed(PatType { ref pat, ref ty, .. }) => match **ty { Type::ImplTrait(TypeImplTrait { ref bounds, .. }) => { if let Some(conv) = parse_bounds(bounds) { - argtypes.push(FnArg::Typed(PatType { - attrs: Vec::new(), - pat: pat.clone(), - colon_token: *colon_token, - ty: Box::new(conv.target_type()), - })); let ident = pat_to_ident(pat); - conversions.add(ident.clone(), conv); argexprs.push(conv.conversion_expr(ident)); } else { - argtypes.push(input.clone()); argexprs.push(pat_to_expr(pat)); } } Type::Path(..) => { if let Some(conv) = parse_bounded_type(ty).and_then(|ident| ty_conversions.get(&ident)) { - argtypes.push(FnArg::Typed(PatType { - attrs: Vec::new(), - pat: pat.clone(), - colon_token: *colon_token, - ty: Box::new(conv.target_type()), - })); let ident = pat_to_ident(pat); - conversions.add(ident.clone(), *conv); argexprs.push(conv.conversion_expr(ident)); } else { - argtypes.push(input.clone()); argexprs.push(pat_to_expr(pat)); } } _ => { - argtypes.push(input.clone()); argexprs.push(pat_to_expr(pat)); } }, }); - (argtypes, conversions, argexprs, has_self) -} - -struct Conversions { - intos: Vec, - as_refs: Vec, - as_muts: Vec, -} - -impl Conversions { - fn add(&mut self, ident: Ident, conv: Conversion) { - match conv { - Conversion::Into(_) => self.intos.push(ident), - Conversion::AsRef(_) => self.as_refs.push(ident), - Conversion::AsMut(_) => self.as_muts.push(ident), - } - } -} - -fn has_conversion(idents: &[Ident], expr: &Expr) -> bool { - if let Expr::Path(ExprPath { ref path, .. }) = *expr { - if path.segments.len() == 1 { - let seg = path.segments.iter().last().unwrap(); - return idents.iter().any(|i| i == &seg.ident); - } - } - false -} - -#[allow(clippy::collapsible_if)] -impl Fold for Conversions { - fn fold_expr(&mut self, expr: Expr) -> Expr { - //TODO: Also catch `Expr::Call` with suitable paths & args - match expr { - Expr::MethodCall(mc) if mc.args.is_empty() => match &*mc.method.to_string() { - "into" if has_conversion(&self.intos, &mc.receiver) => *mc.receiver, - - "as_ref" if has_conversion(&self.as_refs, &mc.receiver) => *mc.receiver, - "as_mut" if has_conversion(&self.as_muts, &mc.receiver) => *mc.receiver, - - _ => syn::fold::fold_expr(self, Expr::MethodCall(mc)), - }, - Expr::Call(call) if call.args.len() == 1 => match &*call.func { - Expr::Path(ExprPath { - path: Path { segments, .. }, - .. - }) if segments.len() == 2 => match (&*segments[0].ident.to_string(), &*segments[1].ident.to_string()) { - ("Into", "into") if has_conversion(&self.intos, &call.args[0]) => call.args[0].clone(), - - ("AsRef", "as_ref") if matches!(&call.args[0], Expr::Reference(ExprReference { expr, mutability: None, .. }) if has_conversion(&self.as_refs, expr)) => { - if let Expr::Reference(ExprReference { expr, .. }) = &call.args[0] { - (**expr).clone() - } else { - panic!("expr must be Reference") - } - } - ("AsMut", "as_mut") if matches!(&call.args[0], Expr::Reference(ExprReference { expr, mutability: Some(_), .. }) if has_conversion(&self.as_muts, expr)) => { - if let Expr::Reference(ExprReference { expr, .. }) = &call.args[0] { - (**expr).clone() - } else { - panic!("expr must be Reference") - } - } - - _ => syn::fold::fold_expr(self, Expr::Call(call)), - }, - _ => syn::fold::fold_expr(self, Expr::Call(call)), - }, - _ => syn::fold::fold_expr(self, expr), - } - } + (argexprs, has_self) } fn contains_self_type_path(path: &Path) -> bool { @@ -339,8 +198,8 @@ fn momo_inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream { }; if let Item::Fn(item_fn) = fn_item { - let (ty_conversions, generics) = parse_generics(&item_fn.sig); - let (argtypes, mut conversions, argexprs, has_self) = convert(&item_fn.sig.inputs, &ty_conversions); + let ty_conversions = parse_generics(&item_fn.sig); + let (argexprs, has_self) = convert(&item_fn.sig.inputs, &ty_conversions); let uses_self = has_self || item_fn.sig.inputs.iter().any(has_self_type) @@ -367,11 +226,9 @@ fn momo_inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream { vis: Visibility::Inherited, sig: Signature { ident: inner_ident.clone(), - generics, - inputs: argtypes, ..item_fn.sig.clone() }, - block: Box::new(conversions.fold_block(*item_fn.block)), + block: item_fn.block, }); if uses_self { diff --git a/gix/src/repository/config/transport.rs b/gix/src/repository/config/transport.rs index d43fffaf560..811848a5dc9 100644 --- a/gix/src/repository/config/transport.rs +++ b/gix/src/repository/config/transport.rs @@ -23,14 +23,13 @@ impl crate::Repository { )), allow(unused_variables) )] - #[allow(clippy::needless_lifetimes)] #[momo] pub fn transport_options<'a>( &self, url: impl Into<&'a BStr>, remote_name: Option<&BStr>, ) -> Result>, crate::config::transport::Error> { - let url = gix_url::parse(url)?; + let url = gix_url::parse(url.into())?; use gix_url::Scheme::*; match &url.scheme { diff --git a/gix/src/repository/object.rs b/gix/src/repository/object.rs index 2dafe6b4245..e7300a6250a 100644 --- a/gix/src/repository/object.rs +++ b/gix/src/repository/object.rs @@ -24,6 +24,7 @@ impl crate::Repository { /// Loose object could be partially decoded, even though that's not implemented. #[momo] pub fn find_object(&self, id: impl Into) -> Result, object::find::existing::Error> { + let id = id.into(); if id == gix_hash::ObjectId::empty_tree(self.object_hash()) { return Ok(Object { id, @@ -43,6 +44,7 @@ impl crate::Repository { #[doc(alias = "read_header", alias = "git2")] #[momo] pub fn find_header(&self, id: impl Into) -> Result { + let id = id.into(); if id == gix_hash::ObjectId::empty_tree(self.object_hash()) { return Ok(gix_odb::find::Header::Loose { kind: gix_object::Kind::Tree, @@ -60,6 +62,7 @@ impl crate::Repository { &self, id: impl Into, ) -> Result, object::find::Error> { + let id = id.into(); if id == gix_hash::ObjectId::empty_tree(self.object_hash()) { return Ok(Some(gix_odb::find::Header::Loose { kind: gix_object::Kind::Tree, @@ -72,6 +75,7 @@ impl crate::Repository { /// Try to find the object with `id` or return `None` if it wasn't found. #[momo] pub fn try_find_object(&self, id: impl Into) -> Result>, object::find::Error> { + let id = id.into(); if id == gix_hash::ObjectId::empty_tree(self.object_hash()) { return Ok(Some(Object { id, @@ -175,11 +179,11 @@ impl crate::Repository { constraint: PreviousValue, ) -> Result, tag::Error> { let tag = gix_object::Tag { - target: target.into(), + target: target.as_ref().into(), target_kind, - name: name.into(), + name: name.as_ref().into(), tagger: tagger.map(|t| t.to_owned()), - message: message.into(), + message: message.as_ref().into(), pgp_signature: None, }; let tag_id = self.write_object(&tag)?; diff --git a/gix/src/repository/reference.rs b/gix/src/repository/reference.rs index 6c8274e2f68..4fd6aeb9f69 100644 --- a/gix/src/repository/reference.rs +++ b/gix/src/repository/reference.rs @@ -22,14 +22,14 @@ impl crate::Repository { target: impl Into, constraint: PreviousValue, ) -> Result, reference::edit::Error> { - let id = target; + let id = target.into(); let mut edits = self.edit_reference(RefEdit { change: Change::Update { log: Default::default(), expected: constraint, new: Target::Peeled(id), }, - name: format!("refs/tags/{name}").try_into()?, + name: format!("refs/tags/{}", name.as_ref()).try_into()?, deref: false, })?; assert_eq!(edits.len(), 1, "reference splits should ever happen"); diff --git a/gix/src/repository/worktree.rs b/gix/src/repository/worktree.rs index 4773c6cd8e1..af2ff10434c 100644 --- a/gix/src/repository/worktree.rs +++ b/gix/src/repository/worktree.rs @@ -63,10 +63,11 @@ impl crate::Repository { id: impl Into, ) -> Result<(gix_worktree_stream::Stream, gix_index::File), crate::repository::worktree_stream::Error> { use gix_odb::{FindExt, HeaderExt}; + let id = id.into(); let header = self.objects.header(id)?; if !header.kind().is_tree() { return Err(crate::repository::worktree_stream::Error::NotATree { - id: id.to_owned(), + id, actual: header.kind(), }); } From e1b9d51acd137cdea7680584451702e52aab775f Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 29 Aug 2023 21:43:40 +1000 Subject: [PATCH 14/29] Remove unnecessary `#[allow(clippy::needless_lifetimes)]` Thanks for new internal of `gix_macros::momo`! Signed-off-by: Jiahao XU --- gix/src/pathspec.rs | 2 -- gix/src/repository/revision.rs | 1 - 2 files changed, 3 deletions(-) diff --git a/gix/src/pathspec.rs b/gix/src/pathspec.rs index 01445601a4f..4c56be28e81 100644 --- a/gix/src/pathspec.rs +++ b/gix/src/pathspec.rs @@ -94,7 +94,6 @@ impl<'repo> Pathspec<'repo> { alias = "git2" )] #[momo] - #[allow(clippy::needless_lifetimes)] pub fn pattern_matching_relative_path<'a>( &mut self, relative_path: impl Into<&'a BStr>, @@ -115,7 +114,6 @@ impl<'repo> Pathspec<'repo> { /// The simplified version of [`pattern_matching_relative_path()`](Self::pattern_matching_relative_path()) which returns /// `true` if `relative_path` is included in the set of positive pathspecs, while not being excluded. #[momo] - #[allow(clippy::needless_lifetimes)] pub fn is_included<'a>(&mut self, relative_path: impl Into<&'a BStr>, is_dir: Option) -> bool { self.pattern_matching_relative_path(relative_path, is_dir) .map_or(false, |m| !m.is_excluded()) diff --git a/gix/src/repository/revision.rs b/gix/src/repository/revision.rs index 27aeb38e194..bb9b56d57ac 100644 --- a/gix/src/repository/revision.rs +++ b/gix/src/repository/revision.rs @@ -10,7 +10,6 @@ impl crate::Repository { /// - `@` actually stands for `HEAD`, whereas `git` resolves it to the object pointed to by `HEAD` without making the /// `HEAD` ref available for lookups. #[doc(alias = "revparse", alias = "git2")] - #[allow(clippy::needless_lifetimes)] #[momo] pub fn rev_parse<'a>(&self, spec: impl Into<&'a BStr>) -> Result, revision::spec::parse::Error> { revision::Spec::from_bstr( From 86b8e50fafa7e5d57989acb9e8b848fd95d271a9 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 29 Aug 2023 21:47:44 +1000 Subject: [PATCH 15/29] Remove unnecessary change in `repository/config/transport.rs` Signed-off-by: Jiahao XU --- gix/src/repository/config/transport.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gix/src/repository/config/transport.rs b/gix/src/repository/config/transport.rs index 811848a5dc9..99b5a7f47ff 100644 --- a/gix/src/repository/config/transport.rs +++ b/gix/src/repository/config/transport.rs @@ -273,7 +273,7 @@ impl crate::Repository { .proxy .as_deref() .filter(|url| !url.is_empty()) - .map(|url| gix_url::parse(<&BStr>::from(url))) + .map(|url| gix_url::parse(url.into())) .transpose()? .filter(|url| url.user().is_some()) .map(|url| -> Result<_, config::transport::http::Error> { From b5f78be06792153cd981c316a486974c000f1fd8 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 29 Aug 2023 22:51:59 +1000 Subject: [PATCH 16/29] feat `momo`: Support parsing pattern in params So that we can slap it on more functions. Signed-off-by: Jiahao XU --- gix-macros/src/lib.rs | 118 ++++++++++++++++++++++++--------------- gix-macros/tests/test.rs | 19 +++++++ 2 files changed, 91 insertions(+), 46 deletions(-) diff --git a/gix-macros/src/lib.rs b/gix-macros/src/lib.rs index f2503e9b332..e6c9b4dfe79 100644 --- a/gix-macros/src/lib.rs +++ b/gix-macros/src/lib.rs @@ -13,7 +13,7 @@ enum Conversion<'a> { } impl<'a> Conversion<'a> { - fn conversion_expr(&self, i: Ident) -> Expr { + fn conversion_expr(&self, i: &Ident) -> Expr { match *self { Conversion::Into(_) => parse_quote!(#i.into()), Conversion::AsRef(_) => parse_quote!(#i.as_ref()), @@ -23,12 +23,10 @@ impl<'a> Conversion<'a> { } fn parse_bounded_type(ty: &Type) -> Option { - if let Type::Path(TypePath { qself: None, ref path }) = ty { - if path.segments.len() == 1 { - return Some(path.segments[0].ident.clone()); - } + match &ty { + Type::Path(TypePath { qself: None, path }) if path.segments.len() == 1 => Some(path.segments[0].ident.clone()), + _ => None, } - None } fn parse_bounds(bounds: &Punctuated) -> Option { @@ -60,7 +58,6 @@ fn parse_generics(decl: &Signature) -> HashMap> { if let GenericParam::Type(ref tp) = gp { if let Some(conversion) = parse_bounds(&tp.bounds) { ty_conversions.insert(tp.ident.clone(), conversion); - continue; } } } @@ -70,7 +67,6 @@ fn parse_generics(decl: &Signature) -> HashMap> { if let Some(ident) = parse_bounded_type(&pt.bounded_ty) { if let Some(conversion) = parse_bounds(&pt.bounds) { ty_conversions.insert(ident, conversion); - continue; } } } @@ -79,51 +75,76 @@ fn parse_generics(decl: &Signature) -> HashMap> { ty_conversions } -fn pat_to_ident(pat: &Pat) -> Ident { - if let Pat::Ident(ref pat_ident) = *pat { - return pat_ident.ident.clone(); - } - unimplemented!("No non-ident patterns for now!"); -} - -fn pat_to_expr(pat: &Pat) -> Expr { - let ident = pat_to_ident(pat); - parse_quote!(#ident) -} - fn convert<'a>( inputs: &'a Punctuated, ty_conversions: &HashMap>, -) -> (Punctuated, bool) { +) -> (Punctuated, Punctuated, bool) { + let mut argtypes = Punctuated::new(); let mut argexprs = Punctuated::new(); let mut has_self = false; - inputs.iter().for_each(|input| match input { - FnArg::Receiver(..) => { + inputs.iter().enumerate().for_each(|(i, input)| match input.clone() { + FnArg::Receiver(receiver) => { has_self = true; + argtypes.push(FnArg::Receiver(receiver)); } - FnArg::Typed(PatType { ref pat, ref ty, .. }) => match **ty { - Type::ImplTrait(TypeImplTrait { ref bounds, .. }) => { - if let Some(conv) = parse_bounds(bounds) { - let ident = pat_to_ident(pat); - argexprs.push(conv.conversion_expr(ident)); - } else { - argexprs.push(pat_to_expr(pat)); + FnArg::Typed(mut pat_type) => { + let pat_ident = match &mut *pat_type.pat { + Pat::Ident(pat_ident) if pat_ident.by_ref.is_none() && pat_ident.subpat.is_none() => pat_ident, + _ => { + pat_type.pat = Box::new(Pat::Ident(PatIdent { + ident: Ident::new(&format!("arg_{i}_gen_by_momo_"), proc_macro2::Span::call_site()), + attrs: Default::default(), + by_ref: None, + mutability: None, + subpat: None, + })); + + if let Pat::Ident(pat_ident) = &mut *pat_type.pat { + pat_ident + } else { + panic!() + } } - } - Type::Path(..) => { - if let Some(conv) = parse_bounded_type(ty).and_then(|ident| ty_conversions.get(&ident)) { - let ident = pat_to_ident(pat); - argexprs.push(conv.conversion_expr(ident)); - } else { - argexprs.push(pat_to_expr(pat)); + }; + // Outer function type argument pat does not need mut unless its + // type is `impl AsMut`. + pat_ident.mutability = None; + + let ident = &pat_ident.ident; + + let to_expr = || parse_quote!(#ident); + + match *pat_type.ty { + Type::ImplTrait(TypeImplTrait { ref bounds, .. }) => { + if let Some(conv) = parse_bounds(bounds) { + argexprs.push(conv.conversion_expr(ident)); + if let Conversion::AsMut(_) = conv { + pat_ident.mutability = Some(Default::default()); + } + } else { + argexprs.push(to_expr()); + } + } + Type::Path(..) => { + if let Some(conv) = parse_bounded_type(&pat_type.ty).and_then(|ident| ty_conversions.get(&ident)) { + argexprs.push(conv.conversion_expr(ident)); + if let Conversion::AsMut(_) = conv { + pat_ident.mutability = Some(Default::default()); + } + } else { + argexprs.push(to_expr()); + } + } + _ => { + argexprs.push(to_expr()); } } - _ => { - argexprs.push(pat_to_expr(pat)); - } - }, + + // Now that mutability is decided, push the type into argtypes + argtypes.push(FnArg::Typed(pat_type)); + } }); - (argexprs, has_self) + (argtypes, argexprs, has_self) } fn contains_self_type_path(path: &Path) -> bool { @@ -199,7 +220,7 @@ fn momo_inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream { if let Item::Fn(item_fn) = fn_item { let ty_conversions = parse_generics(&item_fn.sig); - let (argexprs, has_self) = convert(&item_fn.sig.inputs, &ty_conversions); + let (argtypes, argexprs, has_self) = convert(&item_fn.sig.inputs, &ty_conversions); let uses_self = has_self || item_fn.sig.inputs.iter().any(has_self_type) @@ -211,6 +232,11 @@ fn momo_inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream { proc_macro2::Span::call_site(), ); + let outer_sig = Signature { + inputs: argtypes, + ..item_fn.sig.clone() + }; + let new_inner_item = Item::Fn(ItemFn { // Remove doc comment since they will increase compile-time and // also generates duplicate warning/error messages for the doc, @@ -226,7 +252,7 @@ fn momo_inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream { vis: Visibility::Inherited, sig: Signature { ident: inner_ident.clone(), - ..item_fn.sig.clone() + ..item_fn.sig }, block: item_fn.block, }); @@ -243,7 +269,7 @@ fn momo_inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream { let new_item = Item::Fn(ItemFn { attrs: item_fn.attrs, vis: item_fn.vis, - sig: item_fn.sig, + sig: outer_sig, block: if has_self { parse_quote!({ self.#inner_ident(#argexprs) }) } else { @@ -258,7 +284,7 @@ fn momo_inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream { let new_item = Item::Fn(ItemFn { attrs: item_fn.attrs, vis: item_fn.vis, - sig: item_fn.sig, + sig: outer_sig, block: parse_quote!({ #[allow(unused_mut)] #new_inner_item diff --git a/gix-macros/tests/test.rs b/gix-macros/tests/test.rs index 10006bf1be4..51c522c1fae 100644 --- a/gix-macros/tests/test.rs +++ b/gix-macros/tests/test.rs @@ -192,6 +192,25 @@ impl TryInto for S { } } +#[allow(unused)] +#[momo] +fn test_fn_pat( + a: impl Into, + b: impl AsRef, + mut c: impl AsMut, + d: impl TryInto, + S(_g): S, +) -> Result<(), E> { + let mut s = a.into(); + s += b.as_ref(); + s += c.as_mut(); + s += &d.try_into()?; + + drop(s); + + Ok(()) +} + #[test] fn test_basic_fn() { assert_eq!( From b6194568e1d3042305f472103e1c00549cc4ccb9 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 29 Aug 2023 22:56:31 +1000 Subject: [PATCH 17/29] feat `momo`: Rm unnecessary `#[allow(unused_mut)]` on generated inner fn Now that the inner function is actually identical to the original function before passed into `momo`, except for visibility, function name and the lack of function doc. Signed-off-by: Jiahao XU --- gix-macros/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gix-macros/src/lib.rs b/gix-macros/src/lib.rs index e6c9b4dfe79..dcfa6ad5d70 100644 --- a/gix-macros/src/lib.rs +++ b/gix-macros/src/lib.rs @@ -276,7 +276,7 @@ fn momo_inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream { parse_quote!({ Self::#inner_ident(#argexprs) }) }, }); - quote!(#new_item #[allow(unused_mut)] #new_inner_item) + quote!(#new_item #new_inner_item) } else { // Put the new inner function within the function block // to avoid duplicate function name and support associated @@ -286,7 +286,6 @@ fn momo_inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream { vis: item_fn.vis, sig: outer_sig, block: parse_quote!({ - #[allow(unused_mut)] #new_inner_item #inner_ident(#argexprs) From 89ae797c7f1f4d26c48ed54c5d8b31f39599f063 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 29 Aug 2023 23:00:08 +1000 Subject: [PATCH 18/29] Rm unnecessary `#[allow(unused_mut)]` put on `momo`ed functions Signed-off-by: Jiahao XU --- gix/src/discover.rs | 1 - gix/src/init.rs | 1 - gix/src/open/repository.rs | 1 - 3 files changed, 3 deletions(-) diff --git a/gix/src/discover.rs b/gix/src/discover.rs index a5a3b8a4ad4..50dcb9fa773 100644 --- a/gix/src/discover.rs +++ b/gix/src/discover.rs @@ -64,7 +64,6 @@ impl ThreadSafeRepository { /// Finally, use the `trust_map` to determine which of our own repository options to use /// based on the trust level of the effective repository directory. #[momo] - #[allow(unused_mut)] pub fn discover_with_environment_overrides_opts( directory: impl AsRef, mut options: upwards::Options<'_>, diff --git a/gix/src/init.rs b/gix/src/init.rs index 9b9cb5a61b0..21c2debd807 100644 --- a/gix/src/init.rs +++ b/gix/src/init.rs @@ -59,7 +59,6 @@ impl ThreadSafeRepository { /// Instead of naming the default branch `master`, we name it `main` unless configured explicitly using the `init.defaultBranch` /// configuration key. #[momo] - #[allow(unused_mut)] pub fn init_opts( directory: impl AsRef, kind: crate::create::Kind, diff --git a/gix/src/open/repository.rs b/gix/src/open/repository.rs index df096a4e3dd..2f9127bbd7d 100644 --- a/gix/src/open/repository.rs +++ b/gix/src/open/repository.rs @@ -60,7 +60,6 @@ impl ThreadSafeRepository { /// Note that opening a repository for implementing custom hooks is also handle specifically in /// [`open_with_environment_overrides()`][Self::open_with_environment_overrides()]. #[momo] - #[allow(unused_mut)] pub fn open_opts(path: impl Into, mut options: Options) -> Result { let _span = gix_trace::coarse!("ThreadSafeRepository::open()"); let (path, kind) = { From cd3c2893b095d38d36f0c969549f0aaadfcef2ee Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 29 Aug 2023 23:06:42 +1000 Subject: [PATCH 19/29] Apply `momo` to mod `gix::create::into` Signed-off-by: Jiahao XU --- gix/src/create.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gix/src/create.rs b/gix/src/create.rs index 4cc01402c07..c3839f66fcf 100644 --- a/gix/src/create.rs +++ b/gix/src/create.rs @@ -7,6 +7,7 @@ use std::{ use gix_config::parse::section; use gix_discover::DOT_GIT_DIR; +use gix_macros::momo; /// The error used in [`into()`]. #[derive(Debug, thiserror::Error)] @@ -124,6 +125,7 @@ pub struct Options { /// Note that this is a simple template-based initialization routine which should be accompanied with additional corrections /// to respect git configuration, which is accomplished by [its callers][crate::ThreadSafeRepository::init_opts()] /// that return a [Repository][crate::Repository]. +#[momo] pub fn into( directory: impl Into, kind: Kind, From 25912fe1e5d60765458a2e90c0fa487657b0831c Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 29 Aug 2023 23:13:21 +1000 Subject: [PATCH 20/29] Apply `momo` to mod `config::snapshot::access` Signed-off-by: Jiahao XU --- gix/src/config/snapshot/access.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gix/src/config/snapshot/access.rs b/gix/src/config/snapshot/access.rs index 284fe0fb843..0a82d966c00 100644 --- a/gix/src/config/snapshot/access.rs +++ b/gix/src/config/snapshot/access.rs @@ -2,6 +2,7 @@ use std::borrow::Cow; use gix_features::threading::OwnShared; +use gix_macros::momo; use crate::{ bstr::{BStr, BString}, @@ -25,6 +26,7 @@ impl<'repo> Snapshot<'repo> { } /// Like [`boolean()`][Self::boolean()], but it will report an error if the value couldn't be interpreted as boolean. + #[momo] pub fn try_boolean<'a>(&self, key: impl Into<&'a BStr>) -> Option> { self.repo.config.resolved.boolean_by_key(key) } @@ -40,6 +42,7 @@ impl<'repo> Snapshot<'repo> { } /// Like [`integer()`][Self::integer()], but it will report an error if the value couldn't be interpreted as boolean. + #[momo] pub fn try_integer<'a>(&self, key: impl Into<&'a BStr>) -> Option> { self.repo.config.resolved.integer_by_key(key) } @@ -47,6 +50,7 @@ impl<'repo> Snapshot<'repo> { /// Return the string at `key`, or `None` if there is no such value. /// /// Note that this method takes the most recent value at `key` even if it is from a file with reduced trust. + #[momo] pub fn string<'a>(&self, key: impl Into<&'a BStr>) -> Option> { self.repo.config.resolved.string_by_key(key) } @@ -54,6 +58,7 @@ impl<'repo> Snapshot<'repo> { /// Return the trusted and fully interpolated path at `key`, or `None` if there is no such value /// or if no value was found in a trusted file. /// An error occurs if the path could not be interpolated to its final value. + #[momo] pub fn trusted_path<'a>( &self, key: impl Into<&'a BStr>, @@ -101,6 +106,7 @@ impl<'repo> SnapshotMut<'repo> { /// Set the value at `key` to `new_value`, possibly creating the section if it doesn't exist yet, or overriding the most recent existing /// value, which will be returned. + #[momo] pub fn set_value<'b>( &mut self, key: &'static dyn crate::config::tree::Key, @@ -119,6 +125,7 @@ impl<'repo> SnapshotMut<'repo> { /// Set the value at `key` to `new_value` in the given `subsection`, possibly creating the section and sub-section if it doesn't exist yet, /// or overriding the most recent existing value, which will be returned. + #[momo] pub fn set_subsection_value<'a, 'b>( &mut self, key: &'static dyn crate::config::tree::Key, From 1d9030112b54699db9bd8d1125116e46c4a6f71e Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 29 Aug 2023 23:16:34 +1000 Subject: [PATCH 21/29] Apply `momo` to fn `gix::revision::Spec::from_bstr` Signed-off-by: Jiahao XU --- gix/src/revision/spec/parse/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gix/src/revision/spec/parse/mod.rs b/gix/src/revision/spec/parse/mod.rs index f69ecc4afb0..950dfa0040b 100644 --- a/gix/src/revision/spec/parse/mod.rs +++ b/gix/src/revision/spec/parse/mod.rs @@ -1,6 +1,7 @@ use std::collections::HashSet; use gix_hash::ObjectId; +use gix_macros::momo; use gix_revision::spec::parse; use crate::{bstr::BStr, revision::Spec, Repository}; @@ -30,6 +31,7 @@ impl<'repo> Spec<'repo> { /// Parse `spec` and use information from `repo` to resolve it, using `opts` to learn how to deal with ambiguity. /// /// Note that it's easier and to use [`repo.rev_parse()`][Repository::rev_parse()] instead. + #[momo] pub fn from_bstr<'a>(spec: impl Into<&'a BStr>, repo: &'repo Repository, opts: Options) -> Result { let mut delegate = Delegate::new(repo, opts); match gix_revision::spec::parse(spec.into(), &mut delegate) { From 875c28757e4a91cf314ec59dd1a0dde779698e53 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 29 Aug 2023 23:18:54 +1000 Subject: [PATCH 22/29] Apply `momo` to fn `gix::Remote::save_as_to` Signed-off-by: Jiahao XU --- gix/src/remote/save.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gix/src/remote/save.rs b/gix/src/remote/save.rs index a61df64c0d6..2a91dfa9c1d 100644 --- a/gix/src/remote/save.rs +++ b/gix/src/remote/save.rs @@ -1,5 +1,7 @@ use std::convert::TryInto; +use gix_macros::momo; + use crate::{ bstr::{BStr, BString}, config, remote, Remote, @@ -111,6 +113,7 @@ impl Remote<'_> { /// If this name is different from the current one, the git configuration will still contain the previous name, /// and the caller should account for that. #[allow(clippy::result_large_err)] + #[momo] pub fn save_as_to( &mut self, name: impl Into, From 48a20888d158b94811074a09a8c57ff5c8410769 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 2 Sep 2023 19:37:51 +0200 Subject: [PATCH 23/29] refactor --- gix-macros/src/lib.rs | 293 +--------------------- gix-macros/src/momo.rs | 290 +++++++++++++++++++++ gix-macros/tests/macros.rs | 1 + gix-macros/tests/{test.rs => momo/mod.rs} | 7 +- 4 files changed, 297 insertions(+), 294 deletions(-) create mode 100644 gix-macros/src/momo.rs create mode 100644 gix-macros/tests/macros.rs rename gix-macros/tests/{test.rs => momo/mod.rs} (98%) diff --git a/gix-macros/src/lib.rs b/gix-macros/src/lib.rs index dcfa6ad5d70..dee0d581a7c 100644 --- a/gix-macros/src/lib.rs +++ b/gix-macros/src/lib.rs @@ -1,299 +1,12 @@ -use std::collections::HashMap; - +//! A crate of use proc_macro::TokenStream; -use quote::quote; -use syn::{punctuated::Punctuated, spanned::Spanned, *}; - -#[derive(Copy, Clone)] -// All conversions we support. Check references to this type for an idea how to add more. -enum Conversion<'a> { - Into(&'a Type), - AsRef(&'a Type), - AsMut(&'a Type), -} - -impl<'a> Conversion<'a> { - fn conversion_expr(&self, i: &Ident) -> Expr { - match *self { - Conversion::Into(_) => parse_quote!(#i.into()), - Conversion::AsRef(_) => parse_quote!(#i.as_ref()), - Conversion::AsMut(_) => parse_quote!(#i.as_mut()), - } - } -} - -fn parse_bounded_type(ty: &Type) -> Option { - match &ty { - Type::Path(TypePath { qself: None, path }) if path.segments.len() == 1 => Some(path.segments[0].ident.clone()), - _ => None, - } -} - -fn parse_bounds(bounds: &Punctuated) -> Option { - if bounds.len() != 1 { - return None; - } - if let TypeParamBound::Trait(ref tb) = bounds.first().unwrap() { - if let Some(seg) = tb.path.segments.iter().last() { - if let PathArguments::AngleBracketed(ref gen_args) = seg.arguments { - if let GenericArgument::Type(ref arg_ty) = gen_args.args.first().unwrap() { - if seg.ident == "Into" { - return Some(Conversion::Into(arg_ty)); - } else if seg.ident == "AsRef" { - return Some(Conversion::AsRef(arg_ty)); - } else if seg.ident == "AsMut" { - return Some(Conversion::AsMut(arg_ty)); - } - } - } - } - } - None -} - -// create a map from generic type to Conversion -fn parse_generics(decl: &Signature) -> HashMap> { - let mut ty_conversions = HashMap::new(); - for gp in decl.generics.params.iter() { - if let GenericParam::Type(ref tp) = gp { - if let Some(conversion) = parse_bounds(&tp.bounds) { - ty_conversions.insert(tp.ident.clone(), conversion); - } - } - } - if let Some(ref wc) = decl.generics.where_clause { - for wp in wc.predicates.iter() { - if let WherePredicate::Type(ref pt) = wp { - if let Some(ident) = parse_bounded_type(&pt.bounded_ty) { - if let Some(conversion) = parse_bounds(&pt.bounds) { - ty_conversions.insert(ident, conversion); - } - } - } - } - } - ty_conversions -} - -fn convert<'a>( - inputs: &'a Punctuated, - ty_conversions: &HashMap>, -) -> (Punctuated, Punctuated, bool) { - let mut argtypes = Punctuated::new(); - let mut argexprs = Punctuated::new(); - let mut has_self = false; - inputs.iter().enumerate().for_each(|(i, input)| match input.clone() { - FnArg::Receiver(receiver) => { - has_self = true; - argtypes.push(FnArg::Receiver(receiver)); - } - FnArg::Typed(mut pat_type) => { - let pat_ident = match &mut *pat_type.pat { - Pat::Ident(pat_ident) if pat_ident.by_ref.is_none() && pat_ident.subpat.is_none() => pat_ident, - _ => { - pat_type.pat = Box::new(Pat::Ident(PatIdent { - ident: Ident::new(&format!("arg_{i}_gen_by_momo_"), proc_macro2::Span::call_site()), - attrs: Default::default(), - by_ref: None, - mutability: None, - subpat: None, - })); - - if let Pat::Ident(pat_ident) = &mut *pat_type.pat { - pat_ident - } else { - panic!() - } - } - }; - // Outer function type argument pat does not need mut unless its - // type is `impl AsMut`. - pat_ident.mutability = None; - - let ident = &pat_ident.ident; - - let to_expr = || parse_quote!(#ident); - - match *pat_type.ty { - Type::ImplTrait(TypeImplTrait { ref bounds, .. }) => { - if let Some(conv) = parse_bounds(bounds) { - argexprs.push(conv.conversion_expr(ident)); - if let Conversion::AsMut(_) = conv { - pat_ident.mutability = Some(Default::default()); - } - } else { - argexprs.push(to_expr()); - } - } - Type::Path(..) => { - if let Some(conv) = parse_bounded_type(&pat_type.ty).and_then(|ident| ty_conversions.get(&ident)) { - argexprs.push(conv.conversion_expr(ident)); - if let Conversion::AsMut(_) = conv { - pat_ident.mutability = Some(Default::default()); - } - } else { - argexprs.push(to_expr()); - } - } - _ => { - argexprs.push(to_expr()); - } - } - - // Now that mutability is decided, push the type into argtypes - argtypes.push(FnArg::Typed(pat_type)); - } - }); - (argtypes, argexprs, has_self) -} - -fn contains_self_type_path(path: &Path) -> bool { - path.segments.iter().any(|segment| { - segment.ident == "Self" - || match &segment.arguments { - PathArguments::AngleBracketed(AngleBracketedGenericArguments { args, .. }) => { - args.iter().any(|generic_arg| match generic_arg { - GenericArgument::Type(ty) => contains_self_type(ty), - GenericArgument::Const(expr) => contains_self_type_expr(expr), - _ => false, - }) - } - PathArguments::Parenthesized(ParenthesizedGenericArguments { inputs, output, .. }) => { - inputs.iter().any(contains_self_type) - || matches!(output, ReturnType::Type(_, ty) if contains_self_type(ty)) - } - _ => false, - } - }) -} - -fn contains_self_type_expr(expr: &Expr) -> bool { - match expr { - Expr::Path(ExprPath { qself: Some(_), .. }) => true, - Expr::Path(ExprPath { path, .. }) => contains_self_type_path(path), - _ => false, - } -} - -fn contains_self_type(input: &Type) -> bool { - match input { - Type::Array(TypeArray { elem, len, .. }) => { - // Call `matches!` first so that we can do tail call here - // as an optimization. - contains_self_type_expr(len) || contains_self_type(elem) - } - Type::Group(TypeGroup { elem, .. }) => contains_self_type(elem), - Type::Paren(TypeParen { elem, .. }) => contains_self_type(elem), - Type::Ptr(TypePtr { elem, .. }) => contains_self_type(elem), - Type::Reference(TypeReference { elem, .. }) => contains_self_type(elem), - Type::Slice(TypeSlice { elem, .. }) => contains_self_type(elem), - - Type::Tuple(TypeTuple { elems, .. }) => elems.iter().any(contains_self_type), - - Type::Path(TypePath { qself: Some(_), .. }) => true, - Type::Path(TypePath { path, .. }) => contains_self_type_path(path), - - _ => false, - } -} - -fn has_self_type(input: &FnArg) -> bool { - match input { - FnArg::Receiver(_) => true, - FnArg::Typed(PatType { ty, .. }) => contains_self_type(ty), - } -} /// Generate lightweight monomorphized wrapper around main implementation. /// May be applied to functions and methods. #[proc_macro_attribute] pub fn momo(_attrs: TokenStream, input: TokenStream) -> TokenStream { //TODO: alternatively parse ImplItem::Method - momo_inner(input.into()).into() + momo::inner(input.into()).into() } -fn momo_inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream { - let fn_item: Item = match syn::parse2(code.clone()) { - Ok(input) => input, - Err(err) => return err.to_compile_error(), - }; - - if let Item::Fn(item_fn) = fn_item { - let ty_conversions = parse_generics(&item_fn.sig); - let (argtypes, argexprs, has_self) = convert(&item_fn.sig.inputs, &ty_conversions); - - let uses_self = has_self - || item_fn.sig.inputs.iter().any(has_self_type) - || matches!(&item_fn.sig.output, ReturnType::Type(_, ty) if contains_self_type(ty)); - - let inner_ident = Ident::new( - // Use long qualifier to avoid name colision. - &format!("_{}_inner_generated_by_gix_macro_momo", item_fn.sig.ident), - proc_macro2::Span::call_site(), - ); - - let outer_sig = Signature { - inputs: argtypes, - ..item_fn.sig.clone() - }; - - let new_inner_item = Item::Fn(ItemFn { - // Remove doc comment since they will increase compile-time and - // also generates duplicate warning/error messages for the doc, - // especially if it contains doc-tests. - attrs: { - let mut attrs = item_fn.attrs.clone(); - attrs.retain(|attr| { - let segments = &attr.path().segments; - !(segments.len() == 1 && segments[0].ident == "doc") - }); - attrs - }, - vis: Visibility::Inherited, - sig: Signature { - ident: inner_ident.clone(), - ..item_fn.sig - }, - block: item_fn.block, - }); - - if uses_self { - // We can use `self` or `Self` inside function defined within - // the impl-fn, so instead declare two separate functions. - // - // Since it's an impl block, it's unlikely to have name conflict, - // though this won't work for impl-trait. - // - // This approach also make sure we can call the right function - // using `Self` qualifier. - let new_item = Item::Fn(ItemFn { - attrs: item_fn.attrs, - vis: item_fn.vis, - sig: outer_sig, - block: if has_self { - parse_quote!({ self.#inner_ident(#argexprs) }) - } else { - parse_quote!({ Self::#inner_ident(#argexprs) }) - }, - }); - quote!(#new_item #new_inner_item) - } else { - // Put the new inner function within the function block - // to avoid duplicate function name and support associated - // function that doesn't use `self` or `Self`. - let new_item = Item::Fn(ItemFn { - attrs: item_fn.attrs, - vis: item_fn.vis, - sig: outer_sig, - block: parse_quote!({ - #new_inner_item - - #inner_ident(#argexprs) - }), - }); - quote!(#new_item) - } - } else { - Error::new(fn_item.span(), "expect a function").to_compile_error() - } -} +mod momo; diff --git a/gix-macros/src/momo.rs b/gix-macros/src/momo.rs new file mode 100644 index 00000000000..37d52c55e09 --- /dev/null +++ b/gix-macros/src/momo.rs @@ -0,0 +1,290 @@ +use std::collections::HashMap; + +use quote::quote; +use syn::{punctuated::Punctuated, spanned::Spanned, *}; + +pub(crate) fn inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream { + let fn_item: Item = match syn::parse2(code.clone()) { + Ok(input) => input, + Err(err) => return err.to_compile_error(), + }; + + if let Item::Fn(item_fn) = fn_item { + let ty_conversions = parse_generics(&item_fn.sig); + let (argtypes, argexprs, has_self) = convert(&item_fn.sig.inputs, &ty_conversions); + + let uses_self = has_self + || item_fn.sig.inputs.iter().any(has_self_type) + || matches!(&item_fn.sig.output, ReturnType::Type(_, ty) if contains_self_type(ty)); + + let inner_ident = Ident::new( + // Use long qualifier to avoid name collision. + &format!("_{}_inner_generated_by_gix_macro_momo", item_fn.sig.ident), + proc_macro2::Span::call_site(), + ); + + let outer_sig = Signature { + inputs: argtypes, + ..item_fn.sig.clone() + }; + + let new_inner_item = Item::Fn(ItemFn { + // Remove doc comment since they will increase compile-time and + // also generates duplicate warning/error messages for the doc, + // especially if it contains doc-tests. + attrs: { + let mut attrs = item_fn.attrs.clone(); + attrs.retain(|attr| { + let segments = &attr.path().segments; + !(segments.len() == 1 && segments[0].ident == "doc") + }); + attrs + }, + vis: Visibility::Inherited, + sig: Signature { + ident: inner_ident.clone(), + ..item_fn.sig + }, + block: item_fn.block, + }); + + if uses_self { + // We can use `self` or `Self` inside function defined within + // the impl-fn, so instead declare two separate functions. + // + // Since it's an impl block, it's unlikely to have name conflict, + // though this won't work for impl-trait. + // + // This approach also make sure we can call the right function + // using `Self` qualifier. + let new_item = Item::Fn(ItemFn { + attrs: item_fn.attrs, + vis: item_fn.vis, + sig: outer_sig, + block: if has_self { + parse_quote!({ self.#inner_ident(#argexprs) }) + } else { + parse_quote!({ Self::#inner_ident(#argexprs) }) + }, + }); + quote!(#new_item #new_inner_item) + } else { + // Put the new inner function within the function block + // to avoid duplicate function name and support associated + // function that doesn't use `self` or `Self`. + let new_item = Item::Fn(ItemFn { + attrs: item_fn.attrs, + vis: item_fn.vis, + sig: outer_sig, + block: parse_quote!({ + #new_inner_item + + #inner_ident(#argexprs) + }), + }); + quote!(#new_item) + } + } else { + Error::new(fn_item.span(), "expect a function").to_compile_error() + } +} + +#[derive(Copy, Clone)] +// All conversions we support. Check references to this type for an idea how to add more. +enum Conversion<'a> { + Into(&'a Type), + AsRef(&'a Type), + AsMut(&'a Type), +} + +impl<'a> Conversion<'a> { + fn conversion_expr(&self, i: &Ident) -> Expr { + match *self { + Conversion::Into(_) => parse_quote!(#i.into()), + Conversion::AsRef(_) => parse_quote!(#i.as_ref()), + Conversion::AsMut(_) => parse_quote!(#i.as_mut()), + } + } +} + +fn parse_bounded_type(ty: &Type) -> Option { + match &ty { + Type::Path(TypePath { qself: None, path }) if path.segments.len() == 1 => Some(path.segments[0].ident.clone()), + _ => None, + } +} + +fn parse_bounds(bounds: &Punctuated) -> Option { + if bounds.len() != 1 { + return None; + } + if let TypeParamBound::Trait(ref tb) = bounds.first().unwrap() { + if let Some(seg) = tb.path.segments.iter().last() { + if let PathArguments::AngleBracketed(ref gen_args) = seg.arguments { + if let GenericArgument::Type(ref arg_ty) = gen_args.args.first().unwrap() { + if seg.ident == "Into" { + return Some(Conversion::Into(arg_ty)); + } else if seg.ident == "AsRef" { + return Some(Conversion::AsRef(arg_ty)); + } else if seg.ident == "AsMut" { + return Some(Conversion::AsMut(arg_ty)); + } + } + } + } + } + None +} + +// create a map from generic type to Conversion +fn parse_generics(decl: &Signature) -> HashMap> { + let mut ty_conversions = HashMap::new(); + for gp in decl.generics.params.iter() { + if let GenericParam::Type(ref tp) = gp { + if let Some(conversion) = parse_bounds(&tp.bounds) { + ty_conversions.insert(tp.ident.clone(), conversion); + } + } + } + if let Some(ref wc) = decl.generics.where_clause { + for wp in wc.predicates.iter() { + if let WherePredicate::Type(ref pt) = wp { + if let Some(ident) = parse_bounded_type(&pt.bounded_ty) { + if let Some(conversion) = parse_bounds(&pt.bounds) { + ty_conversions.insert(ident, conversion); + } + } + } + } + } + ty_conversions +} + +fn convert<'a>( + inputs: &'a Punctuated, + ty_conversions: &HashMap>, +) -> (Punctuated, Punctuated, bool) { + let mut argtypes = Punctuated::new(); + let mut argexprs = Punctuated::new(); + let mut has_self = false; + inputs.iter().enumerate().for_each(|(i, input)| match input.clone() { + FnArg::Receiver(receiver) => { + has_self = true; + argtypes.push(FnArg::Receiver(receiver)); + } + FnArg::Typed(mut pat_type) => { + let pat_ident = match &mut *pat_type.pat { + Pat::Ident(pat_ident) if pat_ident.by_ref.is_none() && pat_ident.subpat.is_none() => pat_ident, + _ => { + pat_type.pat = Box::new(Pat::Ident(PatIdent { + ident: Ident::new(&format!("arg_{i}_gen_by_momo_"), proc_macro2::Span::call_site()), + attrs: Default::default(), + by_ref: None, + mutability: None, + subpat: None, + })); + + if let Pat::Ident(pat_ident) = &mut *pat_type.pat { + pat_ident + } else { + panic!() + } + } + }; + // Outer function type argument pat does not need mut unless its + // type is `impl AsMut`. + pat_ident.mutability = None; + + let ident = &pat_ident.ident; + + let to_expr = || parse_quote!(#ident); + + match *pat_type.ty { + Type::ImplTrait(TypeImplTrait { ref bounds, .. }) => { + if let Some(conv) = parse_bounds(bounds) { + argexprs.push(conv.conversion_expr(ident)); + if let Conversion::AsMut(_) = conv { + pat_ident.mutability = Some(Default::default()); + } + } else { + argexprs.push(to_expr()); + } + } + Type::Path(..) => { + if let Some(conv) = parse_bounded_type(&pat_type.ty).and_then(|ident| ty_conversions.get(&ident)) { + argexprs.push(conv.conversion_expr(ident)); + if let Conversion::AsMut(_) = conv { + pat_ident.mutability = Some(Default::default()); + } + } else { + argexprs.push(to_expr()); + } + } + _ => { + argexprs.push(to_expr()); + } + } + + // Now that mutability is decided, push the type into argtypes + argtypes.push(FnArg::Typed(pat_type)); + } + }); + (argtypes, argexprs, has_self) +} + +fn contains_self_type_path(path: &Path) -> bool { + path.segments.iter().any(|segment| { + segment.ident == "Self" + || match &segment.arguments { + PathArguments::AngleBracketed(AngleBracketedGenericArguments { args, .. }) => { + args.iter().any(|generic_arg| match generic_arg { + GenericArgument::Type(ty) => contains_self_type(ty), + GenericArgument::Const(expr) => contains_self_type_expr(expr), + _ => false, + }) + } + PathArguments::Parenthesized(ParenthesizedGenericArguments { inputs, output, .. }) => { + inputs.iter().any(contains_self_type) + || matches!(output, ReturnType::Type(_, ty) if contains_self_type(ty)) + } + _ => false, + } + }) +} + +fn contains_self_type_expr(expr: &Expr) -> bool { + match expr { + Expr::Path(ExprPath { qself: Some(_), .. }) => true, + Expr::Path(ExprPath { path, .. }) => contains_self_type_path(path), + _ => false, + } +} + +fn contains_self_type(input: &Type) -> bool { + match input { + Type::Array(TypeArray { elem, len, .. }) => { + // Call `matches!` first so that we can do tail call here + // as an optimization. + contains_self_type_expr(len) || contains_self_type(elem) + } + Type::Group(TypeGroup { elem, .. }) => contains_self_type(elem), + Type::Paren(TypeParen { elem, .. }) => contains_self_type(elem), + Type::Ptr(TypePtr { elem, .. }) => contains_self_type(elem), + Type::Reference(TypeReference { elem, .. }) => contains_self_type(elem), + Type::Slice(TypeSlice { elem, .. }) => contains_self_type(elem), + + Type::Tuple(TypeTuple { elems, .. }) => elems.iter().any(contains_self_type), + + Type::Path(TypePath { qself: Some(_), .. }) => true, + Type::Path(TypePath { path, .. }) => contains_self_type_path(path), + + _ => false, + } +} + +fn has_self_type(input: &FnArg) -> bool { + match input { + FnArg::Receiver(_) => true, + FnArg::Typed(PatType { ty, .. }) => contains_self_type(ty), + } +} diff --git a/gix-macros/tests/macros.rs b/gix-macros/tests/macros.rs new file mode 100644 index 00000000000..821fc9835b0 --- /dev/null +++ b/gix-macros/tests/macros.rs @@ -0,0 +1 @@ +mod momo; diff --git a/gix-macros/tests/test.rs b/gix-macros/tests/momo/mod.rs similarity index 98% rename from gix-macros/tests/test.rs rename to gix-macros/tests/momo/mod.rs index 51c522c1fae..fd4da846627 100644 --- a/gix-macros/tests/test.rs +++ b/gix-macros/tests/momo/mod.rs @@ -212,7 +212,7 @@ fn test_fn_pat( } #[test] -fn test_basic_fn() { +fn basic_fn() { assert_eq!( test_fn("12345", "12345", String::from("12345"), S(true)).unwrap(), "123451234512345!2345" @@ -236,8 +236,7 @@ fn test_basic_fn() { } #[test] -fn test_struct_method() { - // Test test_method +fn struct_method() { assert_eq!( TestStruct .test_method("12345", "12345", String::from("12345"), S(true)) @@ -263,7 +262,7 @@ fn test_struct_method() { } #[test] -fn test_struct_fn() { +fn struct_fn() { assert_eq!( TestStruct::test_fn("12345", "12345", String::from("12345"), S(true), ()).unwrap(), "123451234512345!2345" From 705f2f34f1fa95d767646b154f41d2a6ce65ad10 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 2 Sep 2023 20:02:10 +0200 Subject: [PATCH 24/29] improve docs --- gix-macros/src/lib.rs | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/gix-macros/src/lib.rs b/gix-macros/src/lib.rs index dee0d581a7c..1329a6b1526 100644 --- a/gix-macros/src/lib.rs +++ b/gix-macros/src/lib.rs @@ -1,8 +1,22 @@ -//! A crate of +//! A crate of useful macros used in `gix` primarily. +//! +//! Note that within `gix-*` crates, monomorphization should never be used for convenience, but only for performance +//! reasons. And in the latter case, manual denomophization should be considered if the trait in questions isn't called +//! often enough or measurements indicate that `&dyn Trait` is increasing the runtime. Thus, `gix-*` crates should probably +//! by default prefer using `&dyn` unless measurements indicate otherwise. use proc_macro::TokenStream; -/// Generate lightweight monomorphized wrapper around main implementation. -/// May be applied to functions and methods. +/// When applied to functions or methods, it will turn it into a wrapper that will immediately call +/// a de-monomorphized implementation (i.e. one that uses `&dyn Trait`). +/// +/// That way, the landing-pads for convenience will be as small as possible which then delegate to a single +/// function or method for implementation. +/// +/// The parameters using the following traits can be de-monomorphized: +/// +/// * `Into` +/// * `AsRef` +/// * `AsMut` #[proc_macro_attribute] pub fn momo(_attrs: TokenStream, input: TokenStream) -> TokenStream { //TODO: alternatively parse ImplItem::Method From c4ed7c180e3ec1ff75cb10d78d4b8eed3b75be2f Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 3 Sep 2023 07:37:31 +1000 Subject: [PATCH 25/29] Remove `TODO` in `gix-macros/src/lib.rs` Signed-off-by: Jiahao XU --- gix-macros/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/gix-macros/src/lib.rs b/gix-macros/src/lib.rs index 1329a6b1526..4693a79e824 100644 --- a/gix-macros/src/lib.rs +++ b/gix-macros/src/lib.rs @@ -19,7 +19,6 @@ use proc_macro::TokenStream; /// * `AsMut` #[proc_macro_attribute] pub fn momo(_attrs: TokenStream, input: TokenStream) -> TokenStream { - //TODO: alternatively parse ImplItem::Method momo::inner(input.into()).into() } From c8e732430f3740348ccedd0dc1a9a28b06a0adee Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 3 Sep 2023 19:14:23 +0200 Subject: [PATCH 26/29] Add test-suite for failure modes Also, discover that apparently `momo` isn't effective where it should be *or* the implementation is more complex. --- Cargo.lock | 31 +++++++++++++++++++ gix-macros/Cargo.toml | 3 ++ gix-macros/src/momo.rs | 8 +++++ gix-macros/tests/momo/mod.rs | 13 ++++++++ .../tests/momo/ux/error_if_ineffective.rs | 3 ++ gix-macros/tests/momo/ux/error_on_struct.rs | 5 +++ .../tests/momo/ux/error_on_struct.stderr | 5 +++ 7 files changed, 68 insertions(+) create mode 100644 gix-macros/tests/momo/ux/error_if_ineffective.rs create mode 100644 gix-macros/tests/momo/ux/error_on_struct.rs create mode 100644 gix-macros/tests/momo/ux/error_on_struct.stderr diff --git a/Cargo.lock b/Cargo.lock index e73076b78bc..7bbfd8861b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -294,6 +294,15 @@ version = "0.21.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "604178f6c5c21f02dc555784810edfb88d34ac2c73b2eae109655649ee73ce3d" +[[package]] +name = "basic-toml" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7bfc506e7a2370ec239e1d072507b2a80c833083699d3c6fa176fbb4de8448c6" +dependencies = [ + "serde", +] + [[package]] name = "bitflags" version = "1.3.2" @@ -1803,6 +1812,7 @@ dependencies = [ "proc-macro2", "quote", "syn 2.0.29", + "trybuild", ] [[package]] @@ -2575,6 +2585,12 @@ dependencies = [ "symlink", ] +[[package]] +name = "glob" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2fabcfbdc87f4758337ca535fb41a6d701b65693ce38287d856d1674551ec9b" + [[package]] name = "gloo-timers" version = "0.2.6" @@ -4428,6 +4444,21 @@ version = "0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3528ecfd12c466c6f163363caf2d02a71161dd5e1cc6ae7b34207ea2d42d81ed" +[[package]] +name = "trybuild" +version = "1.0.83" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6df60d81823ed9c520ee897489573da4b1d79ffbe006b8134f46de1a1aa03555" +dependencies = [ + "basic-toml", + "glob", + "once_cell", + "serde", + "serde_derive", + "serde_json", + "termcolor", +] + [[package]] name = "tui-react" version = "0.20.0" diff --git a/gix-macros/Cargo.toml b/gix-macros/Cargo.toml index d208d929c1f..2cb8bff3600 100644 --- a/gix-macros/Cargo.toml +++ b/gix-macros/Cargo.toml @@ -20,3 +20,6 @@ proc_macro = true syn = { version = "2.0", features = ["full", "fold"] } quote = "1.0" proc-macro2 = "1.0" + +[dev-dependencies] +trybuild = "1.0" diff --git a/gix-macros/src/momo.rs b/gix-macros/src/momo.rs index 37d52c55e09..3e0f2ad20eb 100644 --- a/gix-macros/src/momo.rs +++ b/gix-macros/src/momo.rs @@ -11,6 +11,14 @@ pub(crate) fn inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream if let Item::Fn(item_fn) = fn_item { let ty_conversions = parse_generics(&item_fn.sig); + // TODO: uncomment and see it fail in places where it should actually succeed. + // if ty_conversions.is_empty() { + // return Error::new( + // item_fn.span(), + // "Couldn't apply a single conversion - momo is ineffective here", + // ) + // .to_compile_error(); + // } let (argtypes, argexprs, has_self) = convert(&item_fn.sig.inputs, &ty_conversions); let uses_self = has_self diff --git a/gix-macros/tests/momo/mod.rs b/gix-macros/tests/momo/mod.rs index fd4da846627..2e79db76ece 100644 --- a/gix-macros/tests/momo/mod.rs +++ b/gix-macros/tests/momo/mod.rs @@ -302,3 +302,16 @@ fn struct_fn() { ) .unwrap_err(); } + +#[test] +fn ux() { + let t = trybuild::TestCases::new(); + t.compile_fail("tests/momo/ux/error_on_struct.rs"); +} + +#[test] +#[ignore = "needs work"] +fn ux_todo() { + let t = trybuild::TestCases::new(); + t.compile_fail("tests/momo/ux/error_if_ineffective.rs"); +} diff --git a/gix-macros/tests/momo/ux/error_if_ineffective.rs b/gix-macros/tests/momo/ux/error_if_ineffective.rs new file mode 100644 index 00000000000..cf41170c5da --- /dev/null +++ b/gix-macros/tests/momo/ux/error_if_ineffective.rs @@ -0,0 +1,3 @@ +/// If nothing is done, momo should fail. +#[gix_macros::momo] +fn main() {} diff --git a/gix-macros/tests/momo/ux/error_on_struct.rs b/gix-macros/tests/momo/ux/error_on_struct.rs new file mode 100644 index 00000000000..ff8db306c2f --- /dev/null +++ b/gix-macros/tests/momo/ux/error_on_struct.rs @@ -0,0 +1,5 @@ +fn main() {} + +/// Only functions work with momo +#[gix_macros::momo] +struct S; diff --git a/gix-macros/tests/momo/ux/error_on_struct.stderr b/gix-macros/tests/momo/ux/error_on_struct.stderr new file mode 100644 index 00000000000..e54864dcf8f --- /dev/null +++ b/gix-macros/tests/momo/ux/error_on_struct.stderr @@ -0,0 +1,5 @@ +error: expect a function + --> tests/momo/ux/error_on_struct.rs:3:1 + | +3 | /// Only functions work with momo + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 9be26220876498df4d1add77da45c415268a77dc Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Thu, 7 Sep 2023 22:03:32 +1000 Subject: [PATCH 27/29] Fix ui test `error_if_ineffective` `ty_conversions` only cover named generics, it doesn't cover `impl Trait`. I also changed the span for the error to `call_site`, otherwise it would show that the `compile_error!` comes from the comment, which is impossible. Signed-off-by: Jiahao XU --- gix-macros/src/momo.rs | 24 ++++++++++--------- gix-macros/tests/momo/mod.rs | 11 +-------- .../tests/momo/ux/error_if_ineffective.stderr | 13 ++++++++++ 3 files changed, 27 insertions(+), 21 deletions(-) create mode 100644 gix-macros/tests/momo/ux/error_if_ineffective.stderr diff --git a/gix-macros/src/momo.rs b/gix-macros/src/momo.rs index 3e0f2ad20eb..dea1a7ae03f 100644 --- a/gix-macros/src/momo.rs +++ b/gix-macros/src/momo.rs @@ -11,15 +11,14 @@ pub(crate) fn inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream if let Item::Fn(item_fn) = fn_item { let ty_conversions = parse_generics(&item_fn.sig); - // TODO: uncomment and see it fail in places where it should actually succeed. - // if ty_conversions.is_empty() { - // return Error::new( - // item_fn.span(), - // "Couldn't apply a single conversion - momo is ineffective here", - // ) - // .to_compile_error(); - // } - let (argtypes, argexprs, has_self) = convert(&item_fn.sig.inputs, &ty_conversions); + let (has_conversion_in_effect, argtypes, argexprs, has_self) = convert(&item_fn.sig.inputs, &ty_conversions); + if !has_conversion_in_effect { + return Error::new( + proc_macro2::Span::call_site(), + "Couldn't apply a single conversion - momo is ineffective here", + ) + .to_compile_error(); + } let uses_self = has_self || item_fn.sig.inputs.iter().any(has_self_type) @@ -171,7 +170,8 @@ fn parse_generics(decl: &Signature) -> HashMap> { fn convert<'a>( inputs: &'a Punctuated, ty_conversions: &HashMap>, -) -> (Punctuated, Punctuated, bool) { +) -> (bool, Punctuated, Punctuated, bool) { + let mut has_conversion_in_effect = false; let mut argtypes = Punctuated::new(); let mut argexprs = Punctuated::new(); let mut has_self = false; @@ -210,6 +210,7 @@ fn convert<'a>( match *pat_type.ty { Type::ImplTrait(TypeImplTrait { ref bounds, .. }) => { if let Some(conv) = parse_bounds(bounds) { + has_conversion_in_effect = true; argexprs.push(conv.conversion_expr(ident)); if let Conversion::AsMut(_) = conv { pat_ident.mutability = Some(Default::default()); @@ -220,6 +221,7 @@ fn convert<'a>( } Type::Path(..) => { if let Some(conv) = parse_bounded_type(&pat_type.ty).and_then(|ident| ty_conversions.get(&ident)) { + has_conversion_in_effect = true; argexprs.push(conv.conversion_expr(ident)); if let Conversion::AsMut(_) = conv { pat_ident.mutability = Some(Default::default()); @@ -237,7 +239,7 @@ fn convert<'a>( argtypes.push(FnArg::Typed(pat_type)); } }); - (argtypes, argexprs, has_self) + (has_conversion_in_effect, argtypes, argexprs, has_self) } fn contains_self_type_path(path: &Path) -> bool { diff --git a/gix-macros/tests/momo/mod.rs b/gix-macros/tests/momo/mod.rs index 2e79db76ece..98cc8813a5d 100644 --- a/gix-macros/tests/momo/mod.rs +++ b/gix-macros/tests/momo/mod.rs @@ -15,14 +15,6 @@ fn test_open_opts(directory: impl Into, options: Options) -> test_open_opts_inner(directory, options) } -#[allow(dead_code)] -#[momo] -fn test_with_try(t: impl TryInto, _options: Options) -> Result<(), ()> { - let t = t.try_into()?; - t.strip_prefix('1').ok_or(())?; - Ok(()) -} - #[momo] fn test_fn( a: impl Into, @@ -310,8 +302,7 @@ fn ux() { } #[test] -#[ignore = "needs work"] -fn ux_todo() { +fn ux_on_ineffective() { let t = trybuild::TestCases::new(); t.compile_fail("tests/momo/ux/error_if_ineffective.rs"); } diff --git a/gix-macros/tests/momo/ux/error_if_ineffective.stderr b/gix-macros/tests/momo/ux/error_if_ineffective.stderr new file mode 100644 index 00000000000..9f63fef1385 --- /dev/null +++ b/gix-macros/tests/momo/ux/error_if_ineffective.stderr @@ -0,0 +1,13 @@ +error: Couldn't apply a single conversion - momo is ineffective here + --> tests/momo/ux/error_if_ineffective.rs:2:1 + | +2 | #[gix_macros::momo] + | ^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the attribute macro `gix_macros::momo` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0601]: `main` function not found in crate `$CRATE` + --> tests/momo/ux/error_if_ineffective.rs:3:13 + | +3 | fn main() {} + | ^ consider adding a `main` function to `$DIR/tests/momo/ux/error_if_ineffective.rs` From 72545e971c894de511a723f6d5515f637a84f28f Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Thu, 7 Sep 2023 22:08:39 +1000 Subject: [PATCH 28/29] Revert to use `item_fn.span()` Signed-off-by: Jiahao XU --- gix-macros/src/momo.rs | 2 +- gix-macros/tests/momo/ux/error_if_ineffective.stderr | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/gix-macros/src/momo.rs b/gix-macros/src/momo.rs index dea1a7ae03f..802f4b5b024 100644 --- a/gix-macros/src/momo.rs +++ b/gix-macros/src/momo.rs @@ -14,7 +14,7 @@ pub(crate) fn inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream let (has_conversion_in_effect, argtypes, argexprs, has_self) = convert(&item_fn.sig.inputs, &ty_conversions); if !has_conversion_in_effect { return Error::new( - proc_macro2::Span::call_site(), + item_fn.span(), "Couldn't apply a single conversion - momo is ineffective here", ) .to_compile_error(); diff --git a/gix-macros/tests/momo/ux/error_if_ineffective.stderr b/gix-macros/tests/momo/ux/error_if_ineffective.stderr index 9f63fef1385..1d93bc3ff39 100644 --- a/gix-macros/tests/momo/ux/error_if_ineffective.stderr +++ b/gix-macros/tests/momo/ux/error_if_ineffective.stderr @@ -1,10 +1,8 @@ error: Couldn't apply a single conversion - momo is ineffective here - --> tests/momo/ux/error_if_ineffective.rs:2:1 + --> tests/momo/ux/error_if_ineffective.rs:1:1 | -2 | #[gix_macros::momo] - | ^^^^^^^^^^^^^^^^^^^ - | - = note: this error originates in the attribute macro `gix_macros::momo` (in Nightly builds, run with -Z macro-backtrace for more info) +1 | /// If nothing is done, momo should fail. + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0601]: `main` function not found in crate `$CRATE` --> tests/momo/ux/error_if_ineffective.rs:3:13 From d76efddf5afb73563ce7e837cf975cedd01e979c Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Thu, 7 Sep 2023 15:10:40 +0200 Subject: [PATCH 29/29] consolidate compile tests --- gix-macros/tests/momo/mod.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/gix-macros/tests/momo/mod.rs b/gix-macros/tests/momo/mod.rs index 98cc8813a5d..a6b7b63174f 100644 --- a/gix-macros/tests/momo/mod.rs +++ b/gix-macros/tests/momo/mod.rs @@ -298,11 +298,5 @@ fn struct_fn() { #[test] fn ux() { let t = trybuild::TestCases::new(); - t.compile_fail("tests/momo/ux/error_on_struct.rs"); -} - -#[test] -fn ux_on_ineffective() { - let t = trybuild::TestCases::new(); - t.compile_fail("tests/momo/ux/error_if_ineffective.rs"); + t.compile_fail("tests/momo/ux/*.rs"); }