Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix IntoIterator derived bounds (#208) #284

Merged
merged 12 commits into from
Aug 11, 2023
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Fix `Error` derive not working with `const` generics.
- Support trait objects for source in Error, e.g.
`Box<dyn Error + Send + 'static>`
- Fix bounds on derived `IntoIterator` impls for generic structs.
([#284](https://github.com/JelteF/derive_more/pull/284))

## 0.99.10 - 2020-09-11

Expand Down
17 changes: 10 additions & 7 deletions impl/src/into_iterator.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::utils::{
add_extra_generic_param, add_extra_ty_param_bound_ref, SingleFieldData, State,
add_extra_generic_param, add_extra_where_clauses, SingleFieldData, State,
};
use proc_macro2::TokenStream;
use quote::{quote, ToTokens};
Expand All @@ -26,16 +26,19 @@ pub fn expand(input: &DeriveInput, trait_name: &'static str) -> Result<TokenStre
let reference_with_lifetime = ref_type.reference_with_lifetime();

let generics_impl;
let generics =
add_extra_ty_param_bound_ref(&input.generics, trait_path, ref_type);
let (_, ty_generics, where_clause) = generics.split_for_impl();
let (impl_generics, _, _) = if ref_type.is_ref() {
generics_impl = add_extra_generic_param(&generics, lifetime.clone());
generics_impl = add_extra_generic_param(&input.generics, lifetime.clone());
generics_impl.split_for_impl()
} else {
generics.split_for_impl()
input.generics.split_for_impl()
};
// let generics = add_extra_ty_param_bound(&input.generics, trait_path);

let generics = add_extra_where_clauses(
&input.generics,
quote! { where #reference_with_lifetime #field_type: #trait_path },
MegaBluejay marked this conversation as resolved.
Show resolved Hide resolved
);
let (_, ty_generics, where_clause) = generics.split_for_impl();

let casted_trait = &quote! {
<#reference_with_lifetime #field_type as #trait_path>
};
Expand Down
21 changes: 0 additions & 21 deletions impl/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,27 +159,6 @@ pub fn add_extra_ty_param_bound<'a>(
generics
}

pub fn add_extra_ty_param_bound_ref<'a>(
generics: &'a Generics,
bound: &'a TokenStream,
ref_type: RefType,
) -> Generics {
match ref_type {
RefType::No => add_extra_ty_param_bound(generics, bound),
_ => {
let generics = generics.clone();
let idents = generics.type_params().map(|x| &x.ident);
let ref_with_lifetime = ref_type.reference_with_lifetime();
add_extra_where_clauses(
&generics,
quote! {
where #(#ref_with_lifetime #idents: #bound),*
},
)
}
}
}

pub fn add_extra_generic_param(
generics: &Generics,
generic_param: TokenStream,
Expand Down
126 changes: 126 additions & 0 deletions tests/into_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,52 @@
#![allow(dead_code, unused_imports)]

#[cfg(not(feature = "std"))]
#[macro_use]
extern crate alloc;

#[cfg(not(feature = "std"))]
use alloc::vec::Vec;
use core::fmt::Debug;

use derive_more::IntoIterator;

#[track_caller]
fn assert_iter<T: PartialEq + Debug, I: IntoIterator<Item = T>>(iter: I, vals: &[T]) {
assert_eq!(iter.into_iter().collect::<Vec<_>>(), vals);
Copy link
Collaborator

@tyranron tyranron Aug 11, 2023

Choose a reason for hiding this comment

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

@MegaBluejay such unification is not good for tests, as doesn't allow us to understand well which assertion in the test case is failing exactly:

---- generic stdout ----
thread 'generic' panicked at 'assertion failed: `(left == right)`
  left: `[1, 2, 3]`,
 right: `[]`', tests/into_iterator.rs:18:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- generic_bounds stdout ----
thread 'generic_bounds' panicked at 'assertion failed: `(left == right)`
  left: `[1, 2, 3]`,
 right: `[]`', tests/into_iterator.rs:18:5

---- generic_refs stdout ----
thread 'generic_refs' panicked at 'assertion failed: `(left == right)`
  left: `[1, 2, 3]`,
 right: `[]`', tests/into_iterator.rs:18:5

---- generic_owned stdout ----
thread 'generic_owned' panicked at 'assertion failed: `(left == right)`
  left: `[1, 2, 3]`,
 right: `[]`', tests/into_iterator.rs:18:5

---- named_single stdout ----
thread 'named_single' panicked at 'assertion failed: `(left == right)`
  left: `[1, 2, 3]`,
 right: `[]`', tests/into_iterator.rs:18:5

---- tuple_single stdout ----
thread 'tuple_single' panicked at 'assertion failed: `(left == right)`
  left: `[1, 2, 3]`,
 right: `[]`', tests/into_iterator.rs:18:5

#[track_caller] may be helpful here to some degree, but not when multiple assertions are united in the same helper function.

}

#[derive(IntoIterator)]
#[into_iterator(owned, ref, ref_mut)]
struct MyVec(Vec<i32>);

#[test]
fn tuple_single() {
let mut vals = vec![1, 2, 3];
let mut iter = MyVec(vals.clone());

assert_iter(&mut iter, &vals.iter_mut().collect::<Vec<_>>());
assert_iter(&iter, &vals.iter().collect::<Vec<_>>());
assert_iter(iter, &vals);
}

#[derive(IntoIterator)]
#[into_iterator(owned, ref, ref_mut)]
struct Numbers {
numbers: Vec<i32>,
}

#[test]
fn named_single() {
let mut vals = vec![1, 2, 3];
let mut iter = Numbers {
numbers: vals.clone(),
};

assert_iter(&mut iter, &vals.iter_mut().collect::<Vec<_>>());
assert_iter(&iter, &vals.iter().collect::<Vec<_>>());
assert_iter(iter, &vals);
}

#[derive(IntoIterator)]
struct Numbers2 {
#[into_iterator(owned, ref, ref_mut)]
Expand All @@ -27,6 +56,19 @@ struct Numbers2 {
useless2: bool,
}

fn named_many() {
let mut vals = vec![1, 2, 3];
let mut iter = Numbers2 {
numbers: vals.clone(),
useless: true,
useless2: true,
};

assert_iter(&mut iter, &vals.iter_mut().collect::<Vec<_>>());
assert_iter(&iter, &vals.iter().collect::<Vec<_>>());
assert_iter(iter, &vals);
}

#[derive(IntoIterator)]
struct Numbers3 {
#[into_iterator(ref, ref_mut)]
Expand All @@ -45,3 +87,87 @@ impl ::core::iter::IntoIterator for Numbers3 {
<Vec<i32> as ::core::iter::IntoIterator>::into_iter(self.numbers)
}
}

#[derive(IntoIterator)]
struct Generic1<T> {
#[into_iterator(owned, ref, ref_mut)]
items: Vec<T>,
}
MegaBluejay marked this conversation as resolved.
Show resolved Hide resolved

#[test]
fn generic() {
let mut vals = vec![1, 2, 3];
let mut iter = Generic1 {
items: vals.clone(),
};

assert_iter(&mut iter, &vals.iter_mut().collect::<Vec<_>>());
assert_iter(&iter, &vals.iter().collect::<Vec<_>>());
assert_iter(iter, &vals);
}

#[derive(IntoIterator)]
struct Generic2<'a, T, U: Send>
where
T: Send,
{
#[into_iterator(owned, ref, ref_mut)]
items: Vec<T>,
useless: &'a U,
}

#[test]
fn generic_bounds() {
let mut vals = vec![1, 2, 3];
let useless = false;
let mut iter = Generic2 {
items: vals.clone(),
useless: &useless,
};

assert_iter(&mut iter, &vals.iter_mut().collect::<Vec<_>>());
assert_iter(&iter, &vals.iter().collect::<Vec<_>>());
assert_iter(iter, &vals);
}

#[derive(IntoIterator)]
struct Generic3<'a, 'b, T> {
#[into_iterator(owned)]
items: &'a mut Vec<&'b mut T>,
MegaBluejay marked this conversation as resolved.
Show resolved Hide resolved
}

#[test]
fn generic_refs() {
let mut numbers = vec![1, 2, 3];
let mut numbers2 = numbers.clone();

let mut number_refs = numbers.iter_mut().collect::<Vec<_>>();
let mut number_refs2 = numbers2.iter_mut().collect::<Vec<_>>();

assert_iter(
Generic3 {
items: &mut number_refs,
},
&number_refs2.iter_mut().collect::<Vec<_>>(),
)
}

#[derive(IntoIterator)]
struct Generic4<T> {
#[into_iterator]
items: Vec<T>,
useless: bool,
}

#[test]
fn generic_owned() {
let numbers = vec![1, 2, 3];

assert_iter(
Generic4 {
items: numbers.clone(),
useless: true,
},
&numbers,
);
}