Remove complicated syntax in argument_loader::call()

[rhaschke]

There doesn't seem to be the need for std::move(*this).

[wjakob]

• @jagerman, who designed this this implementation.

[jagerman]

The reason for the rvalue qualification on the methods is that call_impl moves away the argument casters, which is not a good thing to do if the caller context isn't itself an rvalue, and so I think those should stay. The std::move(*this).template elimination seems okay: call_impl isn't actually rvalue qualified and is private, so the move there seems not actually needed.

[rhaschke]

Actually, the caller context isn't an rvalue as call_impl isn't declared as an rvalue method.
Do I miss something?

[jagerman]

Actually, the caller context isn't an rvalue as call_impl isn't declared as an rvalue method.
Do I miss something?

I'm talking about the caller of call(). You're right that the std::move for the call_impl invocation doesn't need the move.

[rhaschke]

I think I am confused because call() has the rvalue qualifiers, while call_impl() has not.
I would expect both to have the same qualifiers.
I agree that having them nicely indicates that the internal argcasters are moved away.
On the other hand, removing them (everywhere) greatly simplifies the syntax.

[jagerman]

I think you misunderstood me; I probably wasn't sufficiently clear:

• leave the && qualifications on call() so that it can't be called in non-rvalue context (because it moves away internal members)
• simplify away the std::move() on the internal call to call_impl because it isn't needed: it's a private, internal implementation method (yes, it moves away members, but it's private so it seems fine to not requiring that it also be &&-qualified, and just remove the unneeded std::move(...) where it is invoked).

[rhaschke]

I deliberately removed all &&-qualifiers to illustrate that they are not required.
I'm still convinced that you should use a consistent approach of using &&-qualifiers everywhere or never.

[jagerman]

I deliberately removed all &&-qualifiers to illustrate that they are not required.

They are not required for compilation, but they serve a significant purpose on the public method: they signal that this method leaves the object in an undefined state (with the internal data moved away), and prevent it from being called in an lvalue context where that would be inappropriate. There currently is no such call, of course, and that's the point: such a call would intentionally fail to compile, but if you remove the && qualifier it won't fail to compile, and that to me is a bug.

[jagerman]

(If you want to go the other way and add && to the internal private method, that's fine and arguably more correct)

[rhaschke]

If you want to go the other way and add && to the internal private method, that's fine and arguably more correct

That's fine with me too. Hence, I filed a new PR #2057 and will close this one here.