13

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);
curiousguy
  • 8,038
  • 2
  • 40
  • 58
Stuart Golodetz
  • 20,238
  • 4
  • 51
  • 80
  • @Nawaz: I'm using .QL to write queries over large code-bases :) I don't think it should matter what compiler, I'm ideally looking for a platform-independent answer. – Stuart Golodetz Jul 18 '12 at 09:47
  • 2
    Why I asked because MSVC++ provides compiler extension which allows temporaries to bind to non-const references. And if you're using Microsoft static analysis tool, then it might consider this extension as well. – Nawaz Jul 18 '12 at 09:48
  • @Nawaz: Ah, right :) In that case, I should probably provide an optional extension to the rule for VC++. Thanks! (I have to say I'm not a fan of compiler extensions like this in general though.) – Stuart Golodetz Jul 18 '12 at 09:51
  • I'm not using a Microsoft static analysis tool, it's an in-house one - but it sounds like I need to add some Microsoft-specific stuff in there. – Stuart Golodetz Jul 18 '12 at 09:55
  • You can just disable the binding by adding the `/Za`(disable language extensions) flag. – Xeo Jul 26 '12 at 01:59
  • @Xeo: Most of the Microsoft header files don't compile if you do that - or at any rate they didn't the last time I checked :) – Stuart Golodetz Jul 26 '12 at 08:41
  • I don't know when the last time was for you, but I have no trouble compiling my C++ code with `/W4` and `/Za`. – Xeo Jul 26 '12 at 15:10
  • @Xeo: Fair enough, I'll check again - I may be thinking of older versions of VC++, where /Za was essentially an option you could never actually use (which was frustrating). I confess I haven't tried it lately, as I've been primarily coding on non-Windows platforms in languages other than C++ for a bit. Either way, it unfortunately doesn't help me here, as people using the tool may not have /Za set on their own code - I'll still need to make it work for them. – Stuart Golodetz Jul 27 '12 at 00:14

2 Answers2

8

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
Luc Touraille
  • 79,925
  • 15
  • 92
  • 137
ecatmur
  • 152,476
  • 27
  • 293
  • 366
1

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.)

Brangdon
  • 627
  • 5
  • 7
  • Thanks, good points, especially for the future. The context of this is that I'm implementing a fairly draconian rule set for our commercial tool - they're pinned down by a standard, so I have to be careful about deviating too far from the letter of the rules. That said, the aim is to reduce false positives wherever possible. We don't yet support C++11, but when we do I would certainly be inclined to modify the rule to take rvalue reference overloads into account. – Stuart Golodetz Jul 24 '12 at 22:19
  • One of the cases where returning a reference to an rvalue parameter is "safe" is with `std::move` and `std::forward`, which may both take rvalue arguments. IOW, if you're planning to pass the result of such a function directly to another expression, whichever it may be (like another function). – Xeo Jul 26 '12 at 02:02