3

I recently found the equivalent of this code in our code base:

#include <iostream>

struct A
{
    A(int a) : m_a(a) {}

    const int& a() const { return m_a; }
private:
    int m_a;
};

int main()
{
    int a = A(5).a(); // OK
    const int& a_ref = A(10).a(); // Not OK

    std::cout << "a=" << a << std::endl;
    std::cout << "a_ref=" << a_ref << std::endl;
}

Running this with g++ -std=c++11 -Wall -pedantic -Wextra test.cc gives output (gcc version 5.4.0)

5
10

However, adding the -O3 flag gives the result

5
0

This makes sense, as we're returning a reference to m_a which is belongs to the rvalue A(). I'm mostly surprised that the compiler does not complain about this. Right now we've solved it by giving two versions of the function:

const int& a() & const { return m_a; }
int a() && const { return m_a; }

This works, but I'm a little worried that we might have similar issues at other places in the code. Are there any compiler flag that I can turn on that would warn for this?

pingul
  • 3,351
  • 3
  • 25
  • 43
  • Similar question, although no satisfactory answer: https://stackoverflow.com/questions/49396276/is-there-any-c-compiler-which-can-issue-a-warning-for-a-dangling-reference – M.M Jul 02 '18 at 23:15
  • @M.M Thanks, I missed that one. One of the answers there says "it's impossible for the compiler to know" -- I guess this is the parts that confuses me. Sure, it can't know for sure, but that's what I thought the warnings were for -- to warn you that something might be wrong :) – pingul Jul 02 '18 at 23:21
  • Yeah, it seems to me that static analysis should be able to catch some cases at least – M.M Jul 02 '18 at 23:22
  • There is no requirement for a compiler to issue a diagnostic for this kind of undefined behavior. If a compiler is smart enough to figure out this error, that's just an extra bonus, but you cannot rely on your compiler as a safety net. The best way to avoid making these kinds of programming bugs is to make it logically impossible for them to happen. This means using safe programming practices, like refraining from storing or returning plain reference; and instead using smart pointers. If whatever `m_a` is, is actually an object, make it a shared_ptr, and return that. Can't screw that up. – Sam Varshavchik Jul 03 '18 at 00:39
  • In this case, return int always by value. For the broader case, I recommend code review. Returning a shared pointers often gives an unwanted performance overhead for extra memory allocations. On top of that, it makes reasoning about Dtors much harder. I would refrain from it whenever possible. – JVApen Jul 03 '18 at 06:07
  • To give context why the object is not a shared pointer: Our `A` is basically an equivalent of the proposed `std::expected` in that it contains the result from a function together with an error code. The idea is for this to be a thin wrapper over function calls, and so allocating on the heap is not an option. This is obviously a user error that we hopefully would catch in review (at least now with this knowledge), I was mainly looking for ways to automatically catch these mistakes. I appreciate your comment @SamVarshavchik, but I'm still surprised that they _can't_ output this warning. – pingul Jul 03 '18 at 21:46

0 Answers0