dasp icon indicating copy to clipboard operation
dasp copied to clipboard

Impl Interpolator for &mut (impl Iterator).

Open quadrupleslap opened this issue 6 years ago • 4 comments

Wouldn't you usually want to keep the state across interpolate calls?

This isn't a breaking change afaik.

quadrupleslap avatar Jul 17 '18 02:07 quadrupleslap

Yeah, right now next_source_frame only takes &mut self so it does actually keep state across calls when it's used in the Converter. Converter currently owns the Interpolator, but borrows it for each successive call.

Instead, maybe the advantage of this is that it would let you use the same backing buffer for multiple Converter objects in succession, provided that only one Converter is alive at a time. In other words, it can save some memory allocations in very special circumstances at runtime, which is always a plus.

Anyway, this could technically be a breaking change if a downstream crate has an overlapping implementation, and you'd have the conflicting impls error. IDK where that falls on the semver hierarchy, but it's a perfectly wonderful contribution either way IMO. Thanks!

andrewcsmith avatar Jul 19 '18 16:07 andrewcsmith

Looks fine to add to me!

@andrewcsmith I don't think this is a breaking change as I don't think a user can implement Interpolator for &'a mut T (or any &'a mut <insert_type>) downstream due to rust's orphan rules which aim to prevent this problem (though I haven't actually checked and could be wrong! just going by memory).

For some reason travis failed on nightly, running the build again to make sure travis can finish successfully before merging.

mitchmindtree avatar Jul 21 '18 08:07 mitchmindtree

Ahh, looks like this is legitimately failing to compile in the --no-default-features build.

mitchmindtree avatar Jul 21 '18 11:07 mitchmindtree

This branch has conflicts that must be resolved

micwoj92 avatar Jul 12 '22 22:07 micwoj92