std::forward has a single use-case: to cast a templated function parameter of type forwarding reference ( T&& ) to the value category ( lvalue or rvalue ) the caller used to pass it. This allows rvalue arguments to be passed on as rvalues , and lvalues to be passed on as lvalues . This scheme is known as perfect forwarding . Note that the standard states that “a forwarding reference is an rvalue reference to a cv-unqualified template parameter that does NOT represent a template parameter of a class template” . Refer to the last noncompliant code example.
I believe the description is too restrictive, std::forward can also be useful to forward non forwarding references. Consider the following code:
#include <utility>
template<typename T>
class Function
{
private:
using fun_t = void(*)(T&&);
fun_t m_fun;
public:
void operator()(T args) const
{
m_fun(std::forward<T>(args));
}
};
int main(int, const char*[])
{
int *myPtr = nullptr;
Function<int&> &myFun = *(Function<int&>*)(nullptr);
myFun(*myPtr);
return 0;
}
(ignore the fact that it crashes when dereferencing a nullptr, I tried to simplify this as much as possible)
The code above compiles fine but triggers cpp:S5417. Changing the std::forward<T> to std::move as recommended results in the following compilation error:
Error C2664 ‘void (T)’: cannot convert argument 1 from ‘int’ to ‘T’
I’d be curious to know how this code should be refactored to become compliant with cpp:S5417
As you have correctly mentioned, there exist the cases where the std::forward<T> can be used on not forwarding reference (like in your example or cases of reference wrappers). However, as they are both rare and hard to detect, we decided to still raise issue in such case. Once, you have confirmed that this is intent, I would recommend closing the issue on the SonarQube/SonarCloud side.
In such cases, the rure is intedent to work as review helper, to suggest double checking this particular and uncommon use, and leaving appreciate comment.
Thanks for the quick reply and confirmation. I will proceed with closing these on the SonarQube side. I would suggest rephrasing the description to inform about this other use case, in particular " std::forward has a single use-case" makes it sound like anything else is a misuse of std::forward.
The emphasis on the “NOT” in the quote makes this description even more authoritative, suggesting that we should never use std::forward when the template parameters are set on the class instead of the method.
In my case, I ended up having to read specifications, prototype alternatives, etc. for a good hour before I felt confident that the description was inaccurate and decided to post here to double check. I cannot expect every developer to go through this process.
Maybe relaxing the description to “The principal use case for std::forward is to cast a templated function parameter”
In addition, an example of std::forward that will be marked as non-compliant but can safely be ignored (ex. the case presented here) could be included in the examples of the description.
I recognize that std::forward<T>(t) can be seen as nicer spelling for static_cast<T&&>(t), which conveys the intent of forwarding a forwarding/universal reference cleaner. However, using it as just a replacement for the above cast is missing the purpose and may mislead the readers of the code.
One alternative that I haven’t mentioned before would be to simply use static_cast<T&&>(t) in your code instead of std::forward<T>(t) - they are equivalent in this context.
Wow. I had not realized that in this context, std::forward<T> and static_cast<T&&> are equivalent. It took me a while to process - my first thought was that static_cast<T&&> would not work on variadic templates, but it does (i.e. m_fun(static_cast<T&&>(args)...)
The generated assembly is identical (when passing by copy, I see the copy ctor called once with either std::forward<T> or std::static_cast<T&&>, as opposed to twice when I don’t static_cast or std:forward at all.
It is awesome to get this level of quality feedback so quickly, thanks again!