C++ class name collision

For the following C++ code, I'm getting unexpected behavior. The behavior was verified with recent GCC, Clang and MSVC++. To trigger it, it is required to split the code among several files.

def.h

#pragma once

template<typename T>
struct Base
{
    void call() {hook(data);}
    virtual void hook(T& arg)=0;
    T data;
};

foo.h

#pragma once
void foo();

foo.cc

#include "foo.h"
#include <iostream>
#include "def.h"

struct X : Base<int>
{
    virtual void hook(int& arg) {std::cout << "foo " << arg << std::endl;}
};


void foo()
{
    X x;
    x.data=1;
    x.call();
}

bar.h

#pragma once
void bar();

bar.cc

#include "bar.h"

#include <iostream>
#include "def.h"

struct X : Base<double>
{
    virtual void hook(double& arg) {std::cout << "bar " << arg << std::endl;}
};


void bar()
{
    X x;
    x.data=1;
    x.call();
}

main.cc

#include "foo.h"
#include "bar.h"

int main()
{
    foo();
    bar();
    return 0;
}

Expected output:

foo 1
bar 1

Actual output:

bar 4.94066e-324
bar 1

What I expected to happen:

Inside of foo.cc, an instance of X defined within foo.cc is made and through calling call(), the implementation of hook() within foo.cc is called. Same for bar.

What actually happens:

An instance of X as defined in foo.cc is made in foo(). But when calling call, it dispatches not to hook() defined in foo.cc but to hook() defined in bar.cc. This leads to corruption, as the argument to hook is still an int, not a double.

The problem can be solved by putting definition of X within foo.cc in an other namespace than definition of X within bar.cc

So finally the question: There is no compiler warning about this. Neither gcc, nor clang or MSVC++ did even show a warning about this. Is that behavior valid as defined per C++ standard?

The situation seems to be a bit constructed, but it happened in a real world scenario. I was writing tests with rapidcheck, where possible actions on a unit to be tested are defined as classes. Most container classes have similar actions, so when writing tests for a queue and a vector, classes with names like "Clear", "Push" or "Pop" may come up several times. As these are only required locally, I've put them in directly in the sources where the tests are execute.


Solution 1:

The program is Ill-formed, because it violates the One-Definition Rule by having two different definitions for class X. So it is not a valid C++ program. Note that the standard specifically allows compilers not to diagnose this violation. So the compilers are conforming, but the program is not valid C++ and as such has Undefined Behaviour when executed (and thus anything can happen).

Solution 2:

You have two identically named, but different, classes X in different compilation units, rendering the program ill-formed, as there are now two symbols with the same name. Since the problem can only be detected during linking, compilers are not able (and not required) to report this.

The only way to avoid this sort of thing, is to put any code that is not meant to be exported (in particular all code that has not been declared in a header file) into an anonymous or unnamed namespace:

#include "foo.h"
#include <iostream>
#include "def.h"

namespace {
    struct X : Base<int>
    {
        virtual void hook(int& arg) {std::cout << "foo " << arg << std::endl;}
    };
}

void foo()
{
    X x;
    x.data=1;
    x.call();
}

and equivalently for bar.cc. In fact, this is the main (sole?) purpose of unnamed namespaces.

Simply re-naming your classes (e.g. fooX and barX) may work for you in practice, but is not a stable solution, because there is no guarantee that these symbol names are not used by some obscure third-party library loaded at link- or run-time (now or at some point in the future).