16

In one of my classes, I am trying to use std::priority queue with a specified lambda for comparison:

#pragma once
#include <queue>
#include <vector>

auto compare = [] (const int &a, const int &b) { return a > b; };
class foo
{
public:
    foo() {  };
    ~foo() {  };
    int bar();
private:
    std::priority_queue< int, std::vector<int>, decltype(compare)> pq;
};

My program compiles perfectly until I add a .cpp file to accompany the header:

#include "foo.h"

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

This time, my compiler generates an error:

>main.obj : error LNK2005: "class <lambda> compare" (?compare@@3V<lambda>@@A) already defined in foo.obj

Why can't I create a accompanying .cpp file if my header file contains a lambda?

Compiler: Visual Studio 2012

My main.cpp:

#include "foo.h"

int main(){
    return 0;
}
yizzlez
  • 8,757
  • 4
  • 29
  • 44
  • 5
    Mark it `const`, that way it has internal linkage by default. Or better yet, make it a functor. – Rapptz Aug 07 '13 at 20:28
  • 3
    You're declaring two globals both named `compare` because `foo.h` is being included in two separate source files. I concur with Rapptz. – WhozCraig Aug 07 '13 at 20:29
  • 1
    Don't use lambdas this way. They are meant to create small local functions, not generally used functions. This is less readable than a normal function. – GManNickG Aug 07 '13 at 20:53
  • I'm only using the lambda to initialize the `priority_queue`, but should I switch it to a functor anyways? By the way, thanks Rapptz – yizzlez Aug 07 '13 at 20:59
  • I'm assuming that you're just giving an example, and your lambda is more complicated. Because right now it's the equivalent of `std::greater` – Dave S Aug 07 '13 at 21:27
  • Why you don't pass the comparer as a template parameter, and use the lambda as a default value directly? – Manu343726 Aug 07 '13 at 22:47

2 Answers2

15

As @Rapptz suggested,

const auto compare = [] (const int &a, const int &b) { return a > b; };

Solved the problem. Why?

Internal vs External linkage. By default, auto, like int has external linkage. So just how:

int j = 5;

In foo.h that would later be included by foo.cpp throws a

Error 2 error LNK2005: "int j" (?j@@3HA) already defined in Header.obj

(VS 2013)

However, const makes the linkage internal by default, which means it is only accessible in one translation unit, thereby avoiding the problem.

Community
  • 1
  • 1
yizzlez
  • 8,757
  • 4
  • 29
  • 44
1

I'm unable to replicate this problem for some reason. I'm trying this on VS2010 though - not sure if that made a difference. In fact, I tried including your header in two source files and it compiles, links and runs fine.

That said, do you want to consider using std::function. That way you can define the lambda in the cpp code and it won't get defined multiple times for whatever reason. (BTW, where's foo.obj coming from? Do you have another source file that is including this header ?).

foo.h:

#pragma once
#include <queue>
#include <vector>
#include <functional>

typedef std::function<bool (int, int) > comptype;
//auto compare = [] (const int &a, const int &b) { return a > b; };
class foo
{
public:
    foo() {  };
    ~foo() {  };
    int bar();

private:
    std::priority_queue< int, std::vector<int>, comptype> pq;
};

Then later in the cpp include and define the lambda and when you create the pq pass it to the constructor.

foo.cpp:

auto compare = [] (const int &a, const int &b) { return a > b; };

foo::foo():pq(compare){}

This way you're deftly not defining the function multiple times.

Arun R
  • 873
  • 6
  • 10