Sign in to follow this  
simotix

std::make_pair, Visual Studio 2010 and std::tr1 errors

Recommended Posts

I was attempting to create an automated registry design (from here) and I ended up running into the following error. From what I can tell from research (google), this is new to Visual Studio 2010.

While I will post all of the code, I narrowed it down this line. This is one of those tricky errors that end up erroring out in some other file, and not telling you the origin (ie, the file category from the Error List window). Although, it does fail in the "type_traits" file.

std::pair<char*, T> thePair = std::make_pair(s, &createT<T>);


It will produce the following errors

Error 2 error C2752: 'std::tr1::_Remove_reference<_Ty>' : more than one partial specialization matches the template argument list c:\program files (x86)\microsoft visual studio 10.0\vc\include\type_traits 197
Error 3 error C2528: 'abstract declarator' : pointer to reference is illegal c:\program files (x86)\microsoft visual studio 10.0\vc\include\type_traits 965
Error 4 error C2528: 'type' : pointer to reference is illegal c:\program files (x86)\microsoft visual studio 10.0\vc\include\type_traits 349

Does anyone know why I am getting this error and how to prevent it?


#include <map>
using std::map;

class Component
{
protected:
const char *m_ComponentName;
Component(const char *componentName) : m_ComponentName(componentName) { }

public:
//Component() { }
virtual ~Component() { }
};

template<typename T> Component * createT() { return new T; }

struct BaseFactory {
typedef std::map<char*, Component*(*)()> map_type;

static Component * createInstance(char* s) {
map_type::iterator it = getMap()->find(s);
if(it == getMap()->end())
return 0;
return it->second();
}

protected:
static map_type * getMap() {
// never delete'ed. (exist until program termination)
// because we can't guarantee correct destruction order
if(!map) { map = new map_type; }
return map;
}

private:
static map_type * map;
};

template<typename T>
struct DerivedRegister : BaseFactory {
DerivedRegister(char* const& s) {
std::pair<char*, T> thePair = std::make_pair(s, &createT<T>);
getMap()->insert(thePair);
}
};

class ComponentA : public Component
{
private:
float A;
static DerivedRegister<ComponentA> reg;

public:
ComponentA() : Component("ComponentA") { A = 0.0f; }
~ComponentA() { }
};
DerivedRegister<ComponentA> ComponentA::reg("ComponentA");

class ComponentB : public Component
{
private:
float B;
static DerivedRegister<ComponentB> reg;

public:
ComponentB() : Component("ComponentB") { B = 0.0f; }
~ComponentB() { }
};
DerivedRegister<ComponentB> ComponentB::reg("ComponentB");

int main(void)
{
BaseFactory theFacory;
Component* testA = theFacory.createInstance("ComponentA");

return(0);
}

Share this post


Link to post
Share on other sites
Well, the types are incorrect. &createT<T> is not of type T, so that assignment cannot work. Also you are passing "char* const& s" as the constructor argument, which is just bizarre. While I think you should consider using std::string for this, if you are going to use C strings I think sticking to "const char *" is the easiest way to do it.

The whole point of std::make_pair is to remove the need to specify the type, so I'd just pass the result directly to insert().

include <map>
using std::map;

class Component
{
protected:
const char *m_ComponentName;
Component(const char *componentName) : m_ComponentName(componentName) { }

public:
//Component() { }
virtual ~Component() { }
};

template<typename T> Component * createT() { return new T; }

struct BaseFactory {
typedef Component *(*ComponentFactoryFuncPtr)();
typedef std::map<const char *, ComponentFactoryFuncPtr> map_type;

static Component * createInstance(const char* s) {
map_type::iterator it = getMap().find(s);
if(it == getMap().end())
return 0;
return it->second();
}

protected:
static map_type &getMap() {
static map_type map;
return map;
}

private:

};

template<typename T>
struct DerivedRegister : BaseFactory {
DerivedRegister(const char *s) {
getMap().insert(std::make_pair(s, &createT<T>));
}
};

class ComponentA : public Component
{
private:
float A;
static DerivedRegister<ComponentA> reg;

public:
ComponentA() : Component("ComponentA") { A = 0.0f; }
~ComponentA() { }
};
DerivedRegister<ComponentA> ComponentA::reg("ComponentA");

class ComponentB : public Component
{
private:
float B;
static DerivedRegister<ComponentB> reg;

public:
ComponentB() : Component("ComponentB") { B = 0.0f; }
~ComponentB() { }
};
DerivedRegister<ComponentB> ComponentB::reg("ComponentB");

int main(void)
{
BaseFactory theFacory;
Component* testA = theFacory.createInstance("ComponentA");

return(0);
}


This is off topic but note how getMap() now returns a reference to a static map. This is a nicer solution than using a pointer. Of course you could still dynamically allocate the map if you want, but this way you get cleanup for free. But the main idea is that the interface is nicer when you are not using a pointer.

Share this post


