Calling rvalue functions twice

tobiasheineken commented 5 years ago

@Naios

Commit Hash

Git Tag 3.1.0

Expected Behavior

As Rvalue functions are meant to be single shot functions, the second invocation should result in raising a bad_function_call

Actual Behavior

Output:

10
10

Steps to Reproduce

#include <function2.hpp>
#include <iostream>

int main() {
    fu2::function<void(int)&&> fun = [](int x) {std::cout << x << std::endl;};
    std::move(fun)(10);
    std::move(fun)(10);
    return 0;
}

Your Environment

OS: Ubuntu 18.04.1 LTS
Compiler and version: gcc version 7.3.0 (Ubuntu 7.3.0-27ubuntu1~18.04)

Additional Questions

As I wish to use your library in a large GPL3 project, I cannot use the new Version 4.0, due to its cmake version higher than 3.5. Why did you increase the Cmake version? It seems to work fine for me. Has anyone suggestions how to use the newer versions with old cmake?

Naios commented 5 years ago

Hey, thanks for your issue report.

I thought about this and decided that it is more a feature request than an actual issue of the library, which I don't want to support, due to the following reasons:

Copying/moving objects by R-value doesn't imply that the object moved from is invalidated. As far as I know the C++ standard says that the behaviour is not specified, what happens with the object moved from, althoug in practice all standard contains gets reset.
I want to support usage cases, in which we possibly want to call a callable multiple times.
Adding a reset for the R-value call case would also burden overhead for usage cases where the callable wrapper is immediately destroyed after usage (I'm personally using function2 as a wrapper for callables dispatched on a single event loop where this applies already).

Implementing a wrapper over function2 that resets its state after invocation is trivial and can easily be implemented on the user side.

The reason for requiring CMake 3.11 is that lower versions don't handle alias imports from interface targets correctly, see https://github.com/Naios/function2/blob/7cca8ada919a2c5ec46e15be4e43c24ada389c78/CMakeLists.txt#L15 for the non compatible line, and there is no reason to support earlier CMake versions when the import doesn't work on those correctly anyway.

The library header isn't requiring the CMake build system and can easily be used standalone which means copying it into your project is supported, if you wish not to use the CMake library import.

tobiasheineken commented 5 years ago

Thanks for your response.

The reason I thought this is a bug is due to the following line in your readme:

one-shot functions which are invalidated after the first call

Do I miss the proper way of using these One-Shot functions?

Naios commented 5 years ago

The readme text could be misleading here: Usually the underlying erased object should be responsible for invalidating its state if called through a R-value qualified operator() (...) &&, but in theory the method could leave the object in any state it likes. The state of the type erase wrapper and the object it calls are not the same, but this behaviour is discussable. For the future I'll make the readme text clearer here regarding this behaviour.

Naios / function2