4
votes

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.
1
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
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
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
@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

1 Answers

1
votes

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.