What is VS C++ compiler supposed to do with this code?

Question

Here is a reduced code snippet I needed to debug today.

I accidently typed something like this:

QImage myImage = LoadImage(path);
QImage scaledImage = myImage.scaled(100, 100);
if (condition)
{
   QImage scaledImage = scaledImage.mirrored(true, true); // *** Crash ***
}

Instead of the intendend code:

QImage myImage = LoadImage(path);
QImage scaledImage = myImage.scaled(100, 100);
if (condition)
{
   scaledImage = scaledImage.mirrored(true, true); // *** Works fine ***
}

I would have expected the flawed code to either work as expected or to generate a compiler error. But it simply crashed with a division by zero. Why? Can someone tell me the compiler's point of view about this mistake?

Answer 1

Think about what this line of code does:

QImage scaledImage = scaledImage.mirrored(true, true); // Crash

1. The symbol scaledImage is defined. This scaledImage symbol name overrides the one of the same name in the outer scope.
2. Calls the mirrored() method.
3. scaledImage is now created using the copy constructor with the output of mirrored().

As PeterT points out in the comments, this is undefined behavior: you're calling a method on an object before it's created. In this case crashing helped you avoid a mistake that's easy to make.

Here's an example that demonstrates exactly how and why this problem exists:

class Tester {
public:
    Tester() {
        qDebug() << "Default c'tor";
    }

    Tester( const Tester& other ) {
        qDebug() << "Copy c'tor";
    }

    Tester& Tester::operator=( const Tester& other ) {
        qDebug() << "Assignment";
        return *this;
    }

    Tester& test() {
        data = "test";
        return *this;
    }
};

int main(int argc, char *argv[])
{
    Q_UNUSED(argc);
    Q_UNUSED(argv);

    Tester test = test.test();
    return 0;
}

The program will output the following:

Test

Copy c'tor

In other words, the test() method was called before any constructor was called. That's bad!

But it gets much worse if we modify our example with a data member:

class Tester {
    QString data;
public:
    Tester() {
        qDebug() << "Default c'tor";
        data = "data";
    }

    Tester( const Tester& other ) {
        qDebug() << "Copy c'tor";
        data = other.data;
    }

    Tester& Tester::operator=( const Tester& other ) {
        qDebug() << "Assignment";
        data = other.data;
        return *this;
    }

    Tester& test() {
        data = "test";
        qDebug() << "Test";
        return *this;
    }

};

Now the program crashes before anything can be printed out. Specifically, the first line in test() is the culprit:

data = "test";

If you think about it, this is trying to assign something to a QString that hasn't yet been constructed. Any attempt to access or modify the member variables of an unconstructed object is bad news.

Answer 2

The reason is still there if you reduce this code.

This:

QImage myImage = LoadImage(path);
QImage scaledImage = myImage.scaled(100, 100);
if (condition)
{
   QImage scaledImage = scaledImage.mirrored(true, true); // *** Crash ***
}

has the same error as this:

QImage myImage = LoadImage(path);
QImage scaledImage = scaledImage.mirrored(true, true); // *** Crash ***

Because the scaledImage on the right side is calling a function on the uninitialized object from the left side. This is undefined behavior.