Skip to content

Make the generated vector-like object use a single length #26

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Make the generated vector-like object use a single length #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions soa-derive-internal/src/input.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ impl Input {
Ident::new(&format!("{}Vec", self.name), Span::call_site())
}

pub fn vec_fields_name(&self) -> Ident {
Ident::new(&format!("{}VecFields", self.name), Span::call_site())
}

pub fn slice_name(&self) -> Ident {
Ident::new(&format!("{}Slice", self.name), Span::call_site())
}
Expand Down
279 changes: 87 additions & 192 deletions soa-derive-internal/src/iter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,243 +15,138 @@ pub fn derive(input: &Input) -> TokenStream {
let ref_name = &input.ref_name();
let ref_mut_name = &input.ref_mut_name();

let ref_doc_url = format!("[`{0}`](struct.{0}.html)", ref_name);
let ref_mut_doc_url = format!("[`{0}`](struct.{0}.html)", ref_mut_name);

let fields_names = &input.fields.iter()
.map(|field| field.ident.clone().unwrap())
.collect::<Vec<_>>();
let first_field = &fields_names[0];
let fields_names = input.fields.iter()
.map(|field| field.ident.clone().unwrap())
.collect::<Vec<_>>();
let fields_names_1 = &fields_names;
let fields_names_2 = &fields_names;

let fields_types = &input.fields.iter()
.map(|field| &field.ty)
.collect::<Vec<_>>();
let first_field_type = &fields_types[0];

let mut iter_type = quote!{
slice::Iter<'a, #first_field_type>
};
let mut iter_pat = quote!{
#first_field
};
let mut create_iter = quote!{
self.#first_field.iter()
};

let mut iter_mut_type = quote!{
slice::IterMut<'a, #first_field_type>
};
let mut create_iter_mut = quote!{
self.#first_field.iter_mut()
};

if fields_types.len() > 1 {
for field in &input.fields[1..] {
let field_name = &field.ident;
let field_type = &field.ty;

iter_pat = quote!{
(#iter_pat, #field_name)
};

iter_type = quote!{
iter::Zip<#iter_type, slice::Iter<'a, #field_type>>
};

create_iter = quote!{
#create_iter.zip(self.#field_name.iter())
};

iter_mut_type = quote!{
iter::Zip<#iter_mut_type, slice::IterMut<'a, #field_type>>
};

create_iter_mut = quote!{
#create_iter_mut.zip(self.#field_name.iter_mut())
};
}
}

let mut generated = quote! {
let generated = quote!{
#[allow(non_snake_case, dead_code)]
mod #detail_mod {
use super::*;
use std::slice;
#[allow(unused_imports)]
use std::iter;

#[allow(missing_debug_implementations)]
#visibility struct Iter<'a>(pub(super) #iter_type);

impl<'a> Iterator for Iter<'a> {
type Item = #ref_name<'a>;

#[inline]
fn next(&mut self) -> Option<#ref_name<'a>> {
self.0.next().and_then(|#iter_pat|
Some(#ref_name{
#(#fields_names,)*
})
)
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
self.0.size_hint()
}
}

impl<'a> DoubleEndedIterator for Iter<'a> {

#[inline]
fn next_back(&mut self) -> Option<#ref_name<'a>> {
self.0.next_back().and_then(|#iter_pat|
Some(#ref_name{
#(#fields_names,)*
})
)
}
}

impl #vec_name {
/// Get an iterator over the
#[doc = #ref_doc_url]
/// in this vector
#visibility fn iter(&self) -> Iter {
Iter(#create_iter)
}
pub struct VecIter<'a> {
Copy link
Member

Choose a reason for hiding this comment

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

Why replace the iterator type as well? The previous one should still be constructible through the individual slices. The previous type should also optimize a bit better since we get all the marker traits (ExactSizeIterator and friends) from std, as well as specialization which std can use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Couldn't understand what the std was doing (RawVec doesn't have .iter()) because the code is not straight forward, so I made my simple iterator. Actually 2 days ago I realized that the standard is simply converting to slice and using its iterator. So yes, we could keep the old code with minor changes.

A point for a future debate would be if making a well-implemented custom iterator may be made more performant than using nested zips (but with the obvious downside of more code maintenance)

pub(super) vec: &'a #vec_name,
pub(super) n: usize,
}

