I suggest you ...

C++ compiler should warn about wrong member initialization order

Every few years this suggestion emerges and I think now is the time for next iteration.
In C++ the order of class/struct member initialization is determined by the order of member declaration and not by the order of their appearance in member initialization list. It is therefore extremely easy to to write a code that is very hard to find, e.g.:

struct S {
int a;
int b;

S( int i ) : b(i), a(b+1) {}
};

int main()
{
S s(42);
}

and we have garbage in s.a what might be not obvious for a novice programmer.
GCC issues a warning here:

f:\>g++ -Wall test.cpp
test.cpp: In constructor 'S::S(int)':
test.cpp:3:8: warning: 'S::b' will be initialized after
test.cpp:2:8: warning: 'int S::a'
test.cpp:5:4: warning: when initialized here

But Microsoft compiler doesn't spot any problems:

f:\>cl /Wall test.cpp
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 16.00.40219.01 for 80x86
Copyright (C) Microsoft Corporation. All rights reserved.

test.cpp
Microsoft (R) Incremental Linker Version 10.00.40219.01
Copyright (C) Microsoft Corporation. All rights reserved.

/out:test.exe
test.obj

Please. Add at least level 4 warning for this.

157 votes
Vote
Sign in
Check!
(thinking…)
Reset
or sign in with
  • facebook
  • google
    Password icon
    I agree to the terms of service
    Signed in as (Sign out)
    You have left! (?) (thinking…)
    Waldemar PawlaszekWaldemar Pawlaszek shared this idea  ·   ·  Flag idea as inappropriate…  ·  Admin →

    5 comments

    Sign in
    Check!
    (thinking…)
    Reset
    or sign in with
    • facebook
    • google
      Password icon
      I agree to the terms of service
      Signed in as (Sign out)
      Submitting...
      • Dwayne RobinsonDwayne Robinson commented  ·   ·  Flag as inappropriate

        This burns experienced coders too, not just newbies :/. I understand that it's tough to follow all the dependencies of initialization if one calls another function, but if a direct dependency tries to use a yet uninitialized variable, including itself, a warning would be very nice.

      • PhilippePhilippe commented  ·   ·  Flag as inappropriate

        Well as explained in another comment, the actual order is not always important.

        But the order can be important no only for direct dependence on another member, it can also be important for exception handling code when a member have non trivial constructor or destructor. Thus the warning should also be displayed in such case.

        I think that for consistency and simplicity it might be preferable to always require the actual order.

      • AndrewDoverAndrewDover commented  ·   ·  Flag as inappropriate

        You might as well put a check in for
        S( int i ) : a( a ) {}

        It does not make much sense to initialize using an uninitialized variable.

      • CruzMissileCruzMissile commented  ·   ·  Flag as inappropriate

        Using the ":datamember1(0), datamember2(0)" initialization list syntax keeps creation bound to intialization. The webpage talks about the order that data members appear in class declarations having a direct effect on order of processing in the initialization list.

        So if the initialization list isn't using any dependent expressions, and is just initializing data members to a default literal value, like 0 or something, the order is not important. But if the initialization list has order dependent initialization, such as datamember1(datamember2 + 5), datamember2(5), then order of data member declaration is very important.

        If datamember2 is not initialized before datamember1, datamember1 will not be initialized because it depends on datamember2 first being initialized.

        Here's the test code to demonstrate. Try it out and you'll see order of declaration determines order of initialization.

        Run it twice. In the first run leave data member order of declaration as is and you don't get expected output.
        In the second run, declare datamember2 before datamember1 and you get the expected output.

        //InitVSOrder.h
        #pragma once
        #include <iostream>
        using namespace std;
        class InitVSOrder
        {
        public:
        InitVSOrder(); // Default constructor
        int datamember1;
        int datamember2;
        void showDataMembers();
        };
        //InitVSOrder.cpp
        #pragma once
        #include <iostream>
        #include "InitVSOrder.h"
        using namespace std;
        InitVSOrder::InitVSOrder()
        :datamember1(datamember2 + 5),datamember2(5)
        {
        /*
        Is this object ready to go? No, datamember1 init depends on datamember2 init occurring first
        The order of data member declaration is
        int datamember1;
        int datamember2;
        */
        }
        void InitVSOrder::showDataMembers()
        {
        cout << "datamember1=" << datamember1 << endl;
        cout << "datamember2=" << datamember2 << endl;
        }
        //driver.cpp
        #pragma once
        #include <iostream>
        #include "InitVSOrder.h"
        /*
        Test driver for order of declaration vs. order of initialization
        */
        using namespace std;
        int main(int argc, char * argv[])
        {
        try
        {
        InitVSOrder order1;
        order1.showDataMembers();
        /*
        When class declaration, order of declaration for data members is:
        int datamember1;
        int datamember2;
        The output is:
        datamember1=-858993455
        datamember2=5
        since datamember2 is declared after datamember1, it is intialized after datamember1; datamember1 is invalid
        But, when class declaration, order of declaration for data members is:
        int datamember2;
        int datamember1;
        The output is:
        datamember1=10
        datamember2=5
        since datamember2 is now declared before datamember1, it is initialized before datamember1, hence datamember1 is valid
        */
        }
        catch (exception &e)
        {
        cout << e.what() << endl;
        system("pause"); //pause to view error printed
        exit(EXIT_FAILURE); //exit runs destructors for static and global objects
        }
        cout << endl << "Execution complete: " << endl << exename << endl;
        system("pause"); //execute OS system pause command to keep terminal window open
        exit(EXIT_SUCCESS); //exit runs destructors for static and global objects
        }

      Feedback and Knowledge Base