Refactor constructor to use initialization list

Question

I am working on a very large code base (over 3M loc) we obviously have a lot of classes but most of them do not use initialization lists in their constructors , instead they assign the values in the constructor body (some of the code was written a LONG time ago so this has become a defacto standard). Perhaps these are optimized away by the compiler, but I'm not sure that really is the case.

I am trying to promote the use of initialization lists, but there is a large code base that needs bringing up to date, so are there any tools that will do this for me automatically? Point it at a class, find all the m_var = 0; lines and move them to the initialization list (creating it if needed).

As well as converting in-body initialization to initialization lists, is there a way to find out that the member variables are initialized in the correct order (i.e. the same order as they are defined in the class' header file? I was hoping that CppCheck would pick this up, but it doesn't seem to.

Answer 1

Hello I am a cppcheck developer.

Cppcheck also has a check for mismatching order. But it is an inconclusive check.

For example:

class Fred {
public:
    Fred() : y(0), x(0) {}
    int x;
    int y;
};

Cppcheck output:

daniel@debian:~/cppcheck$ ./cppcheck --enable=style --inconclusive 1.cpp
Checking 1.cpp ...
[1.cpp:3] -> [1.cpp:4]: (style, inconclusive) Member variable 'Fred::x' is in the wrong place in the initializer list.

Our simple check will just warn whenever the order mismatch. That is why it is inconclusive. In the code above the initialization order does not actually matter - since all members in the code above are ints and all initializers are constant literals.

Answer 2

WIth scale like the size of OP's code base, what is needed is a program transformation system (PTS). This is a tool that parses a target language source file into compiler data structures (usually ASTs), allows you to apply transformations to the ASTs, and then can regenerate valid source code including the original comments for the modified program. Think of PSTs as tools for refactoring in the large.

A good PTS will let you write source-to-source transforms of the form:

when you see *this*, replace it by *that* if *condition*

where this and that are expressed in the target language syntax, where this matches only if the source code matches the explicit syntax. [These are not string matches; they work on ASTs so layout does not affect their ability to match].

You need a key rule that looks something like this:

rule move_to_initializer(constructor_name:IDENTIFIER,
                         arguments: argument_list,
                         initializer_list: initializer,
                         member_name:IDENTIFIER,
                         initializer_expression: expression,
                         statements: statement_list
                         ): constructor -> constructor =
   " \constructor_name(\arguments): \initializer_list 
           { \member_name = \initializer_expression ;
             \statements } "
    ->  " \constructor_name(\arguments): \initializer_list, \member_name(\initializer_expression)
           { \statements } ";

The syntax of these rules/patterns for our DMS Software Reengineering Toolkit is explained here. DMS is the only source-to-source PTS I know that can handle C++; it even handles the MSVS dialect].

I have left out a possibly necessary "if condition" check that the member name is indeed a member of the class, under the assumption that your constructors are not abusive.

Because your constructors may not have any initializer list, you need a helper rule to introduce one when necessary:

rule move_to_initializer(constructor_name:IDENTIFIER,
                         arguments: argument_list,
                         member_name:IDENTIFIER,
                         initializer_expression: expression,
                         statements: statement_list
                         ): constructor -> constructor =
   " \constructor_name(\arguments)
           { \member_name = \initializer_expression ;
             \statements } "
    ->  " \constructor_name(\arguments): \member_name(\initializer_expression)
           { \statements } ";

           { \member_name = \e ; } "

Invariably you need additional rules to cover other special cases but it shouldn't be more than a few.

Regarding checking on order-of-initialization, you can trigger such a check using a (DMS) pattern:

pattern check_initializer_order(constructor_name:IDENTIFIER,
                         initializer_list: initializer,
                         statements: statement_list
                         ): constructor =
    " \constructor_name(): \initializer_list, 
           { \statements } "
           if complain_if_not_ordered(constructor_name,initializer_list);

which requires an auxiliary meta-predicate that checks the order, that complains if they are misordered. You need the constructor_name to enable the predicate to look up the corresponding class and inspect the order of the members. [DMS provides the means to access a symbol table with this information].

Alternatively, you might just simply re-order them using a different rewrite rule:

rule order_initializers(constructor_name:IDENTIFIER,
                         arguments: argument_list,
                         initializer_list_prefix: initializer,
                         initializer_list_suffix: initializer,
                         member1_name:IDENTIFIER,
                         initializer1_expression: expression,
                         member2_name:IDENTIFIER,
                         initializer2_expression:expression,                             
                         statements: statement_list
                         ): constructor -> constructor =
   " \constructor_name(\arguments): 
          \initializer_list_prefix,
          \member1_name(\initializer1),
          \member2_name(\initializer2),
          \initialize_list_suffix
           { \statements } "
    -> 
    " \constructor_name(\arguments): 
          \initializer_list_prefix,
          \member2_name(\initializer2),
          \member1_name(\initializer1),
          \initialize_list_suffix
           { \statements } "
      if is_wrong_order(constructor_name,member1_name,member2_name);

This rule essentially sorts the initializers. [Note this is a bubble-sort: but initializer lists tend not to be long, and you're only going to run this once per constructor anyway.] You would run this rule after you lifted all the initializers out of the constructor body using the rules shown earlier.