How to prevent ODR violations in this case?

628 Views Asked by At

disclaimer: this question is about prevention of unintended naming collisions, and make sure the following code fail to compile/link.

[edit] actually I'd be happy with something preventing this to compile/link, or something that solves this, like anonymous namespaces. But anonymous namespaces aren't supposed to go inside headers.

// Class1.h
// --------
namespace SomeLargeLib {
struct S {
  int a;
  S() { a = 1; }
};
}

// Class1.cpp
// ----------
#include "Class1.h"

void foo() { SomeLargeLib::S s; }

// Class2.h
// --------
namespace SomeLargeLib {
struct S {
  int a;
  S() { a = 2; }
};
}

// Class2.cpp
// -----------
#include "Class2.h"

int main() {
  SomeLargeLib::S s;
  return s.a; // returns 1 !!
}

What happens here is that the ctor S::S has two inline definitions, so the linker is allowed to assume all definitions are the same and pick any one. Note that moving the S::S ctors outside of the classes results in a linker error as they are no longer inline.

Anyway, since we shouldn't have anonymous namespaces in headers (see C++ Core Guidelines), what should be done to prevent such ODR violations? A linker error would be perfect but the standard says no diagnostic is needed for ODR violations.

1

There are 1 best solutions below

1
Guillaume Racicot On

ODR violation are extremely hard to detect automatically, that's why it is ill formed no diagnostic required.

However, if your goal is to prevent and detect such cases, simply use modules with a strong ownership. The model completely prevent most (if not all) ODR violations that are hard to detect.

Example:

class1.ixx:

export module class1;
namespace SomeLargeLib {
export struct S {
  int a;
  S() { a = 1; }
};
}

class2.ixx:

export module class2;
namespace SomeLargeLib {
export struct S {
  int a;
  S() { a = 2; }
};
}

test1.cpp:

import class1;
import <iostream>;

void fun() {
    SomeLargeLib::S a;
    std::cout << a.a; // 1
}

test2.cpp:

import class2;
import <iostream>;

void fun() {
    SomeLargeLib::S a;
    std::cout << a.a; // 2
}

And the great thing is that you could link test1.cpp and test2.cpp together in the same binary. No collision ever happen.

However, a compiler error is emitted for test3: test3.cpp:

import class1;
import class2; // error: name SomeLargeLib::S already exists, can't import both

Those use cases are supported in MSVC already today.