4

This question is related to Does this C++ static analysis rule make sense as is?, but is somewhat different. I've now implemented a static analysis rule to find cases where a function returns its const reference parameter as a reference, such as

const X& f(const X& x) { return x; }

This is potentially dodgy when a temporary is bound to x, because the lifetime of the temporary will end before the return value of f can be bound to a reference in the caller. To put it another way, this would be problematic:

const X& r = f(X());

On running the rule, I'm finding an implementation of min in the standard library that looks like this:

template<typename _Tp>
inline const _Tp&
min(const _Tp& __a, const _Tp& __b)
{
  // concept requirements
  __glibcxx_function_requires(_LessThanComparableConcept<_Tp>)
  //return __b < __a ? __b : __a;
  if (__b < __a)
    return __b;
  return __a;
}

This clearly returns its const reference parameters as references, but the function is inline. Does this make a difference in terms of temporary lifetime, or is this genuinely a bit dodgy? The function is marked with the following comment, so it's clearly intended to be callable on temporaries:

*  This is the simple classic generic implementation.  It will work on
*  temporary expressions, since they are only evaluated once, unlike a
*  preprocessor macro.
Community
  • 1
  • 1
Stuart Golodetz
  • 20,238
  • 4
  • 51
  • 80
  • 2
    You are not supposed to store a reference to the result of `min`, but copy or use the value. Also, inlining must not change the semantics of code, it is just an optimization. – Bo Persson Jul 18 '12 at 10:51
  • Ok thanks :) It sounds like I should be finding it as a violation of the (possibly overly strict) rule, which presumably is intended to find things that could (rather than necessarily will) lead to bugs. – Stuart Golodetz Jul 18 '12 at 10:55
  • I believe this is a deliberate design in the standard library, to avoid copying values in the `min` function. If you want to know which specific element is the smallest, there is also a `min_element` function that returns an iterator. – Bo Persson Jul 18 '12 at 10:59
  • Take a look here: http://herbsutter.com/2008/01/01/gotw-88-a-candidate-for-the-most-important-const/ – MadScientist Jul 18 '12 at 11:12
  • @MadScientist: Thanks for the link - that's a somewhat different case though because the function there is not returning a reference. The code there is fine. – Stuart Golodetz Jul 18 '12 at 11:16

1 Answers1

1

Whether a function is inline or not makes absolutely no difference in the lifetime of any temporaries. Binding the return value of a function like your f (or std::min) to a const reference with local scope is a sure way to end up with a dangling reference. But I don't see where there's any real danger, since it's not something you would do in reasonable code, unless the reference is to something long lived in the first place, e.g.:

T const& obj = std::min( myMap['x'], myMap['y'] );

Except in such special cases, local variables will be values, not references.

James Kanze
  • 150,581
  • 18
  • 184
  • 329
  • Thanks James. I agree that it's likely not a problem in practice - some of the rules I have to implement are unfortunately quite strict (which is why I'm doing my best to cut down on the number of false positives). – Stuart Golodetz Jul 18 '12 at 10:59