5

I'm testing this code and wondering why this wasn't failed at compile time ?. I'm using c++11 and g++ 4.7.2.

I had similar structure on my production code, it was giving errors at run time, then I found that I'm constructing the class with wrong argument type.

#include <iostream>
#include <vector>


typedef std::vector<std::string> Word;

class Data {
    public:
        const Word &word;
        Data(Word w) : word(w) {}
};

class Base{
    const Data &data;
    public:
        Base(const Data &d): data(d) {}
        ~Base() {}
};

class Work : public Base{
    public:
        Work(const Data &d): Base(d){}
        ~Work() {}
};


int main(int argc, char **argv){
    Word words;
    words.push_back("work");

    /*
    * I'm confused with this constructor, why this passed the compilation
    * ??
    * Any special rule to reason this scenario ??
    *
    * But obviously it will fail at run time.
    */
    const Work *work  = new Work(words);

    return 0;
}
Haridas N
  • 529
  • 5
  • 20
  • 1
    Maybe an implicit conversion is going on somewhere? – Marc Claesen Sep 06 '13 at 07:51
  • The words are convert to Data by the Data constructor and then call the Work(Data) – ZijingWu Sep 06 '13 at 07:52
  • Why it should fail? On http://ideone.com/cxkf4X it returns Success. – xanatos Sep 06 '13 at 07:52
  • 1
    Beside the question, Isn't it an undefined behavior due to dangling reference in `Data` class? The `word` refers to a temporary object. – masoud Sep 06 '13 at 08:19
  • It is. I recommend Data { const Word& word; Data(Word&); }; and so on ... Maybe even Data { Word& word; Data(Word&); }; to get a decent compiler error if the argument in the constructor does not match. –  Sep 06 '13 at 08:32
  • @MM. Like this, it is not UB. As soon as it is used, the UB is invoked. – BЈовић Sep 06 '13 at 10:00

3 Answers3

10

Data is constructible from Word, so you can pass a Word to the Work constructor. Under the hood, an instance of Data will be created from the passed Word and, in turn, passed to the constructor.

You can avoid this by marking the constructor of Data that takes a Word as explicit, like this:

class Data {
    public:
        const Word &word;
        explicit Data(Word w) : word(w) {}
};

That way, the constructor cannot be implicitly applied anymore, and your call to the Work constructor will fail to compile unless you explicitly invoke the Data constructor:

const Work *work  = new Work(words);        // Implicit call, fails to compile.
const Work *work  = new Work(Data(words));  // Explicit call, compiles.
Community
  • 1
  • 1
Frédéric Hamidi
  • 258,201
  • 41
  • 486
  • 479
  • 1
    Of course, the constructor of `Data` won't work, because it binds the reference to the argument, which will be destructed when the constructor returns. – James Kanze Sep 06 '13 at 08:15
  • 1
    @MM., yes, it is. `words` being local to `main()` does not matter much because the instance of `Work` is also local to `main()`, but the `Work` constructor holds to a temporary `Data` instance, as James points out, and that's bad news indeed. – Frédéric Hamidi Sep 06 '13 at 08:23
  • 1
    Now I understood why my program gives segfault while run time ( crashes when accessing the dangling reference). – Haridas N Sep 06 '13 at 08:53
5

It works compiles* because Data has an implicit conversion constructor that takes a Word reference:

Data(Word w) : word(w) {}

This means you can do things such as

Word words;
Data d1 = words;
Data d2(words);

and you can construct a Work from a Data via constructor Work(const Data &d): Base(d){}:

Work w(d2);

which means the following is also valid, because it only involves one user-defined conversion:

Work w2(words); // constructs Data temporary from words, then constructs w2 with it

This behaviour can be suppressed by declaring the converting constructor as explicit:

explicit Data(Word w) : word(w) {}

* Your doesn't actually work because it involves a dangling reference to a temporary Data object

juanchopanza
  • 223,364
  • 34
  • 402
  • 480
2

After exit of the following constructor, the data member word of reference type will be referencing an object that no longer exists:

class Data {
    public:
        const Word &word;
        Data(Word w) : word(w) {}
};

A variable of automatic storage duration is created to hold w. You store a reference to that variable as a member word, then the referee is destroyed on exit of the constructor.

Regardless of the other problems, I don't think this is your intention.

Andrew Tomazos
  • 66,139
  • 40
  • 186
  • 319