Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Does this C++ static analysis rule make sense as is?

I'm implementing some C++ static analysis rules, and one of them prohibits a function from returning a reference or pointer to a reference parameter of the function, i.e. the following are all non-compliant:

int *f(int& x) { return &x; } // #1
const int *g(const int& x) { return &x; } // #2
int& h(int& x) { return x; } // #3
const int& m(const int& x) { return x; } // #4

The justification given for this is that "It is implementation-defined behaviour whether the reference parameter is a temporary object or a reference to the parameter."

I'm puzzled by this, however, because stream operators in C++ are written in this way, e.g.

std::ostream& operator<<(std::ostream& os, const X& x) {
    //...
    return os;
}

I think I'm pretty confident that stream operators in C++ do not in general exhibit implementation-defined behaviour, so what's going on?

According to my understanding as it is at present, I would expect #1 and #3 to be well-defined, on the basis that temporaries cannot be bound to non-const references, so int& x refers to a real object that has lifetime beyond the scope of the function, hence returning a pointer or reference to that object is fine. I would expect #2 to be dodgy, because a temporary could have been bound to const int& x, in which case trying to take its address would seem a bad plan. I'm not sure about #4 - my gut feeling is that that's also potentially dodgy, but I'm not sure. In particular, I'm not clear on what would happen in the following case:

const int& m(const int& x) { return x; }
//...
const int& r = m(23);
like image 965
Stuart Golodetz Avatar asked Jul 18 '12 09:07

Stuart Golodetz


2 Answers

As you say, #1 and #3 are fine (though #1 is arguably bad style).

#4 is dodgy for the same reason #2 is; it allows propagating a const reference to a temporary past its lifetime.

Let's check:

#include <iostream>

struct C {
  C() { std::cout << "C()\n"; }
  ~C() { std::cout << "~C()\n"; }
  C(const C &) { std::cout << "C(const C &)\n"; }
};

const C &foo(const C &c) { return c; }

int main() { 
   const C &c = foo(C());
   std::cout << "c in scope\n";
}

This outputs:

C()
~C()
c in scope
like image 113
ecatmur Avatar answered Oct 22 '22 19:10

ecatmur


In C++11, #2 and #4 can be made safe if there are also rvalue reference overloads. Thus:

const int *get( const int &x ) { return &x; }
const int *get( const int &&x ) { return nullptr; }

void test() {
    const int x = 0;
    const int *p1 = get( x ); // OK; p1 is &x.
    const int *p2 = get( x+42 ); // OK; p2 is nullptr.
}

So although they are dodgy, they do have safe uses if the programmer knows what they are doing. It'd be draconian to forbid this.

(Perhaps safer would be if the const rvalue reference overload was made private, left undefined, or otherwise caused a compile-time or link-time error. This is especially true for the #4 case, where we return a reference but there is nothing good to return a reference to and the language doesn't allow references to null.)

like image 38
Brangdon Avatar answered Oct 22 '22 19:10

Brangdon