Naios / function2

Improved and configurable drop-in replacement to std::function that supports move only types, multiple overloads and more
http://naios.github.io/function2
Boost Software License 1.0
539 stars 47 forks source link

IsThrowing affects movability of fu2::function #26

Closed Rajveer86 closed 5 years ago

Rajveer86 commented 5 years ago

@Naios


Commit Hash

d2acdb6 (release 4.0.0)

Expected Behavior

When using fu2::function_base to create a custom unique_function with IsThrowing set to false, storing the custom unique_function in a std::vector should not invoke abort() on vector resize (due to the vector trying to call the copy constructor):

Actual Behavior

abort() is called when the vector tries to copy it's contents internally.

Steps to Reproduce

The following code calls abort() on functions.reserve(...):

#include <vector>
#include "function2.hpp"

template<typename Signature>
using UniqueFunction = fu2::function_base<true, false, fu2::capacity_default,
    false, false, Signature>;

int main(int argc, char* argv[])
{
    std::vector<UniqueFunction<void()>> functions;
    int x = 0;

    functions.emplace_back([&]()
    {
        ++x;
    });

    functions.reserve(functions.size() + 1); //abort() called here

    return 0;
}

I have read that std::vector will only internally move the object on memory reallocation if the type's move constructor and assignment operator are marked as noexcept, therefore I tried changing the vector to std::vector<UniqueFunction<void() noexcept>> and adding noexcept to the lambda with no effect.

Changing IsThrowing to true corrects everything. Please let me know if I'm misunderstanding anything!

Your Environment

Naios commented 5 years ago

This is probably a duplicate of #23 which was fixed in e3695b4b4. Could you test whether there are still issues when using the current HEAD e3695b4b4fa3c672e of the master branch?

Rajveer86 commented 5 years ago

Thanks for the quick reply, I'll test this out tonight and will let you know!

Rajveer86 commented 5 years ago

@Naios Sorry I didn't see the other issue, I've confirmed that the current HEAD fixes this. Thank you :)