ryanhaining / cppitertools

Implementation of python itertools and builtin iteration functions for C++17
https://twitter.com/cppitertools
BSD 2-Clause "Simplified" License
1.37k stars 115 forks source link

Support for move only types #14

Closed iliocatallo closed 10 years ago

iliocatallo commented 10 years ago

Hi,

I would like to use the library with move only types. Here's a toy example:

#include <iostream>
#include <vector>
#include <itertools/product.hpp>

struct MoveOnlyType {
    MoveOnlyType() = default;
    MoveOnlyType(MoveOnlyType const& other) = delete;
    MoveOnlyType(MoveOnlyType&& other) = default;
    MoveOnlyType& operator=(MoveOnlyType const& other) = delete;
    MoveOnlyType& operator=(MoveOnlyType&& other) = default;
};

int main() {

    std::vector<MoveOnlyType> v1, v2;
    v1.push_back(MoveOnlyType());
    v1.push_back(MoveOnlyType());

    v2.push_back(MoveOnlyType());
    v2.push_back(MoveOnlyType());
    v2.push_back(MoveOnlyType());

    for (auto&& pair_of_moveonly: iter::product(v1, v2)) 
        std::cout << "it does not work" << std::endl;
}

Thanks.

ryanhaining commented 10 years ago

I believe the current implementation of product does copy the data. I was considering reworking it and similar to use reference wrappers. The majority of the library does support move only types though afaik, is it product specifically that's giving you problems or in general you are experiencing trouble?

iliocatallo commented 10 years ago

Unfortunately I have the same problem with iter::powerset. Basically, I have a std::vector of move only objects and I would like to iterate over the container with iter::powerset and iter::product.

Thanks.

ryanhaining commented 10 years ago

Okay I think I can do this with reference_wrapper without any sort of API break. I'll have to consider it a bit more, and probably get to this over the weekend.

On Thu, Sep 4, 2014 at 10:36 AM, iliocatallo notifications@github.com wrote:

Unfortunately I have the same problem with iter::powerset. Basically, I have a std::vector of move only objects and I would like to iterate over the container with iter::powerset and iter::product.

Thanks.

— Reply to this email directly or view it on GitHub https://github.com/ryanhaining/cppitertools/issues/14#issuecomment-54486132 .

iliocatallo commented 10 years ago

Yeah, hopefully std::reference_wrapper should do the trick.

Thanks.

ryanhaining commented 10 years ago

My mistake, since product returning tuples, std::reference_wrapper isn't right. It needs to be a tuple of references. I'm in the process of reworking product to support rvalues properly, afterwhich I'll be going through all the combinatoric functions and figuring out the right solution at each step.

iliocatallo commented 10 years ago

Probably you should return a std::tuple<std::reference_wrapper<Args>...>.

Thank you.

ryanhaining commented 10 years ago

nah, because reference_wrapper would be wrong if dereferencing one of the iterators returned a value. As it turns out I fixed this issue while rewriting product to support temporaries so product now works with containers of moveonly types. The previous version used make_tuple which would deduce the types, so even if *iter had type T&, the resulting tuple would be tuple<T>. I now have it explicitly templating the tuple; I'm not using make_tuple anwhere. That solves product, but reference wrapper still needs to be added in the other combinatoric functions.

ryanhaining commented 10 years ago

I got combinations working with move-only as well. That same pattern should apply to the rest of the combinatorics.

iliocatallo commented 10 years ago

Great! thank you for your awesome library!

ryanhaining commented 10 years ago

thanks for your interest and for bringing this issue to light. I'll close it once I finish the rest of the combinatorics.

ryanhaining commented 10 years ago

I believe this is working for all combinatorics now including powerset, with the exception of permutations which uses next_permutation which has its own requirments. I can't do much about that without implementing a totally new permutations though, and it would still have to cover the case where there are two equal elements in one sequence.

edit: I'll figure it out tomorrow. I got some ideas

ryanhaining commented 10 years ago

did it. lml. permutations is working. it is done for all the combinatorics