Link to post
Share on other sites
I appreciate the help, however, this solution unfortunately does not compile for me using VS2010. By any chance do you have a copy of VS2010 and have gotten it to compile?

Quote:
Original post by rip-off
The whole point of std::make_pair is to remove the need to specify the type, so I'd just pass the result directly to insert().


Ah yes, I was doing that as a debugging step.

Quote:
Original post by rip-off
This is off topic but note how getMap() now returns a reference to a static map. This is a nicer solution than using a pointer. Of course you could still dynamically allocate the map if you want, but this way you get cleanup for free. But the main idea is that the interface is nicer when you are not using a pointer.


Thank you the tip, having the cleanup done for free a definitely nicer.

IN addition to the errors I was receiving before, I now get these four extra errors also ...

Error 5 error C2535: 'std::_Pair_base<_Ty1,_Ty2>::_Pair_base(const _Ty1 &,const _Ty2)' : member function already defined or declared c:\program files (x86)\microsoft visual studio 10.0\vc\include\utility 148
Error 6 error C2535: 'std::_Pair_base<_Ty1,_Ty2>::_Pair_base(const char *&&,Component *(__cdecl &)(void))' : member function already defined or declared c:\program files (x86)\microsoft visual studio 10.0\vc\include\utility 153
Error 7 error C2535: 'std::pair<_Ty1,_Ty2>::pair(const _Ty1 &,const _Ty2)' : member function already defined or declared c:\program files (x86)\microsoft visual studio 10.0\vc\include\utility 230
Error 8 error C2535: 'std::pair<_Ty1,_Ty2>::pair(const char *&&,Component *(__cdecl &)(void))' : member function already defined or declared c:\program files (x86)\microsoft visual studio 10.0\vc\include\utility 236

Share this post


Link to post
Share on other sites
Interesting problem. I admit I must have skimmed your question because I didn't catch the fact that it was caused by VC2010, I was testing using GCC.

I've gotten it to compile by explicitly using a variable to store the function pointer first, so the type is inferred correctly:

#include <map>
using std::map;

class Component
{
protected:
const char *m_ComponentName;
Component(const char *componentName) : m_ComponentName(componentName) { }

public:
//Component() { }
virtual ~Component() { }
};

template<typename T> Component * createT() { return new T; }

struct BaseFactory {
typedef Component *(*ComponentFactoryFuncPtr)();
typedef std::map<const char *, ComponentFactoryFuncPtr> map_type;

static Component * createInstance(const char* s) {
map_type::iterator it = getMap().find(s);
if(it == getMap().end())
return 0;
return it->second();
}

protected:
static map_type &getMap() {
static map_type map;
return map;
}

private:

};

template<typename T>
struct DerivedRegister : BaseFactory {
DerivedRegister(const char *s) {
ComponentFactoryFuncPtr function = &createT<T>;
getMap().insert(std::make_pair(s, function));
}
};

class ComponentA : public Component
{
private:
float A;
static DerivedRegister<ComponentA> reg;

public:
ComponentA() : Component("ComponentA") { A = 0.0f; }
~ComponentA() { }
};
DerivedRegister<ComponentA> ComponentA::reg("ComponentA");

class ComponentB : public Component
{
private:
float B;
static DerivedRegister<ComponentB> reg;

public:
ComponentB() : Component("ComponentB") { B = 0.0f; }
~ComponentB() { }
};
DerivedRegister<ComponentB> ComponentB::reg("ComponentB");

int main(void)
{
BaseFactory theFacory;
Component* testA = theFacory.createInstance("ComponentA");

return(0);
}


It looks like a bug in the library, or at least an unintended consequence of the current implementation (is there a difference? [smile]).

Share this post


Link to post
Share on other sites
Quote:
Original post by rip-off
I've gotten it to compile by explicitly using a variable to store the function pointer first, so the type is inferred correctly:


That does indeed work for me, thank you for the help.

It is interesting to know that this will cause a compiling error in VS2010 but not GCC. I will try to search the Microsoft forums for a bug report on this, if not, I think it may be worth submitting. If it is not really a bug though, then maybe it is just a "unintended consequence of the current implementation" haha

Share this post


Link to post
Share on other sites
I have continued to play around with more, as I have been trying to learn more about the use of static with templates. I modified the way this is registered a bit, just to see if I could make it not have any extra definitions in the class. I came up with a modification, although the behavior is a bit strange at times.

I made a static "AddClass" function to register components outside of the class definition. The resulting functionality would be this

BaseFactory::AddClass<ComponentA>("ComponentA");


The problem is that, is that when I do this I will get the error

error C2761: 'bool BaseFactory::AddClass(const char *)' : member function redeclaration not allowed

However, to prevent the error I could do

bool result = BaseFactory::AddClass<ComponentA>("ComponentA");


Why does it think I am trying to redeclare the function when I am trying to call it?


#include <map>
using std::map;

class Component
{
protected:
const char *m_ComponentName;
Component(const char *componentName) : m_ComponentName(componentName) { }

public:
//Component() { }
virtual ~Component() { }
};

