5
#include <iostream>
using namespace std;

#include "other_library.h"

struct Foo
{
    Foo() { cout << "class\n"; }
};

int main()
{
    Foo();  // or Foo() in any expression
}

This outputs class, or so we would think. The problem is that if other_library.h happens to have a function called Foo whose return type is suitable to appear in whatever expression we used Foo in then it silently changes the behaviour, e.g.:

int Foo() { cout << "func\n"; return 1; }

causes func to be output without any code changing in main. This is bad because of the possibility for insidious and hard-to-detect bugs ; even if it is not malicious intent on the part of other_library, a name clash could go undetected.

What is a good way to deal with this problem? It was originally raised by Dan Saks in 1997, and one suggested resolution is that all classes should be typedef'd:

typedef struct Foo Foo;

as the compiler does have to report a clash between a typedef-name and a function name. However this does not appear to be common practice - why not?

Clarification: this question is about good practices for our code to stop this undetected behaviour change happening without us noticing. (As opposed to how to solve it once we have realized that it is happening, which is easier -- e.g. rename our class).

Community
  • 1
  • 1
M.M
  • 138,810
  • 21
  • 208
  • 365
  • Namespaces and good code Hygiene is my preferred approach. – BlamKiwi Feb 15 '15 at 23:40
  • @MorphingDragon I guess the problem is what happens when you use old code that's not inside a namespace (like `other_library.h` here). Even if you put your own code inside a namespace, you are still in trouble if working inside the namespace (or `using` it). – vsoftco Feb 15 '15 at 23:42
  • What I've seen done is naming functions with camelCase, and types with CapitalCamelCase. No clash possible. – Quentin Feb 15 '15 at 23:42
  • @vsoftco you can explicitly scope your code in files that need to know about some symbols from other_library.h. – BlamKiwi Feb 15 '15 at 23:45
  • Wow, undefined behavior *is* undefined behavior. This caused an infinite loop while testing on Clang. –  Feb 15 '15 at 23:46
  • @remyabel where's the UB? (I thought it was well-defined that the function is called, although if it is UB then the problem is even worse than I thought :) – M.M Feb 15 '15 at 23:51
  • @MattMcNabb [The undefined behavior monster at work.](http://coliru.stacked-crooked.com/a/7852b88754fedcfc) –  Feb 15 '15 at 23:52
  • @remyabel the UB in your code is explained by the warning message :) I'll fix my example – M.M Feb 16 '15 at 00:12

3 Answers3

4

typedef struct Foo Foo;

as the compiler does have to report a clash between a typedef-name and a function name. However this does not appear to be common practice - why not?

I hold this truth to be self-evident: cumbersome code is cumbersome. You can actually rather easily fix this by working in a namespace of your own (which, assuming the header file is C++, too, the other_library should also do.

Marcus Müller
  • 34,677
  • 4
  • 53
  • 94
3

Unfortunately, you cannot avoid this. From C++11 standard (3.3.10):

A class name (9.1) or enumeration name (7.2) can be hidden by the name of a variable, data member, function, or enumerator declared in the same scope. If a class or enumeration name and a variable, data member, function, or enumerator are declared in the same scope (in any order) with the same name, the class or enumeration name is hidden wherever the variable, data member, function, or enumerator name is visible.

The only way to guard against it is the typedef trick you pointed out (thanks for that!), using namespaces, or adhering to naming conventions (the latter not helping when dealing with 3rd party code). You could also try to wrap the include in a namespace (shown bellow), but as the comments pointed out, this could cause link errors in certain situations.

namespace otherlib {
#include "other_library.h"
}

Then:

otherlib::Foo();
Rado
  • 8,634
  • 7
  • 31
  • 44
  • Can't this potentially cause link issues? You're essentially changing all of the names of every symbol in `other_library.h`. – BlamKiwi Feb 15 '15 at 23:49
  • 3
    I think this will cause link errors (unless you want to recompile all of `otherlib` to be in the namespace also, which may not be trivial), and also cause further errors if `other_library.h` does any `#include`s – M.M Feb 15 '15 at 23:49
  • Yes, it could potentially cause link errors. It all depends on the included file. Other than the typedef trick, I don't think there is a way to treat this as an error. Even tried -Weverything in clang and it still compiled. Thank ``C`` for that – Rado Feb 16 '15 at 02:35
1

current best practises protect against this already

  • Naming Conventions, such as
    void camelCase() vs class PascalCase

  • namespaces

  • wrapper libs for everything not modern C++

sp2danny
  • 7,488
  • 3
  • 31
  • 53
  • What do you do when you have to use a library that does not conform to these conventions? (e.g. most of them, or when you are linking to a library implemented in C) – M.M Feb 16 '15 at 00:42
  • thin wrapper to make it conform, with the header usually not exposing anything of the innards – sp2danny Feb 16 '15 at 00:44
  • The downside being, adding a new library to the approved list, has some work to it (and some bureaucracy). Maybe 'current best practises' should have been 'what we do' – sp2danny Feb 16 '15 at 00:49