2

I stumbled on a problem I was able to work around, but I'm not sure why it wasn't working.

This is the code I tried to use. Fields have been stripped for brevity. Let me know if they're needed and I'll put them back:

#pragma once
#ifndef __PageStyle__
#define __PageStyle__
class PageStyle
{
public:
    friend bool operator<(const PageStyle& lhs, const PageStyle& rhs);
};

bool operator<(const PageStyle& lhs, const PageStyle& rhs)
{
    return (lhs.name < rhs.name);
}
#endif

And in my source files I do something like this:

#include "PageStyle.h"

...
void PageStyleManager::loadPageStyles() {
    std::set<PageStyle> pageStyles;
    ...
}

The code compiled fine, but the linker spat this out:

1>PageStyleManager.obj : error LNK2005: "bool __cdecl operator<(class PageStyle const &,class PageStyle const &)" (??M@YA_NABVPageStyle@@0@Z) already defined in BaseContentFiller.obj

BaseContentFiller is the base class for PageStyleManager, as well as for other classes that also use PageStyle in a similar way.

After delving a bit further I discovered that for my purposes (using the class in an STL set) I didn't really need a non-member friend version after all. I made the operator an in-line public member and the code linked without a problem.

Why did this problem occur though? I ensured I had used header guards, this is my first real experience with operator overloading and I'd like to know what I did wrong.

tshepang
  • 12,111
  • 21
  • 91
  • 136
seanhodges
  • 17,426
  • 15
  • 71
  • 93
  • 1
    You shouldn't put the *implementation* of non-member functions in a header file unless they're static. – David Schwartz Feb 14 '12 at 09:55
  • 2
    Two unrelated issues: `#pragma once` isn't universally understood, and having it before the include guards could prevent some of the alternative techniques from working. If you do use `#pragma once`, put it after the include guards. And double underscores in a symbol are undefined behavior, even if the symbol is an include guard. – James Kanze Feb 14 '12 at 10:42

2 Answers2

13

If you include the definition of the function in the header file, it gets defined in every Translation unit where the header file is included.
This violates the One Definition Rule and hence the linker error.

Note that the header guards or the #pragma once just prevent the same header file from being included in the same source file multiple times, but not defining it across different TU.

Two solutions are possible to solve the problem:

  1. Just add the declaration in the header file and the definition in only one cpp file or
  2. make the function defined in the header as inline.
Community
  • 1
  • 1
Alok Save
  • 202,538
  • 53
  • 430
  • 533
  • Moving the declaration to the cpp file worked, thanks! I took the various online tutorials a little too literally (they often displayed the header and source in-line). In retrospect, declaring the operator in the source file makes perfect sense. – seanhodges Feb 14 '12 at 12:16
6

Don't define operator< in the header file; just declare it there, and define it in a .cpp file. #pragma once just keeps the header file from being included multiple times in the same .cpp file, but it may well be included in different .cpp files, in which case the linker will see multiple definitions of operator<.

Aasmund Eldhuset
  • 37,289
  • 4
  • 68
  • 81