template<typename T> Component * createT() { return new T; }

struct BaseFactory {
typedef Component *(*ComponentFactoryFuncPtr)();
typedef std::map<const char *, ComponentFactoryFuncPtr> map_type;

static Component * createInstance(const char* s) {
map_type::iterator it = map.find(s);
if(it == map.end())
return 0;
return it->second();
}

template<typename T>
static bool AddClass(const char* s)
{
ComponentFactoryFuncPtr function = &createT<T>;
map.insert(std::make_pair(s, function));
return true;
}

private:
static map_type map;
};

BaseFactory::map_type BaseFactory::map;

class ComponentA : public Component
{
private:
float A;

public:
ComponentA() : Component("ComponentA")
{

A = 0.0f;
}
~ComponentA() { }
};
BaseFactory::AddClass<ComponentA>("ComponentA");

class ComponentB : public Component
{
private:
float B;

public:
ComponentB() : Component("ComponentB")
{

B = 0.0f;
}
~ComponentB() { }
};
BaseFactory::AddClass<ComponentA>("ComponentA");

int main(void)
{
//BaseFactory theFacory;
Component* testA = BaseFactory::createInstance("ComponentA");

return(0);
}

Share this post


Link to post
Share on other sites
Quote:
The problem is that, is that when I do this I will get the error

error C2761: 'bool BaseFactory::AddClass(const char *)' : member function redeclaration not allowed

And rightfully so as this is not allowed by the standard, gcc is probably using an extension here. Anyway I have to say it is damn ugly code that you are trying to shoe horn to fit here.

Quote:

However, to prevent the error I could do

bool result = BaseFactory::AddClass<ComponentA>("ComponentA");

Yes this is quite different as it is static initialisation, you could also stick the code in a type's constructor and have a global instance but again it is ugly.

Why don't you remove the static map and instead create an instance of the factory, adding the types to it in a function which you can call after the entry point and pass the instance around?

Share this post


Link to post
Share on other sites
Quote:
Original post by dmail
Yes this is quite different as it is static initialisation, you could also stick the code in a type's constructor and have a global instance but again it is ugly.


Sorry about the misrepresentation, this code I created with VS2010, rip-off did not do this in gcc at all.

Why don't you remove the static map and instead create an instance of the factory, adding the types to it in a function which you can call after the entry point and pass the instance around?[/quote]

Quote:
Original post by dmail
Yes this is quite different as it is static initialisation, you could also stick the code in a type's constructor and have a global instance but again it is ugly.


I was thinking about doing this, but it will not be added to the list until the first time the types constructor is called.

Quote:
Original post by dmail
Why don't you remove the static map and instead create an instance of the factory, adding the types to it in a function which you can call after the entry point and pass the instance around?


I just figured I would keep it static and avoid passing an instance around.

Share this post


Link to post
Share on other sites
Quote:
Original post by simotix
Sorry about the misrepresentation, this code I created with VS2010, rip-off did not do this in gcc at all.

Try looking again at the code rip-off posted :)

[Edited by - dmail on December 8, 2010 6:15:39 AM]

Share this post


Link to post
Share on other sites
Quote:

Try looking again at the code rip-off posted :)

My first post had code tested in GCC. The second was tested in MSVC 2010. Testing simotix's latest code in GCC, such calls are not allowed (with the default compiler flags).

Quote:

Why does it think I am trying to redeclare the function when I am trying to call it?

You cannot have a function call statement outside a function. You can only have variable initialisations, which may include a call to a function.

You have to be careful when doing this as a side effect of a variable initialisation. I believe the compiler may be allowed to optimise away certain non-local variables that aren't used, even if their initialisation would have side effects. At least I remember SiCrane talking about something like that, but the exact details escape me now and I cannot find the thread.

Some Googling suggests the above only applies to static variables, but that some linkers can discard unreferenced non-static variables from libraries.
Quote:

I just figured I would keep it static and avoid passing an instance around.

This isn't a great idea. Better to make the dependencies explicit and instantiate them safely inside main. If there are a good few of factories or objects that are passed to a large number of objects, bundling them into a single structure or class can help reduce the number of parameters passed.

I have a "Context" type, which includes references to such service objects and the game world and map.

Share this post


Link to post
Share on other sites
Quote:
Original post by rip-off
You have to be careful when doing this as a side effect of a variable initialisation. I believe the compiler may be allowed to optimise away certain non-local variables that aren't used, even if their initialisation would have side effects. At least I remember SiCrane talking about something like that, but the exact details escape me now and I cannot find the thread.

The relevant verbiage is in section 3.6.2 of the current version of the standard, which is long and complex, but to boil it down, an object with static storage duration (including global variables, static function variables and static member variables) is not guaranteed to be initialized before main() is called. If it's not initialized before main() is called then it has to be initialized before it's first used. So if a variable with static storage duration is never used, it never has to be initialized.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this