returning const references to local variables

Paul

unread,

Jun 22, 2015, 4:14:39 AM6/22/15

to

Is it ok to return const references to local variables. For example, the code below appears to run fine on my machine but it does give a warning. Thanks.

Paul

/** 2 * Return the maximum item in array a. 3 * Assumes a.size( ) > 0. 4 * Comparable objects must provide operator< and operator= 5 */
template <typename Comparable>
const Comparable & findMax( const std::vector<Comparable> & a ) {
int maxIndex = 0;
int max = a[0];
for( int i = 1; i < a.size( ); ++i )
max = std::max(max, a[i]);

return max;
}

int main()
{std::vector<int> x = {23, 45, 67, 98, 3450, -10};
std::cout << std::endl << findMax(x) << std::endl;
}

Marcel Mueller

unread,

Jun 22, 2015, 5:25:49 AM6/22/15

to

On 22.06.15 10.14, Paul wrote:
> /** 2 * Return the maximum item in array a. 3 * Assumes a.size( ) > 0. 4 * Comparable objects must provide operator< and operator= 5 */
> template <typename Comparable>
> const Comparable & findMax( const std::vector<Comparable> & a ) {
> int maxIndex = 0;
> int max = a[0];
> for( int i = 1; i < a.size( ); ++i )
> max = std::max(max, a[i]);
>
> return max;
> }

This is undefined behavior since max goes out of scope at the end of
findMax and from that point you have a dangling reference.

A smart compiler should warn you about this obvious fault.

Marcel

gwowen

unread,

Jun 22, 2015, 10:38:35 AM6/22/15

to

On Monday, June 22, 2015 at 9:14:39 AM UTC+1, Paul wrote:
> Is it ok to return const references to local variables. For example, the code below appears to run fine on my machine but it does give a warning.

No. Perhaps you're confusing it with the rule that says you can return a temporary, and bind it to a constant reference to extend its life?

For this to apply, the return type is an object type, not a reference to an object.

Öö Tiib

unread,

Jun 22, 2015, 10:40:34 AM6/22/15

to

On Monday, 22 June 2015 11:14:39 UTC+3, Paul wrote:
> Is it ok to return const references to local variables. For example,
> the code below appears to run fine on my machine but it does give a
> warning.

No. It is undefined behavior.
It is good that it gives a warning because defective function
whose behavior is undefined but that appears to run fine in simple
test will (by Murphy's laws) blow everything up in most embarrassing
and damaging for your reputation situation.

> Thanks.

Also there is other defect and it is inefficient.

> /** 2 * Return the maximum item in array a.
3 * Assumes a.size( ) > 0.
4 * Comparable objects must provide operator< and operator=
5 */

Comparable must be of type that is or converts implicitly
to 'int' (for example provides non-explicit 'operator int()') or ...

> template <typename Comparable>
> const Comparable & findMax( const std::vector<Comparable> & a ) {
> int maxIndex = 0;
> int max = a[0];

... or above line is programming error.

> for( int i = 1; i < a.size( ); ++i )
> max = std::max(max, a[i]);
>
> return max;
> }
>
> int main()
> {std::vector<int> x = {23, 45, 67, 98, 3450, -10};
> std::cout << std::endl << findMax(x) << std::endl;
> }

Inefficiency: your algorithm makes pointless copies of the
elements of vector.

There is 'std::max_element' that is more generic (works with
any iteratable sequence) and significantly outperforms
your algorithm on case the elements are cheap to compare
but expensive to copy. I trust I already wrote that in other
quite similar thread that you started.

Richard

unread,

Jun 22, 2015, 1:33:59 PM6/22/15

to

[Please do not mail me a copy of your followup]

Paul <peps...@gmail.com> spake the secret code
<a3d6e8ee-993c-4803...@googlegroups.com> thusly:

>Is it ok to return const references to local variables.

No.
--
"The Direct3D Graphics Pipeline" free book <http://tinyurl.com/d3d-pipeline>
The Computer Graphics Museum <http://computergraphicsmuseum.org>
The Terminals Wiki <http://terminals.classiccmp.org>
Legalize Adulthood! (my blog) <http://legalizeadulthood.wordpress.com>