cpp:S5417 states that
std::forward has a single use-case: to cast a templated function parameter of type forwarding reference (
T&& ) to the value category (
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:
using fun_t = void(*)(T&&);
void operator()(T args) const
int main(int, const char*)
int *myPtr = nullptr;
Function<int&> &myFun = *(Function<int&>*)(nullptr);
(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
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
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,
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.
The generated assembly is identical (when passing by copy, I see the copy ctor called once with either
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!
This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.