impl<'a> #slice_name<'a> {
/// Get an iterator over the
#[doc = #ref_doc_url]
/// in this slice.
#visibility fn iter(&self) -> Iter {
Iter(#create_iter)
impl<'a> VecIter<'a> {
pub(self) fn new(vec: &'a #vec_name) -> VecIter<'a> {
VecIter {
vec,
n: 0,
}
}
}

#[allow(missing_debug_implementations)]
#visibility struct IterMut<'a>(pub(super) #iter_mut_type);
impl<'a> Iterator for VecIter<'a> {
type Item = #ref_name<'a>;

impl<'a> Iterator for IterMut<'a> {
type Item = #ref_mut_name<'a>;
fn next(&mut self) -> Option<Self::Item> {
if self.n >= self.vec.len() {
return None;
}

#[inline]
fn next(&mut self) -> Option<#ref_mut_name<'a>> {
self.0.next().and_then(|#iter_pat|
Some(#ref_mut_name{
#(#fields_names,)*
let item = unsafe {
Some(#ref_name {
#(
#fields_names_1: self.vec.data.#fields_names_2.ptr().add(self.n).as_ref().unwrap(),
)*
})
)
};
self.n += 1;
item
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
self.0.size_hint()
if self.n >= self.vec.len() {
return (0, Some(0))
}
let left = self.vec.len() - self.n;
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: how about remaining instead of left?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes!

(left, Some(left))
}
}

impl<'a> DoubleEndedIterator for IterMut<'a> {

#[inline]
fn next_back(&mut self) -> Option<#ref_mut_name<'a>> {
self.0.next_back().and_then(|#iter_pat|
Some(#ref_mut_name{
#(#fields_names,)*
})
)
}
pub struct VecIterMut<'a> {
pub(super) vec: &'a mut #vec_name,
pub(super) n: usize,
}

impl #vec_name {
/// Get a mutable iterator over the
#[doc = #ref_mut_doc_url]
/// in this vector
#visibility fn iter_mut(&mut self) -> IterMut {
IterMut(#create_iter_mut)
impl<'a> VecIterMut<'a> {
pub(self) fn new(vec: &'a mut #vec_name) -> VecIterMut<'a> {
VecIterMut {
vec,
n: 0,
}
}
}

impl<'a> #slice_mut_name<'a> {
/// Get an iterator over the
#[doc = #ref_doc_url]
/// in this vector
#visibility fn iter(&mut self) -> Iter {
Iter(#create_iter)
}

/// Get a mutable iterator over the
#[doc = #ref_mut_doc_url]
/// in this vector
#visibility fn iter_mut(&mut self) -> IterMut {
IterMut(#create_iter_mut)
}
}
}
};
impl<'a> Iterator for VecIterMut<'a> {
type Item = #ref_mut_name<'a>;

if let Visibility::Public(_) = *visibility {
generated.append_all(quote!{
impl<'a> IntoIterator for #slice_name<'a> {
type Item = #ref_name<'a>;
type IntoIter = #detail_mod::Iter<'a>;
fn next(&mut self) -> Option<Self::Item> {
if self.n >= self.vec.len() {
return None;
}

fn into_iter(self) -> Self::IntoIter {
#detail_mod::Iter(#create_iter)
let item = unsafe {
Some(#ref_mut_name {
#(
#fields_names_1: self.vec.data.#fields_names_2.ptr().add(self.n).as_mut().unwrap(),
)*
})
};
self.n += 1;
item
}
}


impl std::iter::FromIterator<#name> for #vec_name {
fn from_iter<T: IntoIterator<Item=#name>>(iter: T) -> Self {
let mut result = #vec_name::new();
for element in iter {
#(
(result.#fields_names).push(element.#fields_names);
)*
fn size_hint(&self) -> (usize, Option<usize>) {
if self.n >= self.vec.len() {
return (0, Some(0))
}
result
let left = self.vec.len() - self.n;
(left, Some(left))
}
}
}

impl<'a, 'b> IntoIterator for &'a #slice_name<'b> {
type Item = #ref_name<'a>;
type IntoIter = #detail_mod::Iter<'a>;

fn into_iter(self) -> Self::IntoIter {
#detail_mod::Iter(#create_iter)
impl #vec_name {
pub fn iter<'a>(&'a self) -> #detail_mod::VecIter<'a> {
#detail_mod::VecIter {
vec: self,
n: 0,
}
}

impl<'a> IntoIterator for &'a #vec_name {
type Item = #ref_name<'a>;
type IntoIter = #detail_mod::Iter<'a>;

fn into_iter(self) -> Self::IntoIter {
#detail_mod::Iter(#create_iter)
pub fn iter_mut<'a>(&'a mut self) -> #detail_mod::VecIterMut<'a> {
#detail_mod::VecIterMut {
vec: self,
n: 0,
}
}
}

impl<'a> IntoIterator for #slice_mut_name<'a> {
type Item = #ref_mut_name<'a>;
type IntoIter = #detail_mod::IterMut<'a>;
impl<'a> IntoIterator for &'a #vec_name {
Copy link
Member

Choose a reason for hiding this comment

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

These implementation where guarded by if let Visibility::Public(_) = *visibility previously, why remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually remade it when replacing the type. I'll reverse this file and change it to use the slices instead of a custom iterator

type Item = #ref_name<'a>;
type IntoIter = #detail_mod::VecIter<'a>;

fn into_iter(self) -> Self::IntoIter {
#detail_mod::IterMut(#create_iter_mut)
}
fn into_iter(self) -> Self::IntoIter {
return self.iter()
}
}

impl<'a> IntoIterator for &'a mut #vec_name {
type Item = #ref_mut_name<'a>;
type IntoIter = #detail_mod::IterMut<'a>;
impl<'a> IntoIterator for &'a mut #vec_name {
Copy link
Member

Choose a reason for hiding this comment

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

This is also missing implementations for slice & slice mut

type Item = #ref_mut_name<'a>;
type IntoIter = #detail_mod::VecIterMut<'a>;

fn into_iter(self) -> Self::IntoIter {
#detail_mod::IterMut(#create_iter_mut)
}
fn into_iter(self) -> Self::IntoIter {
return self.iter_mut()
}
});
}
}
};

return generated;
}
1 change: 1 addition & 0 deletions soa-derive-internal/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pub fn soa_derive(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
generated.append_all(slice::derive_mut(&input));
generated.append_all(iter::derive(&input));
generated.append_all(derive_trait(&input));
// println!("{}", generated); // Useful for debugging.
generated.into()
}

Expand Down
Loading