Avoid unnecessary runtime overhead due to std::string "sink" parameters

[vittorioromeo]

Most of the constructors in cpr take std::string parameters by const std::string&.

Digest(const std::string& username, const std::string& password);

This is not the most efficient way of dealing with "sink" parameters.
Refer to isocpp/CppCoreGuidelines's parameter passing diagram.

My suggestion is using perfect-forwarding, to always make sure sink std::string members are initialized as efficiently as possible. Example:

#define FWD(...) \
    ::std::forward<decltype(__VA_ARGS__)>(__VA_ARGS__)

template<typename TS0, typename TS1>
Digest(TS0&& username, TS1&& password) :
            Authentication{FWD(username), FWD(password)} {}

[whoshuu]

Sounds good to me. Thanks for catching this!

To clarify for documentation purposes, the issue here is that the sink parameters will do a copy into each member parameter for the objects that have std::string members. Using a forwarding reference template parameter will copy lvalues (which is safe and correct) and move rvalues (which is efficient). This is similar to how the Session API has different methods for lvalue payload types and rvalue payload types.

[whoshuu]

Places where this optimization can be done:

https://github.com/whoshuu/cpr/blob/c39b567a2cba6c69ab1ee35b46c9ab214f763520/include/auth.h#L10
https://github.com/whoshuu/cpr/blob/c39b567a2cba6c69ab1ee35b46c9ab214f763520/include/digest.h#L12
https://github.com/whoshuu/cpr/blob/c39b567a2cba6c69ab1ee35b46c9ab214f763520/include/parameters.h#L11
https://github.com/whoshuu/cpr/blob/c39b567a2cba6c69ab1ee35b46c9ab214f763520/include/payload.h#L21
https://github.com/whoshuu/cpr/blob/c39b567a2cba6c69ab1ee35b46c9ab214f763520/include/response.h#L12-L18

[vittorioromeo]

Not sure what the issue is with forwarding c-style strings (as you said in e8e15c7) - could you elaborate?

I used perfect forwarding with std::string lvalues/rvalues and const char* and it seems to handle all the cases correctly. Coliru live example

[vittorioromeo]

Also, I really think having a CPR_FWD(...) macro would make the code much easier to read (and much less error prone). Perfectly forwarding arguments is one of the places where I really think macros can shine.

#define CPR_FWD(...) \
    ::std::forward<decltype(__VA_ARGS__)>(__VA_ARGS__)

Note that:

template<typename T>
void something(T&& x)
{
    // All these are completely equivalent:
    something_else(std::forward<T>(x));
    something_else(std::forward<decltype(x)>(x));
    something_else(CPR_FWD(x));
}

// But the macro version is the one with least code repetition.
// And it's also the shortest one.

[whoshuu]

You're right about the c-style strings. I must've written the forwarding reference constructor wrong the first time and thought the issue was that I was passing c-style strings in the tests when it wouldn't build. This'll be fixed.