3

I've just found something really weird, check out this code:

#include <cstring>
#include <cstdio>
#include <utility>
#include <vector>

using namespace std;

class A {
    public:
    int n = 0;
    A(const char* p) { n = strlen(p); };

    A(A const&) = delete;
    void operator=(A const&) = delete;
};
void F(vector<pair<const char*, const A&>> v) {
    printf("F\n");
    for(vector<pair<const char*, const A&>>::iterator it = v.begin();it!=v.end();++it) printf("  '%s': %p %i\n", it->first, &it->second, it->second.n);
};

int main(int, char**) {
    F({
            { "A", "A" },
            { "B", "BB" },
            { "C", "CCC" },
            { "D", "DDDD" }
        });
};

Now compile it with clang++ -std=c++11 -Wall -Wextra -Wpedantic -O0 main.cc -o main, or something similar (with optimization disabled).

And you should see an output like this:

F
  'A': 0x7fff57a0b988 1
  'B': 0x7fff57a0b9d0 2
  'C': 0x7fff57a0ba18 3
  'D': 0x7fff57a0ba60 4

Which is nice, the compiler automatically creates the vector object and the corresponding A& references, which are all different.

Now, compile it with clang++ -std=c++11 -Wall -Wextra -Wpedantic -O1 main.cc -o main, notice how I just added the lowest level of optimization.

And you would see,

F
  'A': 0x7fff5ac54b30 1629262454
  'B': 0x7fff5ac54b30 1629262454
  'C': 0x7fff5ac54b30 1629262454
  'D': 0x7fff5ac54b30 1629262454

that all parameters reference the same A& object, which I find wrong.

Here are my compiler details:

Apple LLVM version 6.0 (clang-600.0.57) (based on LLVM 3.5svn)
Target: x86_64-apple-darwin14.1.0
Thread model: posix

Is this expected behavior? Is this a compiler bug?

Update: As pointed out by Matt McNabb, vectors and initializer_lists are not designed to work with const references (although, it compiles fine in clang). However, when writing the same function as void F(vector<pair<char*, A&&>>) {} the error still persists.

