1

I have these codes below. What's wrong with the const auto& in this context, it caused unexpected outputs.

It works fine while compiled with gcc-4.8.5, but gets unexpected outputs with gcc-4.9.2.

If I remove the & in const auto&, it works fine with both gcc versions.

// max_dim is a protobuf filed: const auto max_dim = msg.max();
// cat2_num is an element in std::vector<int32_t>: const auto cat2_num = vec[i]
const auto& res_num = std::max(1, std::min(max_dim, cat2_num));
LOG(ERROR) << res_num << ", " << max_dim << ", " << cat2_num
   << ", " << std::max(1, std::min(max_dim, cat2_num));

outputs:

 -1392522416, 3, 1, 1
 2, 3, 2, 2
 3, 3, 3, 3
 -1392522416, 3, 1, 1
 3, 3, 6, 3
 2, 3, 2, 2
 -1392522416, 3, 1, 1
 -1392522416, 3, 1, 1
 2, 3, 2, 2

=========== updated ========

I couldn't reproduce the undefined behavior with these codes:

#include <iostream>
#include <vector>

int main() {
    std::vector<int32_t> v = {-1, 0, 1, 2, 3, 6};
    const int a = 3;
    const auto& b = a;

    for(size_t i = 0; i < v.size(); i++) {
        const auto& c = v[i];
        const auto& d = std::max(1, std::min(b, c));
        std::cout << d << ", " << b << ", " << c << std::endl;
    }
    return 0;
}

output:

1, 3, -1
1, 3, 0
1, 3, 1
2, 3, 2
3, 3, 3
3, 3, 6
Roy Huang
  • 579
  • 3
  • 23
  • 1
    Please provide a [mcve] – 463035818_is_not_an_ai Apr 13 '20 at 14:45
  • 1
    You have dangling reference to prvalue 1 - it means, that you have a reference to a temporary integer. It is valid until the nearest sequence point (semicolon here). So this reference points to "nowhere" when you try to access a value. When you use `const auto` the value returned from `std::max` will be copied and you can use it normally – Georgy Firsov Apr 13 '20 at 14:50
  • 1
    @GeorgyFirsov Temporaries are valid until the end of a full-expression, which can contain multiple sequence points. – aschepler Apr 13 '20 at 14:54
  • @aschepler, my mistake, I aggree with you – Georgy Firsov Apr 13 '20 at 14:56
  • 1
    Re: "I couldn't reproduce the undefined behavior" -- "undefined behavior" means **only** that the language definition does not tell you what the program does. It does not mean that something bad will happen. – Pete Becker Apr 13 '20 at 16:23
  • Stepanov covers the design and issues of `std::min` and `std::max` in his [Efficient Programming with Components](https://www.youtube.com/playlist?list=PLHxtyCq_WDLXryyw91lahwdtpZsmo4BGD) lectures. – badola Apr 13 '20 at 22:33

2 Answers2

10

Your code has undefined behavior. In

const auto& res_num = std::max(1, std::min(max_dim, cat2_num));

The 1 is a prvalue, so a temporary integer is created that gets bound to the function parameter. This would be okay of max was like

template <typename T> const T max(const T&, const T&);

but instead it is defined like

template <typename T> const T& max(const T&, const T&);

So, if that 1 happens to be the maximum value, then max returns to you a reference to that temporary object that was created. After that, the temporary object is destroyed1, leaving res_num as a dangling reference. To fix the code make res_num a non-reference like

const auto res_num = std::max(1, std::min(max_dim, cat2_num));

and now you get a copy of the correct value.

1: all temporaries are destroyed at the end of the full expression the are created in

NathanOliver
  • 171,901
  • 28
  • 288
  • 402
  • Maybe a bit clearer if you also note the signature of `std::max` is `template const T& max(const T&, const T&);` – aschepler Apr 13 '20 at 14:51
  • Since it's a const reference, wouldn't the lifetime of the temporary get extended? – cigien Apr 13 '20 at 14:52
  • 1
    @cigien No. Lifetime extension is not passed through functions. You only get lifetime extension when you assign a prvalue to a function local `const T&` or a `T&&`. The return of `max` is not a prvalue, so no lifetime extension. – NathanOliver Apr 13 '20 at 14:54
  • @aschepler, in this case it is even clearer to note some type inference rules for `auto` - it throws away cv-qualifiers an references. So `const auto = std::max(...)` will have `const int32_t` type (as I understand author uses `int32_t` here), because the reference was thrown away by `auto` – Georgy Firsov Apr 13 '20 at 14:55
  • @aschepler I've added that in. – NathanOliver Apr 13 '20 at 14:55
  • "function local" is not necessary. Temporary lifetime extension can also apply to references at namespace scope or static class members. – aschepler Apr 13 '20 at 15:07
  • Cool. But I couldn't reproduce the undefined behavior with buggy codes, as I updated in the question. – Roy Huang Apr 13 '20 at 15:44
  • 1
    @RoyHuang The "fun" thing about undefined behavior is that the expected result is also part of the set of results that the program can have. As you can see [here](http://coliru.stacked-crooked.com/a/e1400ddb7f61a4ef), I get different results. – NathanOliver Apr 13 '20 at 15:47
  • @NathanOliver Thanks. I have [reproduced it too](http://coliru.stacked-crooked.com/a/b7b5dafb5499f82e)。 And I have tested that it has nothing to do with gcc version, I reproduced it with both gcc-4.8 and gcc-4.9. The only cause is just as you clarified. – Roy Huang Apr 13 '20 at 16:06
0

@NathanOliver has given clear declaration for the cause. Here is some other related infomation.

  1. const T& is helpful for large objects, but it involves lifetime and aliasing problem. So be careful.

  2. For int, double, pointers values, const T& gains nothing. In this situation, copy is cheaper than reference.

reference: int vs const int&

Roy Huang
  • 579
  • 3
  • 23