Ctor conflicts

Perhaps the content of this post is trivial and widely known(?), but I just spent some time fixing a bug related to the following C++ behavior.

Let’s take a look at this code snippet:

// main.cpp ------------------------------

#include 

void bar();
void foo();

int main(int argc, const char *argv[])
{
    bar();
    foo();
    return 0;
}

// a.cpp ---------------------------------

#include 

struct A
{
    A() { printf("apple\n"); }
};

void bar()
{
    new A;
}

// b.cpp ---------------------------------

#include 

struct A
{
    A() { printf("orange\n"); }
};

void foo()
{
    new A;
}

The output of the code above is:

apple
apple

Whether we compile it with VC++ or g++, the result is the same.

The problem is that although the struct or class is declared locally the name of the constructor is considered a global symbol. So while the allocation size of the struct or class is correct, the constructor being invoked is always the first one encountered by the compiler, which in this case is the one which prints ‘apple’.

The problem here is that the compiler doesn’t warn the user in any way that the wrong constructor is being called and in a large project with hundreds of files it may very well be that two constructors collide.

Since namespaces are part of the name of the symbol, the code above can be fixed by adding a namespace:

namespace N
{
struct A
{
    A() { printf("orange\n"); }
};
}
using namespace N;

void foo()
{
    new A;
}

Now the correct constructor will be called.

I wrote a small (dumb) Python script to detect possible ctor conflicts. It just looks for struct or class declarations and reports duplicate symbol names. It’s far from perfect.

# ctor_conflicts.py
import os, sys, re

source_extensions = ["h", "hxx", "hpp", "cpp", "cxx"]

symbols = { }
psym = re.compile("(typedef\\s+)?(struct|class)\\s+([a-zA-Z_][a-zA-Z0-9_]*)(\\s+)?([{])?")

def processSourceFile(fname):
    with open(fname) as f:
        content = f.readlines()
    n = len(content)
    i = 0
    while i < n:
        m = psym.search(content[i])
        i += 1
        if m == None:
            continue
        symname = m.group(3)
        # exclude some recurring symbols in different projects
        if symname == "Dialog" or symname == "MainWindow":
            continue
        # make sure a bracket is present
        if m.group(5) == None and (i >= n or content[i].startswith("{") == False):
            continue
        loc = fname + ":" + str(i)
        if symname in symbols:
            # found a possible collision
            print("Possible collision of '" + symname + "' in:")
            print(symbols[symname])
            print(loc)
            print("")
        else:
            symbols[symname] = loc

def walkFiles(path):
    for root, dirs, files in os.walk(path):
        for f in files:
            # skip SWIG wrappers
            if f.find("PyWrapWin") != -1:
                continue
            # skip Qt ui files
            if f.startswith("ui_"):
                continue
            fname = root + os.sep + f
            ext = os.path.splitext(fname)[1]
            if ext != None and len(ext) > 1 and ext[1:] in source_extensions:
                processSourceFile(fname)


if __name__ == '__main__':
    nargs = len(sys.argv)
    if nargs < 2:
        path = os.getcwd()
    else:
        path = sys.argv[1]
    walkFiles(path)

In my opinion this could be handled better on the compiler side, at least by giving a warning.

ADDENDUM: Myria ‏(@Myriachan) explained the compiler internals on this one on twitter:

I'm just surprised that it doesn't cause a "duplicate symbol" linker error. Symbol flagged "weak" from being inline, maybe? [...] Member functions defined inside classes like that are automatically "inline" by C++ standard. [...] The "inline" keyword has two meanings: hint to compiler that inlining machine code may be wise, and making symbol weak. [...] Regardless of whether the compiler chooses to inline machine code within calling functions, the weak symbol part still applies. [...] It is as if all inline functions (including functions defined inside classes) have __declspec(selectany) on them, in MSVC terms. [...] Without this behavior, if you ever had a class in a header with functions defined, the compiler would either have to always inline the machine code, or you'd have to use #ifdef nonsense to avoid more than one .cpp defining the function.

The explanation is the correct one. And yes, if we define the ctor outside of the class the compiler does generate an error.

The logic mismatch here is that local structures in C do exist, local ctors in C++ don't. So, the correct struct is allocated but the wrong ctor is being called. Also, while the symbol is weak for the reasons explained by Myria, the compiler could still give an error if the ctor code doesn't match across files.

So the rule here could be: if you have local classes, avoid defining the ctor inside the class. If you already have a conflict as I did and don't want to change the code, you can fix it with a namespace as shown above.