Cppcheck says 'Reference to temporary returned' but code runs fine. False positive maybe?

140 Views Asked by At

Consider the following mwe, implementing a simple class that overloads the multiply operator:

#include <iostream>

using namespace std;

class A {
public:
  double a;
  
  A &operator*(double b)
  {
        a *= b;
        return *this;
  }
    
  friend A &operator*(double b, A &m) { return m * b; } // (*)
};

int main()
{
    
    A a;
    a.a = 5.0;
    a * 3.0;
    std::cout << a.a << std::endl;
    3.0 * a;
    std::cout << a.a << std::endl;
    return 0;
}

It compiles and runs just fine, printing the expected output. However cppcheck gives the following warning.

tmp.cpp:15:48: error: Reference to temporary returned. [returnTempReference]
friend A &operator*(double b, A &m) { return m * b; }

When rewriting (*) as

friend A &operator*(double b, A &m) { m * b; return m; }

The error from cppcheck disappears.

So, is this an actual problem and I'm overseeing something, or is it a false positive by cppcheck?

Some context: In my code, class A is actually a Matrix class, for which I don't want to create a new object upon multiplying with a constant, thus returning the reference.

3

There are 3 best solutions below

8
463035818_is_not_an_ai On BEST ANSWER

Your code does not return a reference to a temporary. One is inclided to think that, because what you implemented as operator* is actually an operator*=. If you did implment operator* which does not modify the current A but returns a new one, then returning a reference would be problematic.

I dont have the tool available, but I would expect it to not complain about:

#include <iostream>

using namespace std;

class A {
public:
  double a;
  
  A &operator*=(double b)
  {
        a *= b;
        return *this;
  }
    
  friend A &operator*=(double b, A &m) { return m *= b; } // (*)
};

int main()
{
    
    A a;
    a.a = 5.0;
    a *= 3.0;
    std::cout << a.a << std::endl;
    3.0 *= a;
    std::cout << a.a << std::endl;
    return 0;
}

Which really does the same as your code, but with the expected semantics of *=.

As songyuanyao mentions in a comment, it might be that the tool simply expects m*b to be a rvalue-expression, as it usually is, without actually checking the implementation of your *.

0
MSalters On

It looks like a misleading warning. I wouldn't call it a "false positive", because this sort of code does warrant a warning, but a different warning.

Specifically, the member operator* should be const, and return a temporary by value. This overload of operator* is clearly supposed to be a product (in mathematical sense) considering it commutes.

I understand the idea of multiplying with a constant, and not creating a temporary. You can express that as Matrix operator*(Matrix&& lhs, float rhs). The rvalue reference makes explicit that lhs is altered.

2
Yksisarvinen On

It is a false positive on cppcheck side, but also your code is highly confusing to any readers (as shown in comments) because you don't obey basic rules and idioms for operator overloading. It may or not be related to cppcheck mistake.

Proper implementation of binary operator * should return a copy:

class A {
public:
  double a;
  // no need for friend declarations, your data member is public
};

A operator*(A a, double b)
{
    A.a *= b;
    return a;
}

A operator* (double b, A a)
{
    return a * b;
}