Ale Morales
  • 2,728
  • 4
  • 29
  • 42
  • 1
    You shouldn't put a `;` after functions. – rlbond Jul 23 '15 at 23:45
  • `vector v;` causes 20KB of error messages from g++ – M.M Jul 23 '15 at 23:52
  • @MattMcNabb, Thanks, does `initializer_list l;` throws anything? Sorry, I don't have g++ at hand. – Ale Morales Jul 23 '15 at 23:55
  • Only 16 lines of errors for that one :) – M.M Jul 23 '15 at 23:57
  • Damn, absolutely no errors in clang for both cases (and I'm being -Wpedantic here); hence why I assumed everything was fine. I'd definitely have to work around this thing, I'll try with variadic templates. – Ale Morales Jul 24 '15 at 00:01
  • If you give `A` a move-constructor, then you can write `vector> a; a.emplace_back(make_pair("A", "A")); F(a);` although I can't see how to make it work using braced initialization – M.M Jul 24 '15 at 00:06
  • I've added the lawyer tag as we're going to need an expert to unravel what the standard has to say here :) – M.M Jul 24 '15 at 01:01
  • Both GCC and Clang destroy the temporaries [before you enter `F`](http://coliru.stacked-crooked.com/a/5955a52289acecae). – T.C. Jul 24 '15 at 03:54

3 Answers3

4

Your code seems a bit odd. You're doing this:

void F(vector<pair<const char*, const A&>> v) {

So you're expecting a vector with references to A objects. But you don't have any A objects. You're passing it string literals, for which the compiler implicitly creates A objects - but those are temporary, so by the time your function body runs, they are already gone, and referencing them is undefined behavior, which is why it works with -O0, but not -O1.

If you want to implicitly create A objects and then keep them, you can't use references. Try

void F(vector<pair<const char*, const A>> v) {

Example here: http://ideone.com/VNIgal

EboMike
  • 76,846
  • 14
  • 164
  • 167
  • 1
    That makes sense, but, what's the point of a compiler implicitly constructing objects if they are not going to be available in the immediate function call that needs them? – Ale Morales Jul 23 '15 at 23:29
  • C++ is a very powerful language and lets you do a lot of things. Which means that you can break things in great ways if you're not careful. Implicit construction can be very powerful, and in the example I'm giving above, it works the way you expected it to. – EboMike Jul 23 '15 at 23:34
  • 4
    [Vectors aren't meant to hold const objects](http://stackoverflow.com/questions/6954906/does-c11-allow-vectorconst-t) – M.M Jul 23 '15 at 23:36
  • Yeah I understand, thing is that in my actual situation, A is very expensive to copy, so I have purposedly disabled its copy constructor. – Ale Morales Jul 23 '15 at 23:37
  • @almosnow perhaps you should also disable the copy constructor in this question, so that when people suggest workarounds they will be applicable to your real code – M.M Jul 23 '15 at 23:45
  • That is the rule for C++. These are temporary objects. If you add some output to the constructor and destructor of `A` you will see that they are destroyed before the call to `F`. http://ideone.com/ZFCVq1 – rlbond Jul 23 '15 at 23:49
  • If `A` is expensive to copy, you can use [`emplace_back`](http://en.cppreference.com/w/cpp/container/vector/emplace_back)... – rlbond Jul 23 '15 at 23:50
  • @MattMcNabb Didn't knew that about vectors. Tried the same thing but with `initializer_list`s instead, the same behavior occurs. Either `initializer_list` is not designed for const objects, or something else is happening. – Ale Morales Jul 23 '15 at 23:51
  • 1
    @rlbond I know they are temporary, but again, what would be the point of creating a temporary object to use in a function call if it's going to dissapear before the function enters. They should not be removed until they get out of scope. – Ale Morales Jul 23 '15 at 23:53
  • @rlbond that doesn't prove anything - it could be a compiler bug – M.M Jul 23 '15 at 23:59
  • my comment was directed to @EboMike: the suggested "solution" of `pair` is not supported by the C++ standard. (I don't have any other suggested solutions however, except "don't do this at all") – M.M Jul 24 '15 at 00:00
  • @almosnow the temporaries do *not* disappear before they are used for a function call -- they are passed just fine to the `vector>` constructor. – rlbond Jul 24 '15 at 00:27
  • @MattMcNabb http://en.cppreference.com/w/cpp/language/reference_initialization#Lifetime_of_a_temporary "a temporary bound to a reference member in a constructor initializer list persists only until the constructor exits, not as long as the object exists." – rlbond Jul 24 '15 at 00:41
  • Although I can't find that statement in the standard. And when you change the second template argument to an `A`, the destruction does not happen until after the end of the full-expression. – rlbond Jul 24 '15 at 00:52
  • @rlbond "constructor initializer list" means the stuff introduced by `:` in the constructor definition (not a std::initializer_list being passed to a constructor) – M.M Jul 24 '15 at 00:54
  • 2
    Changing the call to `F({ {"A", A("A")},{"B", A("BB")},{"C", A("CCC")},{"D", A("DDDD")} });` seems to fix the problem. I don't see how this could be anything other than a compiler bug. The implicit `A` objects should live until the end of the full-expression. – rlbond Jul 24 '15 at 01:04
  • @rlbond For the record, are you on clang or gcc ? – Ale Morales Jul 24 '15 at 03:06
  • "until the constructor exits". Which constructor? If you mean the constructor of the `vector`, then this makes sense - the temporaries are destroyed before the body of function `F` begins. Or do you mean the contructor of `A`? – Aaron McDaid Jul 27 '15 at 19:52
2

This is due to the constructor of a pair that takes forwarding references:

template<class U, class V> pair (U&& a, V&& b);

When creating the initializer_list that is passed to the vector constructor, temporary pairs are created. Each pair is constructed using the initializers provided. For example, when you construct the first pair with

{ "A", "A" }

The compiler finds that the best matching constructor is template <typename U,typename V> pair(U&&,V&&) with U and V being const char (&)[2]. This means that a temporary A is not created at the call site. The temporary A is created internally to the pair constructor:

template<class U, class V> pair (U&& a, V&& b)
: first(std::forward<U>(a)), 
  second(std::forward<V>(b))
  // second is of type const A&, but std::forward<V>(b) is an rvalue
  // of type const char [2]
{
}

Since the argument type (const char (&)[2]) doesn't match the member type (const A &), a temporary is created to initialize the member, but this temporary only lasts until the constructor exits. That means your pair is left with a dangling reference.

One way to avoid this would be to explicitly create the temporaries at the call site:

F({
        { "A", A("A") },
        { "B", A("BB") },
        { "C", A("CCC") },
        { "D", A("DDDD") }
    });

Now the temporaries will last until F returns, and no temporaries will be created inside the pair constructor since the types match.

Note that if you had the C++03 form of std::pair, which doesn't have forwarding references, the behavior would be what you expect. The temporaries would be created inside main and would last until the call to F() returns.

Vaughn Cato
  • 63,448
  • 5
  • 82
  • 132
  • "a temporary is created to initialize the reference, but this temporary only lasts until the constructor exits" According to the standard, temporaries are not destroyed until the end of the full-expression (12.2.3). I don't think this answer is correct. – rlbond Jul 27 '15 at 16:15
  • @rlbond: The temporaries are created inside the `pair` constructors, not inside `main()`, so as soon as the pair constructors return, the temporaries are destroyed. I think the confusion comes because the expectation is that the pair constructor is taking a `const char *` and an `A&`, but actually, because the pair has a constructor taking forwarding references, the pair constructors take whatever is passed in. – Vaughn Cato Jul 27 '15 at 18:26
  • "Temporary objects are destroyed as the last step in evaluating the full-expression (1.9) that (lexically) contains the point where they were created" The full-expression ends at the semicolon. The standard is clear here -- the destructors to the implicitly-created pairs should not be destroyed until the end of the full-expression. Nowhere in the standard does it say that the temporaries used in a constructor are destroyed at the end of that constructor. – rlbond Jul 27 '15 at 18:45
  • @rlbond: That would be true if the temporaries were scoped inside main, but they aren't. The pair constructor is called without any temporaries being created, then the temporaries are created inside the pair constructor, and then the pair constructor returns, destroying the temporary. Then the next pair constructor is called, another temporary is created and destroyed, etc. – Vaughn Cato Jul 27 '15 at 18:49
  • @rlbond: The relevant standard reference is 12.2.5: "— A temporary bound to a reference member in a constructor’s ctor-initializer (12.6.2) persists until the constructor exits." – Vaughn Cato Jul 27 '15 at 18:57
  • You're saying that the initializer_list is of type `initializer_list< pair >`. I'm surprised at that. Surely the element type of the initializer list is the value of the vector?, i.e. `initalizer_list< pair>`? – Aaron McDaid Jul 27 '15 at 20:42
  • @AaronMcDaid: Because of forwarding references (U&&,V&&), the constructor takes whatever you pass it, not the T1,T2 types in `pair`. The intent is to make the construction more efficient, by letting the constructor arguments be passed as-is, and then have the first and second members be constructed in-place, but in this case, it backfires. – Vaughn Cato Jul 27 '15 at 22:21
  • @VaughnCato, I understand forwarding references, but I was still a bit confused about initializer_list, because I've never given them much thought. So, to confirm, the type of the initializer_list is `initializer_list< pair>`? And you're talking about the construction of the elements in that initializer_list? After each element, a `pair`, is constructed the temporaries used in the construction of that element are destroyed? – Aaron McDaid Jul 28 '15 at 08:45
  • @AaronMcDaid: That's right. The initializer_list that is passed to the vector has type `initializer_list< pair>`. As it is constructing each pair in the initializer_list, it calls the `pair(const char (&)[2],const char (&)[n])` constructor which is a particular instantiation of the `pair(U&&,V&&)` member template constructor. Inside each call to that constructor, a temporary is created for the constructor initializer list to initialize a `const A &` from a `const char (&)[n]`, when is then destroyed when the pair constructor returns. – Vaughn Cato Jul 28 '15 at 14:55
  • @AaronMcDaid: I've expanded my answer to try to make it more explicit. – Vaughn Cato Jul 28 '15 at 15:06
1

They don't reference the same A& object. They reference different objects that happen to have the same memory address because their lifetimes don't overlap.

There's no particular expectations you should have regarding whether objects with non-overlapping lifetimes do or don't occupy the same memory address.

David Schwartz
  • 179,497
  • 17
  • 214
  • 278
  • I've updated my code with an example that appears to show they reference the same object, which looks bogus btw. – Ale Morales Jul 23 '15 at 23:20
  • 2
    Why don't the lifetimes overlap? I'd expect that the temporaries created by initializing `v` from the `initializer_list` would last for the duration of the call to the function. I wouldn't know where to start looking in the standard for this though (and i'm not even convinced that a vector of pair containing const reference is legal) – M.M Jul 23 '15 at 23:47