Skip to content
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.

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

What is the state of a moved-from inplace_function? #113

Closed
Quuxplusone opened this issue Oct 8, 2017 · 4 comments
Closed

What is the state of a moved-from inplace_function? #113

Quuxplusone opened this issue Oct 8, 2017 · 4 comments
Assignees

Comments

@Quuxplusone
Copy link
Contributor

Both folly::Function and fu2::function, as far as I can tell, make the following test case pass:

function<void()> foo = SomeFunctor{};
function<void()> bar;
assert(!bar);
bar = std::move(foo);  // move the state from foo to bar...
assert(!foo);  // ...leaving foo empty. (This assertion fails with inplace_function.)

To make this test case fail, you have to leave foo in a sort of "partly moved from" state, where calling foo() effectively has undefined behavior (because the contained SomeFunctor object you're calling is in a moved-from state), which kind of defeats the purpose of having an "empty" (bad_function_call-throwing) state at all.

On the other hand, if you do make this test case pass, then you end up with a strongly type-safe object type, where the object is fundamentally not allowed to enter a "nonsense" or "partly-moved-from" state. This strikes me as a good idea. I think inplace_function should follow the example of Folly here, and make this test case pass.

@carlcook
Copy link
Member

Interesting, I'll take a closer look at Folly. So this would mean we don't throw upon a call to an unset inplace function? This is not how std::function works, but maybe that's okay.

@Quuxplusone
Copy link
Contributor Author

Quuxplusone commented Oct 11, 2017

this would mean we don't throw upon a call to an unset function?

Incorrect. I like the throwing behavior (although there are definitely people who think that throwing on a logic error is not useful and would rather UB in that case). This is about whether a moved-from function should become "empty" (Folly, fu2) or become "unspecified/invalid/UB" (inplace_function).

function<void()> foo = SomeFunctor{};
function<void()> bar;
assert(!bar);
bar = std::move(foo);  // move the state from foo to bar...
assert(!foo);  // ...leaving foo empty. (This assertion fails with inplace_function.)
foo();  // Since foo is empty, this should throw bad_function_call. With inplace_function, it has UB instead.

You're correct that std::function behaves like inplace_function here: it becomes invalid/UB on move-from, and calling it becomes UB. But all third-party libraries seem to be converging on the idea that having gratuitous UB in your API is a bad idea. ;)

@carlcook
Copy link
Member

I like the throwing behavior (although there are definitely people who think that throwing on a logic error is not useful and would rather UB in that case). This is about whether a moved-from function should become "empty" (Folly, fu2) or become "unspecified/invalid/UB" (inplace_function).

Ah I see, thanks, and agreed. I'm happy to keep the throw on calling an unset fn, and UB for calling a moved-from fn. I don't mind this being UB (at least until someone protests about this big time)

@Quuxplusone
Copy link
Contributor Author

Hmm, I forget why I closed this via 56feddf; that commit doesn't look relevant!

This std-proposals thread got me thinking: If we allow "move-from" to actually make the source object empty, instead of putting it into the moved-from state, then we can express inplace_function::move in terms of ContainedType::relocate, which can get us a pretty big performance improvement for trivially relocatable types (which is basically all types — for example, decltype([x = std::string(), y = std::make_unique<int>(), z=std::function<int()>()]{}) is trivially relocatable (although the compiler has no way of telling us this as of C++17).

So I think there will be a big advantage to making moved-from inplace_function objects become empty. We can't really get this big advantage until C++[23][0-9], but I think it makes sense to design for it. It will also simplify the code right away (by replacing our move_ptr with a relocate_ptr and making half the number of virtual dispatches during move and swap operations). I'll work up a patch Real Soon Now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants