8

The Standard doesn't not require a compiler to perform return-value-optimization(RVO), but then, since C++11, the result must be moved.

It looks as if, this might introduce UB to/break code, which was valid in C++98.

For example:

#include <vector>
#include <iostream>

typedef std::vector<int> Vec;
struct Manager{
    Vec& vec;
    Manager(Vec& vec_): vec(vec_){}
    ~Manager(){
        //vec[0]=42; for UB
        vec.at(0)=42;
    }
};

Vec create(){
    Vec a(1,21);
    Manager m(a);
    return a;
}

int main(){
    std::cout<<create().at(0)<<std::endl;
}

When compiled with gcc (or clang for that matter) with -O2 -fno-inline -fno-elide-constructors (I'm using std::vector with these build-option, in order to simplify the example. One could trigger the same behavior without these options with handmade-classes and a more complicated create-function) everything is OK for C++98(-std=c++98):

  1. return a; triggers copy-constructor, which leaves a intact.
  2. Destructor of m is called (must happens before a is destructed, because m is constructed after a). Accessing a in destructor is unproblematic.
  3. Destructor of a is called.

The result is as expected: 21 is printed (here live).

The situation is however different when built as C++11(-std=c++11):

  1. return a; triggers move-constructor, which "destroys" a.
  2. Destructor of m is called, but now accessing a is problematic, because a was moved and no longer intact.
  3. vec.at(0) throws now.

Here is a live-demonstration.

Am I missing something and the example is problematic in C++98 as well?

Mike Kinghan
  • 55,740
  • 12
  • 153
  • 182
ead
  • 32,758
  • 6
  • 90
  • 153
  • 2
    But the `Vec a;` outlives `Manager m;`. – Quimby Apr 24 '19 at 11:12
  • `-fno-elide-constructors` precludes return value optimization and makes it use move instead. Why do you use this flag? – Maxim Egorushkin Apr 24 '19 at 11:13
  • 2
    I don't really get what your question is? There are other ways in which the auto-generated move constructors change behavior from C++98 to C++11 as far as I know (side effects of constructors and destructors are one of the few cases where optimizations are allowed to change observable behavior), so what is special about combining it with https://stackoverflow.com/questions/52931095/exact-moment-of-return-in-a-c-function#52931217? – Max Langhof Apr 24 '19 at 11:15
  • 1
    @MaximEgorushkin with RVO there is no UB, because `a` is not moved and intact. Whether RVO is performed or not is up to compiler, so it should be ok not to perform RVO (or I could come up with a more complicated function for which compiler doesn't perform RVO, but I wanted to keep it simple) – ead Apr 24 '19 at 11:17
  • 1
    I would not call this undefined behaviour, more like implementation defined. The `a` object is still in valid state after being moved from. – Quimby Apr 24 '19 at 11:19
  • @MaxLanghof The problem is that C++98 code gets broken with C++11. Btw. for example in https://stackoverflow.com/q/52931095/5769463 the result isn't moved but copied also in C++11. – ead Apr 24 '19 at 11:20
  • 4
    Lots of C++98 code was broken with C++11. See e.g. https://stackoverflow.com/questions/6399615/what-breaking-changes-are-introduced-in-c11. You seem to imply that this particular code should not have broken, can you tell us what makes you think so? – Max Langhof Apr 24 '19 at 11:22
  • it works without `-fno-elide-constructors` anyway this is clue https://stackoverflow.com/a/27088901/1387438 – Marek R Apr 24 '19 at 11:26
  • @MaxLanghof I could leave with code not compiling. Debugging such a change of behavior is another matter. – ead Apr 24 '19 at 11:26
  • @MarekR used `-fno-elide-constructors` to keep the example simple, the point is that compiler doesn't use RVO (which is valid) – ead Apr 24 '19 at 11:27
  • 1
    I feel your pain, but I still don't see what kind of answer you are looking for here. "Yes, it's valid C++98 code"? – Max Langhof Apr 24 '19 at 11:28
  • @MaxLanghof This is valid C++98 code in my book, but I was sure about many other snippets as well and they weren't. I just cannot believe valid code gets invalid and hope somebody shows me what the problem is. – ead Apr 24 '19 at 11:32
  • Is it valid when the copy Ctor doesn't use const? – JVApen Apr 24 '19 at 11:32
  • 1
    _I just cannot believe valid code gets invalid..._ Why not? Consider `int decltype = 1;`. It's valid in C++03, but invalid in C++11. – Daniel Langr Apr 24 '19 at 11:38
  • the part of the standard you link as "must be moved" only states that it "might be moved" – 463035818_is_not_an_ai Apr 24 '19 at 11:38
  • @DanielLangr introducing a syntax error is different from silently introducing ub – 463035818_is_not_an_ai Apr 24 '19 at 11:39
  • @user463035818 I understand `overload resolution to select the constructor for the copy or the return_­value overload to call is first performed as if the object were designated by an rvalue` as it must take the move constructor. But my starting point was this answer: https://stackoverflow.com/a/17473874/5769463 which might not be 100% correct. – ead Apr 24 '19 at 11:43
  • The catch in this example is in using the target vector object in the main() function as a temporary. It may fool compiler into moving its stuff (data values) away from the vector when it is being both rvod and moved. Use a more straightforward named variable and the problem will disappear. – jszpilewski Apr 24 '19 at 11:50
  • @jszpilewski Actually, that's not so, see: http://coliru.stacked-crooked.com/a/7a21982102be542d – Paul Sanders Apr 24 '19 at 12:30
  • @user463035818 It is. However, the question linked by MaxLanghof shows an example where two different overloads of a function are called in C++03 and C++11. It might be easy to elaborate such a case into UB in C++11 only. – Daniel Langr Apr 24 '19 at 12:33
  • @jszpilewski the problem is the function `create´ and it is the same no matter whether the result is temporary or not. – ead Apr 24 '19 at 12:37
  • @ead The problem is still related to handling temporaries. Notice that the C++98 version will show different results when compiled with or without `-fno-elide-constructors`. In the C++11 version the temporary is created with `move` semantics. – jszpilewski Apr 24 '19 at 13:23

3 Answers3

4

This is not a breaking change. Your code was already doomed in C++98. Imagine you have instead

int main(){
    Vec v;
    Manager m(v);
}

In the above example you access the vector when m is destroyed and since the vector is empty you throw an exception (have UB if you use []). This is essential the same scenario you get into when you return vec from create.

This means that your destructor should not be making assumptions about the state of its class members since it doesn't know what state they are in. To make your destructor "safe" for any version of C++ you either need to put the call to at in a try-catch block or you need to test the size of the vector to make sure it is equal to or greater than what you expected.

NathanOliver
  • 171,901
  • 28
  • 288
  • 402
  • I don't really understand your example. My version is with `Vec v(1);` and there is no problem. I mean, that might be not the best design and begging for problems, but the original code has no UB. – ead Apr 24 '19 at 12:49
  • 4
    @ead Yes, the code you wrote had no UB. The code in my example though does, even in C++98. I'm pointing out that your destructor is not correct for any version of the language because you make an assumption about your vector that you can't know is true. Since you take any vector, that includes empty ones which means your destructor is not correct since it unconditionally access it. – NathanOliver Apr 24 '19 at 12:56
2

"I just cannot believe valid code gets invalid..." Yes, it really may get invalid. Another example:

#include <iostream>
#include <string>

using namespace std;

template <typename T>
int stoi(const basic_string<T>& str)
{ 
  return 0;
}

int main()
{
  std::string s("-1");
  int i = stoi(s);
  std::cout << s[i];
}

The code is valid in C++98/03, but has UB in C++11.


The point is that the code like this or yours are extremal cases, which generally does/should not emerge in practice. They (almost) always represent a very bad coding practice. If you follow good coding habits, you likely won't get into any troubles when moving from C++98 to C++11.

Daniel Langr
  • 22,196
  • 3
  • 50
  • 93
  • @ead Sorry, I switched the numbers. Corrected: https://wandbox.org/permlink/EmDGUD8YmqmzND3l. – Daniel Langr Apr 25 '19 at 06:46
  • @DanielLangr I am sorry, but I cannot understand/see why there is a difference between C++98 and C++11 with this code, despite the live example you added showing the behaviour. Can you explain a bit more what is going on? – LoPiTaL Apr 25 '19 at 06:58
  • 1
    @LoPiTaL The difference is that there is no `std::stoi` in C++98/03. It was added in C++11. Consequently, in C++11, `std::stoi` is invoked (better overload candidate), which converts the `"-1"` string into a number and retunrns -1. In C++03, the only candidate that can be called is our custom `stoi`, which returns 0. BTW this example also shows why `using namespace std;` is pure evil ;-). – Daniel Langr Apr 25 '19 at 07:01
  • maybe yoou are right and my expectations are wrong. I've always assumed move-sematic was introduced in such a way, that it doesn't affect the old code. – ead Apr 25 '19 at 07:26
  • @ead It was and it doesn't. In 99.99 percent of cases of correctly- and well-written code. – Daniel Langr Apr 25 '19 at 07:49
1

Your code exposes various behavior depending on whether the RVO is applied (compiled without -fno-elide-constructors) or with creating a temporary to return the result (with -fno-elide-constructors).

With RVO the result is the same for C++98 and C++11 and it is 42. But introducing a temporary will hide the final assignment to 42 in C++98 and the function will return the result 21. In the C++11 version the things go even further as the temporary is created with move semantics so the assignment to a moved (so empty) object will result in an exception.

The takeaway lesson is just to avoid putting any code with side effects in destructors and constructors as well for this matter.

jszpilewski
  • 1,632
  • 1
  • 21
  • 19
  • Not sure how this answers the question. The whole point of using `-fno-elide-constructors` to avoid the unspecified behavior you are pointing out (see also https://stackoverflow.com/q/52931095/5769463). The problem is that in C++11 it becomes UB, while in C++98 the code might not be beatiful but valid. – ead Apr 24 '19 at 15:06
  • @ead C++98 will present different results depending on compiler options and optimization specified by programmer or just picked by compiler. While it is valid C++ it is a real headache from the software engineering point of view. C++11 does not break anything, just reminds you that you should check the size of an array before accessing it. With that in place you will get results like for C++98. – jszpilewski Apr 24 '19 at 15:22