crash: wrong declaration order

[Alatar]

Hi!

I recently ran into a crash, where I had declared member objects of a class in the wrong order. Basically, member A was using member B, but member B was declared after member A. So, when member A was trying to use member B, that did not work, cause member B was not yet constructed.

My question is: how can I prevent this? My compiler did not flag this. Neither XCode nor Visual Studio warned me.
Is it possible to make the compiler aware of this? Or is it possible to use a certain code pattern, so that this problem cannot happen?

Its probably best, if I post example code, so you understand my case better:

class Foo
{
public:
    Foo()
    {
        data = new int();
    }
    
    ~Foo()
    {
        delete data;
    }
    
    void setData(int newValue)
    {
        *data = newValue;
    }
    
private:
    
    //a pointer to an int:
    int* data = nullptr;
};

class Bar
{
public:
    Bar(Foo& foo)
    {
        foo.setData(42);
    }
};

class CrashTest
{
public:
    CrashTest() : bar(foo)
    {
        
    }
    
private:
    Bar bar;
    Foo foo;
    
};

main()
{
    CrashTest crashTest;
}

So here, the class CrashTest is declaring two class members: bar and foo. bar is declared first. BUT: bar is using foo. Cause in the constructor of bar, it requires a Foo object. Now, when bar tries to access foo, the constructor of foo has not yet run. That means, it has not initialised its data. And thus the access from bar to foo is invalid. And it crashes.

One solution is, to reverse the order of declaration in CrashTest. If I declare foo first, and bar seconds, things are fine.

So this is obviously a bug of mine! But I would like a way to prevent this in the future.
Can I somehow make the compiler aware of this so that it generates a warning? Cause currently it does not warn about it.
Or can I use some kind of code pattern, which prevents me from making this mistake ever again? :-)

Curious to see what people here think :-)

[enriquemesa8080]

Look for forward declaration. That can help you. Less or more is the concept.

[dutch]

Swap the order of Foo foo; and Bar bar;
They are constructed in the order they are listed.

[Alatar]

Thanks for your answers.

@enriquemesa8080:
I don't think forward declaration is going to help here? Cause it compiles fine. It crashes during run-time.

@dutch:
Yes, I know that I can do that to solve my problem. But I want the compiler to warn me. Cause its so easy to miss in a larger code base. Or I would like to use a code pattern, which somehow prohibits me to make that mistake in the first place.

[ne555]

    CrashTest() :
        foo(), //warning: field 'foo' will be initialized after field 'bar' [-Wreorder]
        bar(foo)
    {

    }

sadly

private:
    Bar bar;
    Foo foo{}; //no warning 

> Or is it possible to use a certain code pattern, so that this problem cannot happen?
http://www.cs.technion.ac.il/users/yechiel/c++-faq/using-this-in-ctors.html

don't pass any data member from the this object (...) to any other data member's initializer!

[keskiverto]

its so easy to miss in a larger code base.

True, although the issue is within one class:

class CrashTest
{
public:
    CrashTest()
    : bar( foo ) // error: initializer uses uninitialized value
    // , foo() // foo initializes here
    {}
    
private:
    Bar bar; // order of members dictated here
    Foo foo;
};

Yes, a class can have many bases and members, and many constructors too.
If that is too much to be diligent about, ...

[Alatar]

Thanks again for your answers.

I am thinking about making my members a shared_ptr. That way the decleration order in the class would not matter.
In other words:

class CrashTest
{
public:
    CrashTest()
    {
         foo = std::make_shared<Foo>();
         bar = std::make_shared<Bar>(foo);   
    }
    
private:
    std::shared_ptr<Bar> bar;
    std::shared_ptr<Foo> foo;
    
};

What do you think?

[ne555]

- wasteful
- ¿why shared instead of unique?
- std::make_shared<Bar>(foo); ¿now Bar constructor wants a shared pointer? ¿why a reference is no longer good enough?
- I guess that you now have a guaranteed* runtime error if you screw up the constructor. Sadly, it's a runtime error.

*nope, undefined behaviour

[mbozzi]

Enable compiler warnings.

If you are using GCC or a GCC-compatible compiler driver, pass the flag -Wreorder to your compiler.
https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html
Preferably, enable all of the most useful diagnostics and ask for strict conformance with the flags -Wall -Wextra -pedantic-errors

If you are using MSVC or a MSVC-compatible compiler driver, pass the flag /W15038 to your compiler.
https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/c5038?view=vs-2019
Preferably, enable all of the most useful diagnostics with the flag /W4

[Enoizat]

Alatar wrote:

What do you think?

It you feel like abandoning member initializer lists in favour of assigning in the constructor body, then in your code the order doesn’t matter any more. I mean, once you get inside the constructor body, as far as I know all properties have already been fully initialized.
I.e. you don’t even need any kind of pointers.

Example (sorry for the cramped style):

class Foo {
public:
    Foo() : data { new int } {}
    ~Foo() { delete data; }
    void setData(int newValue) { *data = newValue; }

private:
    int* data { nullptr };
};


class Bar {
public:
    Bar() = default;
    explicit Bar(Foo& foo_arg) { foo_arg.setData(42); }
    Bar& operator=(Foo& foo_arg) { foo_arg.setData(42); return *this; }
};


class CrashTest {
public:
    CrashTest() { bar = foo; }

private:
    Bar bar;
    Foo foo;
};


main()
{
    CrashTest crash_test;
}