From 65ec30d65913e2368febc1f00a68324ec9a9ecd4 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 11 Aug 2024 08:43:46 -0700 Subject: [PATCH 1/4] Revert "Embed Generics iterator implementations into method bodies" This reverts commit a48273415a63f407a0aed8582a63a3a738b0af6a. --- src/generics.rs | 216 ++++++++++++++++++++++++------------------------ 1 file changed, 108 insertions(+), 108 deletions(-) diff --git a/src/generics.rs b/src/generics.rs index 81542279fd..0aff7cecd6 100644 --- a/src/generics.rs +++ b/src/generics.rs @@ -105,139 +105,31 @@ impl Default for Generics { impl Generics { /// Iterator over the lifetime parameters in `self.params`. pub fn lifetimes(&self) -> impl Iterator { - struct Lifetimes<'a>(Iter<'a, GenericParam>); - - impl<'a> Iterator for Lifetimes<'a> { - type Item = &'a LifetimeParam; - - fn next(&mut self) -> Option { - let next = match self.0.next() { - Some(item) => item, - None => return None, - }; - if let GenericParam::Lifetime(lifetime) = next { - Some(lifetime) - } else { - self.next() - } - } - } - Lifetimes(self.params.iter()) } /// Iterator over the lifetime parameters in `self.params`. pub fn lifetimes_mut(&mut self) -> impl Iterator { - struct LifetimesMut<'a>(IterMut<'a, GenericParam>); - - impl<'a> Iterator for LifetimesMut<'a> { - type Item = &'a mut LifetimeParam; - - fn next(&mut self) -> Option { - let next = match self.0.next() { - Some(item) => item, - None => return None, - }; - if let GenericParam::Lifetime(lifetime) = next { - Some(lifetime) - } else { - self.next() - } - } - } - LifetimesMut(self.params.iter_mut()) } /// Iterator over the type parameters in `self.params`. pub fn type_params(&self) -> impl Iterator { - struct TypeParams<'a>(Iter<'a, GenericParam>); - - impl<'a> Iterator for TypeParams<'a> { - type Item = &'a TypeParam; - - fn next(&mut self) -> Option { - let next = match self.0.next() { - Some(item) => item, - None => return None, - }; - if let GenericParam::Type(type_param) = next { - Some(type_param) - } else { - self.next() - } - } - } - TypeParams(self.params.iter()) } /// Iterator over the type parameters in `self.params`. pub fn type_params_mut(&mut self) -> impl Iterator { - struct TypeParamsMut<'a>(IterMut<'a, GenericParam>); - - impl<'a> Iterator for TypeParamsMut<'a> { - type Item = &'a mut TypeParam; - - fn next(&mut self) -> Option { - let next = match self.0.next() { - Some(item) => item, - None => return None, - }; - if let GenericParam::Type(type_param) = next { - Some(type_param) - } else { - self.next() - } - } - } - TypeParamsMut(self.params.iter_mut()) } /// Iterator over the constant parameters in `self.params`. pub fn const_params(&self) -> impl Iterator { - struct ConstParams<'a>(Iter<'a, GenericParam>); - - impl<'a> Iterator for ConstParams<'a> { - type Item = &'a ConstParam; - - fn next(&mut self) -> Option { - let next = match self.0.next() { - Some(item) => item, - None => return None, - }; - if let GenericParam::Const(const_param) = next { - Some(const_param) - } else { - self.next() - } - } - } - ConstParams(self.params.iter()) } /// Iterator over the constant parameters in `self.params`. pub fn const_params_mut(&mut self) -> impl Iterator { - struct ConstParamsMut<'a>(IterMut<'a, GenericParam>); - - impl<'a> Iterator for ConstParamsMut<'a> { - type Item = &'a mut ConstParam; - - fn next(&mut self) -> Option { - let next = match self.0.next() { - Some(item) => item, - None => return None, - }; - if let GenericParam::Const(const_param) = next { - Some(const_param) - } else { - self.next() - } - } - } - ConstParamsMut(self.params.iter_mut()) } @@ -278,6 +170,114 @@ impl Generics { } } +pub struct Lifetimes<'a>(Iter<'a, GenericParam>); + +impl<'a> Iterator for Lifetimes<'a> { + type Item = &'a LifetimeParam; + + fn next(&mut self) -> Option { + let next = match self.0.next() { + Some(item) => item, + None => return None, + }; + if let GenericParam::Lifetime(lifetime) = next { + Some(lifetime) + } else { + self.next() + } + } +} + +pub struct LifetimesMut<'a>(IterMut<'a, GenericParam>); + +impl<'a> Iterator for LifetimesMut<'a> { + type Item = &'a mut LifetimeParam; + + fn next(&mut self) -> Option { + let next = match self.0.next() { + Some(item) => item, + None => return None, + }; + if let GenericParam::Lifetime(lifetime) = next { + Some(lifetime) + } else { + self.next() + } + } +} + +pub struct TypeParams<'a>(Iter<'a, GenericParam>); + +impl<'a> Iterator for TypeParams<'a> { + type Item = &'a TypeParam; + + fn next(&mut self) -> Option { + let next = match self.0.next() { + Some(item) => item, + None => return None, + }; + if let GenericParam::Type(type_param) = next { + Some(type_param) + } else { + self.next() + } + } +} + +pub struct TypeParamsMut<'a>(IterMut<'a, GenericParam>); + +impl<'a> Iterator for TypeParamsMut<'a> { + type Item = &'a mut TypeParam; + + fn next(&mut self) -> Option { + let next = match self.0.next() { + Some(item) => item, + None => return None, + }; + if let GenericParam::Type(type_param) = next { + Some(type_param) + } else { + self.next() + } + } +} + +pub struct ConstParams<'a>(Iter<'a, GenericParam>); + +impl<'a> Iterator for ConstParams<'a> { + type Item = &'a ConstParam; + + fn next(&mut self) -> Option { + let next = match self.0.next() { + Some(item) => item, + None => return None, + }; + if let GenericParam::Const(const_param) = next { + Some(const_param) + } else { + self.next() + } + } +} + +pub struct ConstParamsMut<'a>(IterMut<'a, GenericParam>); + +impl<'a> Iterator for ConstParamsMut<'a> { + type Item = &'a mut ConstParam; + + fn next(&mut self) -> Option { + let next = match self.0.next() { + Some(item) => item, + None => return None, + }; + if let GenericParam::Const(const_param) = next { + Some(const_param) + } else { + self.next() + } + } +} + /// Returned by `Generics::split_for_impl`. #[cfg(feature = "printing")] #[cfg_attr( From 857942e411d71d3c0787e14e220c8c7d9d491051 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 11 Aug 2024 08:48:53 -0700 Subject: [PATCH 2/4] Revert "Define Fields::members iterator type privately inside method" This reverts commit e8a9292cbd9f81b63479e15b0b59db644243500c. --- src/data.rs | 74 ++++++++++++++++++++++++++--------------------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/src/data.rs b/src/data.rs index 7f8a121c76..000ea0db2c 100644 --- a/src/data.rs +++ b/src/data.rs @@ -138,43 +138,6 @@ impl Fields { /// self.a.clone() }`. For structs with unnamed fields, `Self { 0: /// self.0.clone() }`. And for unit structs, `Self {}`. pub fn members(&self) -> impl Iterator + Clone + '_ { - struct Members<'a> { - fields: punctuated::Iter<'a, Field>, - index: u32, - } - - impl<'a> Iterator for Members<'a> { - type Item = Member; - - fn next(&mut self) -> Option { - let field = self.fields.next()?; - let member = match &field.ident { - Some(ident) => Member::Named(ident.clone()), - None => { - #[cfg(all(feature = "parsing", feature = "printing"))] - let span = crate::spanned::Spanned::span(&field.ty); - #[cfg(not(all(feature = "parsing", feature = "printing")))] - let span = proc_macro2::Span::call_site(); - Member::Unnamed(Index { - index: self.index, - span, - }) - } - }; - self.index += 1; - Some(member) - } - } - - impl<'a> Clone for Members<'a> { - fn clone(&self) -> Self { - Members { - fields: self.fields.clone(), - index: self.index, - } - } - } - Members { fields: self.iter(), index: 0, @@ -234,6 +197,43 @@ ast_struct! { } } +pub struct Members<'a> { + fields: punctuated::Iter<'a, Field>, + index: u32, +} + +impl<'a> Iterator for Members<'a> { + type Item = Member; + + fn next(&mut self) -> Option { + let field = self.fields.next()?; + let member = match &field.ident { + Some(ident) => Member::Named(ident.clone()), + None => { + #[cfg(all(feature = "parsing", feature = "printing"))] + let span = crate::spanned::Spanned::span(&field.ty); + #[cfg(not(all(feature = "parsing", feature = "printing")))] + let span = proc_macro2::Span::call_site(); + Member::Unnamed(Index { + index: self.index, + span, + }) + } + }; + self.index += 1; + Some(member) + } +} + +impl<'a> Clone for Members<'a> { + fn clone(&self) -> Self { + Members { + fields: self.fields.clone(), + index: self.index, + } + } +} + #[cfg(feature = "parsing")] pub(crate) mod parsing { use crate::attr::Attribute; From 7dc05a564398eabc552a2d46bb580f9a60d410fb Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 11 Aug 2024 09:06:58 -0700 Subject: [PATCH 3/4] Add regression test for issue 1718 error[E0502]: cannot borrow `generics.params` as mutable because it is also borrowed as immutable --> tests/test_iterators.rs:81:25 | 77 | let _ = generics | -------- | | | _____________immutable borrow occurs here | | 78 | | .lifetimes() | |____________________- a temporary with access to the immutable borrow is created here ... ... 81 | .unwrap_or_else(|| { | ^^ mutable borrow occurs here 82 | let lifetime: Lifetime = parse_quote!('a); 83 | generics.params.insert( | --------------- second borrow occurs due to use of `generics.params` in closure ... 88 | }); | - ... and the immutable borrow might be used here, when that temporary is dropped and runs the destructor for type `impl Iterator` --- tests/test_iterators.rs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/tests/test_iterators.rs b/tests/test_iterators.rs index 5f0eff59e6..4090bcc8e8 100644 --- a/tests/test_iterators.rs +++ b/tests/test_iterators.rs @@ -1,7 +1,7 @@ -#![allow(clippy::uninlined_format_args)] +#![allow(clippy::map_unwrap_or, clippy::uninlined_format_args)] use syn::punctuated::{Pair, Punctuated}; -use syn::Token; +use syn::{parse_quote, GenericParam, Generics, Lifetime, LifetimeParam, Token}; #[macro_use] mod macros; @@ -68,3 +68,22 @@ fn may_dangle() { } } } + +// Regression test for https://github.com/dtolnay/syn/issues/1718 +#[test] +fn no_opaque_drop() { + let mut generics = Generics::default(); + + let _ = generics + .lifetimes() + .next() + .map(|param| param.lifetime.clone()) + .unwrap_or_else(|| { + let lifetime: Lifetime = parse_quote!('a); + generics.params.insert( + 0, + GenericParam::Lifetime(LifetimeParam::new(lifetime.clone())), + ); + lifetime + }); +} From 2955ac55b70952e2114e559d1cd61267225152e8 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 11 Aug 2024 09:01:07 -0700 Subject: [PATCH 4/4] Expose non-impl-Trait iterator return types --- src/data.rs | 74 +++++++++++++++++++++++++------------------------ src/generics.rs | 48 ++++++++++++++++++++------------ src/macros.rs | 16 +++++++++++ 3 files changed, 84 insertions(+), 54 deletions(-) diff --git a/src/data.rs b/src/data.rs index 000ea0db2c..9e73f02d3c 100644 --- a/src/data.rs +++ b/src/data.rs @@ -105,42 +105,44 @@ impl Fields { } } - /// Get an iterator over the fields of a struct or variant as [`Member`]s. - /// This iterator can be used to iterate over a named or unnamed struct or - /// variant's fields uniformly. - /// - /// # Example - /// - /// The following is a simplistic [`Clone`] derive for structs. (A more - /// complete implementation would additionally want to infer trait bounds on - /// the generic type parameters.) - /// - /// ``` - /// # use quote::quote; - /// # - /// fn derive_clone(input: &syn::ItemStruct) -> proc_macro2::TokenStream { - /// let ident = &input.ident; - /// let members = input.fields.members(); - /// let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); - /// quote! { - /// impl #impl_generics Clone for #ident #ty_generics #where_clause { - /// fn clone(&self) -> Self { - /// Self { - /// #(#members: self.#members.clone()),* - /// } - /// } - /// } - /// } - /// } - /// ``` - /// - /// For structs with named fields, it produces an expression like `Self { a: - /// self.a.clone() }`. For structs with unnamed fields, `Self { 0: - /// self.0.clone() }`. And for unit structs, `Self {}`. - pub fn members(&self) -> impl Iterator + Clone + '_ { - Members { - fields: self.iter(), - index: 0, + return_impl_trait! { + /// Get an iterator over the fields of a struct or variant as [`Member`]s. + /// This iterator can be used to iterate over a named or unnamed struct or + /// variant's fields uniformly. + /// + /// # Example + /// + /// The following is a simplistic [`Clone`] derive for structs. (A more + /// complete implementation would additionally want to infer trait bounds on + /// the generic type parameters.) + /// + /// ``` + /// # use quote::quote; + /// # + /// fn derive_clone(input: &syn::ItemStruct) -> proc_macro2::TokenStream { + /// let ident = &input.ident; + /// let members = input.fields.members(); + /// let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); + /// quote! { + /// impl #impl_generics Clone for #ident #ty_generics #where_clause { + /// fn clone(&self) -> Self { + /// Self { + /// #(#members: self.#members.clone()),* + /// } + /// } + /// } + /// } + /// } + /// ``` + /// + /// For structs with named fields, it produces an expression like `Self { a: + /// self.a.clone() }`. For structs with unnamed fields, `Self { 0: + /// self.0.clone() }`. And for unit structs, `Self {}`. + pub fn members(&self) -> impl Iterator + Clone + '_ [Members] { + Members { + fields: self.iter(), + index: 0, + } } } } diff --git a/src/generics.rs b/src/generics.rs index 0aff7cecd6..692ad53001 100644 --- a/src/generics.rs +++ b/src/generics.rs @@ -103,34 +103,46 @@ impl Default for Generics { } impl Generics { - /// Iterator over the lifetime parameters in `self.params`. - pub fn lifetimes(&self) -> impl Iterator { - Lifetimes(self.params.iter()) + return_impl_trait! { + /// Iterator over the lifetime parameters in `self.params`. + pub fn lifetimes(&self) -> impl Iterator [Lifetimes] { + Lifetimes(self.params.iter()) + } } - /// Iterator over the lifetime parameters in `self.params`. - pub fn lifetimes_mut(&mut self) -> impl Iterator { - LifetimesMut(self.params.iter_mut()) + return_impl_trait! { + /// Iterator over the lifetime parameters in `self.params`. + pub fn lifetimes_mut(&mut self) -> impl Iterator [LifetimesMut] { + LifetimesMut(self.params.iter_mut()) + } } - /// Iterator over the type parameters in `self.params`. - pub fn type_params(&self) -> impl Iterator { - TypeParams(self.params.iter()) + return_impl_trait! { + /// Iterator over the type parameters in `self.params`. + pub fn type_params(&self) -> impl Iterator [TypeParams] { + TypeParams(self.params.iter()) + } } - /// Iterator over the type parameters in `self.params`. - pub fn type_params_mut(&mut self) -> impl Iterator { - TypeParamsMut(self.params.iter_mut()) + return_impl_trait! { + /// Iterator over the type parameters in `self.params`. + pub fn type_params_mut(&mut self) -> impl Iterator [TypeParamsMut] { + TypeParamsMut(self.params.iter_mut()) + } } - /// Iterator over the constant parameters in `self.params`. - pub fn const_params(&self) -> impl Iterator { - ConstParams(self.params.iter()) + return_impl_trait! { + /// Iterator over the constant parameters in `self.params`. + pub fn const_params(&self) -> impl Iterator [ConstParams] { + ConstParams(self.params.iter()) + } } - /// Iterator over the constant parameters in `self.params`. - pub fn const_params_mut(&mut self) -> impl Iterator { - ConstParamsMut(self.params.iter_mut()) + return_impl_trait! { + /// Iterator over the constant parameters in `self.params`. + pub fn const_params_mut(&mut self) -> impl Iterator [ConstParamsMut] { + ConstParamsMut(self.params.iter_mut()) + } } /// Initializes an empty `where`-clause if there is not one present already. diff --git a/src/macros.rs b/src/macros.rs index 2b6708d495..167f2cf260 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -164,3 +164,19 @@ macro_rules! check_keyword_matches { (pub pub) => {}; (struct struct) => {}; } + +#[cfg(any(feature = "full", feature = "derive"))] +macro_rules! return_impl_trait { + ( + $(#[$attr:meta])* + $vis:vis fn $name:ident $args:tt -> $impl_trait:ty [$concrete:ty] $body:block + ) => { + #[cfg(not(docsrs))] + $(#[$attr])* + $vis fn $name $args -> $concrete $body + + #[cfg(docsrs)] + $(#[$attr])* + $vis fn $name $args -> $impl_trait $body + }; +}