Why does this call by reference create a new instance?

Question

Im calling a method foo by const ref:

// method, which is being called
void foo(const Entity & ent);

// call
Entity* e = new Entity;
foo(e);    // wrong: missing * but compiles

This piece of code does not only compile, it creates a new instance of Entity with its default values in the scope of foo. I'd expect that this does not compile or at least crashes.

If I call foo correctly (foo(*e)), everything works as suspected and I see the right values of Entity within foo.

Im using mingw supplied with Qt 4.7.

Here's the interface of Entity:

class Entity : public QObject
{
    Q_OBJECT

public:
    Entity (QObject* parent = NULL);

    long getId() const { return this->id; }
    void setId(const long id) { this->id = id; }

    QString getName() const { return this->name; }
    void setName(const QString & name) {this->name = name; }

private:
    QString name;
    long id;
};

If it compiles, that would suggest that you have a foo(Entity * ) overload somewhere. — RobH, Sep 20 '11 at 13:11
I added the header. This may drift into some Qt specific direction, so I added the qt tag as well. — atamanroman, Sep 20 '11 at 13:19
You have a constructor for `Entity` that will take an `Entity*` since `Entity` is derived from `QObject` - see @Mark B's answer. — pstrjds, Sep 20 '11 at 13:22

Mark B · Accepted Answer · 2011-09-20T14:22:15.867

15

[Edited] You have an implicit converting constructor (that also happens to be a default constructor) from Entity* (via parent QObject*) to Entity and it's being used to create a temporary instance to pass in.

For this reason I always suggest by default making all single-parameter callable constructors (for example where all but one parameter is defaulted) explicit and avoiding implicit conversion operators except when they exactly perform the conversion semantics that would be expected in all circumstances. In other cases make the conversions available through explicit methods.

There are occasionally cases where implicit conversions are useful, and each one should be evaluated on a case-by-case basis.

edited Sep 20 '11 at 14:22

answered Sep 20 '11 at 13:12

Mark B

95,107
10
109
188

2

+1. Note that "single-parameter constructors" includes those multi-parameter constructors that can be called with one parameter due to defaults. – Fred Larson Sep 20 '11 at 13:14
+1 single argument constructors are actually evil unless explcitly told not – stijn Sep 20 '11 at 13:21
Not "almost certainly": very much does so, right there in the code. – Lightness Races in Orbit Sep 20 '11 at 13:24
Thank you very much Mark B and pstrjds (who made me realize whats happening), got this one now. I just missed to make the constructor explicit :/ – atamanroman Sep 20 '11 at 13:25

score 2 · Answer 2 · answered Sep 20 '11 at 13:23

Actually:

Entity* e = new Entity;
foo(e); //will call: 
-> foo ( Entity(e) ) ; //will call:
-> foo ( Entity((QObject*) e );

You are creating a new temporary entity from a pointer to an Entity (which is also a pointer to a QObject).

It compiles because it's valid.

As Mark pointed out, the implicit conversion from Entity* to Entity is done through the constructor which takes an argument of type "pointer to QObject". To test this out, change the inheritance to private, you should get a compilation error.

To avoid such mistakes in the future, declare conversion constructors as explicit.

score 1 · Answer 3 · answered Sep 20 '11 at 13:24

Your Entity constructor takes a QObject* argument and is not marked as explicit. That means that an Entity may be constructed implicitly from an Entity*, which is pretty bad news.

Further, because the function takes a ref-to-const, this implicitly-constructed temporary can be bound to that function argument.

Mark the constructor explicit.

Why does this call by reference create a new instance?

3 Answers3