views:

549

answers:

3

Why does the following code not give me a duplicate symbol linker error for Impl?

I ran across this problem in some code I inherited and I'm recreating a shorter version here for simplicity.

I have two classes, Foo and Bar, that each define a different version of the same struct (Impl) in each of their .cpp files. So Foo.cpp and Bar.cpp each have an identically named Impl definition, but each one has a different inline constructor implementation.

Both Foo and Bar have a member variable of type Impl and each forward declares Impl in its .h file.

Foo.cpp news an instance of Bar inside its constructor. What's interesting is what gets created depends on the order the files are linked.

So this compilation command:

g++ -o a.out main.cpp Bar.cpp Foo.cpp

results in this output:

==> main()
Bar.cpp's Impl::Impl()
Bar.cpp's Impl::Impl()
<== main()

And this command:

g++ -o a.out main.cpp Foo.cpp Bar.cpp

results in this output:

==> main()
Foo.cpp's Impl::Impl()
Foo.cpp's Impl::Impl()
<== main()

I have tried this with gcc 4.1.2, Visual Studio 2008 and the Green Hills Multi 4.2.4 and they all produce the same result.


Foo.h

#ifndef FOO_H

struct Impl;
class Bar;

class Foo
{
public:
   Foo();
   ~Foo();

private:
   Impl* p;
   Bar* bar;
};

#endif

Foo.cpp

#include <iostream>
#include "Foo.h"
#include "Bar.h"

struct Impl
{
   Impl()
   {
      std::cout << "Foo.cpp's Impl::Impl()" << std::endl;
   }
};

Foo::Foo()
 : p(new Impl),
   bar(new Bar)
{
}

Foo::~Foo()
{
   delete p;
   delete bar;
}

Bar.h

#ifndef BAR_H
#define BAR_H

struct Impl;

class Bar
{
public:
   Bar();
   ~Bar();

private:
   Impl* p;
};

#endif

Bar.cpp

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

struct Impl
{
   Impl()
   {
      std::cout << "Bar.cpp's Impl::Impl()" << std::endl;
   }
};

Bar::Bar()
 : p(new Impl)
{
}

Bar::~Bar()
{
   delete p;
}

main.cpp

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

int main (int argc, char const *argv[])
{
   std::cout << "==> main()" << std::endl;
   Foo* f = new Foo();
   std::cout << "<== main()" << std::endl;
   return 0;
}
+3  A: 

G'day,

Isn't default link editor behaviour to take the first symbol that satisfies the requierments and stop searching.

You should be able to enable a complete search to disallow duplicate symbols within the closure of the executable.

Edit: I've just seen that the link editor on Solaris disallows multiple definitions be default. You actually have to use the link editor switch "-z muldefs" to allow linking to proceed with multiple definitions within the objects being used to establish closure for the executable.

Edit2: I'm intrigued here as this should be flagged as a warning. What happens if you add

-std=c++98 -pedantic-errors

to the command line when you build your executable?

Rob Wells
Adding those flags does not add any warnings and it behaves the same. So apparently gcc just ignores the fact that there are two identical symbols.I wonder if there's some way to force it to show any duplicates. Sort of the opposite of -z muldefs. I didn't see anything in the manual.
Bob Mourlam
@Bob, no. neithet did I. )-:
Rob Wells
Duplicate symbols are not always an error and are sometimes required.
Roger Pate
@R. Pate, agreed. But it'd be nice to switch behaviour wouldn't it.
Rob Wells
What would the switch do?
Roger Pate
Flag dupes as errors. Or flag them as warnings.
Rob Wells
+5  A: 

You're violating the one definition rule, and the compiler/linker isn't required to tell you about it.

Bill
+2  A: 

Other have already speak about the One Definition Rule, I thought I would chime in some explanation and a real workaround.

Explanation:

I won't explain the One Definition Rule, but I will explain why the linker does not complain. When you use templates, each object get it's own std::vector<int> instantiation. The linker just pick up the first available.

If it was not the case, you would have to explicitly instantiate the template in one source file, then use the extern keyword in the others... but only Comeau supports it.

Work around:

Since I basically suppose you are trying to implement a Pointer to Implementation, you don't have much choice apart from forwarding.

Having dealt with similar problems before (how I hate to have to rely on Pimpl to simplify the dependencies...), I simply rely on a naming convention, coupled to a namespace I reuse for implementation details:

namespace detail { class FooImpl; }

class Foo
{
  typedef detail::FooImpl Impl; // note that the typedef is private
  Impl* m_impl;
};

Simple and efficient. I always use detail for implementation details and I simply append Impl to the end of the class name of which it is supposed to be an Impl.

Notes:

  • A pity you cannot just forward declare it in the class, but there is nothing we can do about it.
  • The namespace details prevents polluting the main namespace where you class lives with those symbols, especially handy for autocompletion by IDE since otherwise you'd get both Foo and FooImpl as propositions.
  • ImplFoo is not great either for autocompletion since all pimpl would begin by Impl!

Hopes it helps.

Matthieu M.