LNK2005 linker errors when wxUSE_STL and an external module uses std::vector<int>

[hajokirchhoff]

This is with wxMSW, Visual Studio 2008, svn approx 10 months old.

Summary:

#define wxUSE_STL 1 prevents using std::vector<int> in non-wxwidgets modules when trying to link to wxWidgets dynamically. It causes several LNK2005 std::vector<int>::~vector<int> already defined in wxbase*.dll

Bug:

If #define wxUSE_STL 1, then wxbase*.dll exports all std::vector<T> members where T is short, int, double and a lot of others. This clashes with any module using std::vector<T> that attempts to link wxWidgets dynamically. I haven't tried static linking.

IOW, it is not possible to use std::vector<T> in my own code and dynamically link to wxWidgets when T is one of (int, double, short etc...)

Here are the details and a suggested fix:

The problem is dynarray.h, WX_DECLARE_BASE_ARRAY_2

#define _WX_DECLARE_BASEARRAY_2(T, name, predicate, classexp) \
classexp name : public std::vector<T> \

This will expand to

class WXDLLIMPEXP wxBaseArrayInt:public std::vector<int> ...

with WXDLLIMPEXP = __declspec(dllexport)

Experimenting with Visual Studio I found that if I dllexport a class that derives from a template, the linker will instantiate all template members and dllexport them as well.

So in this case all std::vector<int> members will be instantiated (code generated for them) and exported.

When a completely different module (DLL) uses the same template, the instantiated methods will clash with the exported ones from the wxbase*.dll and a linker error LNK2005 (function already defined in foo.obj) prevents linking.

Solution:

Instead of exporting the entire class, export only its members functions, so that the base class will not be exported.

Change DECLARE_BASEARRAY_2 to

#define _WX_DECLARE_BASEARRAY_2(T, name, predicate, classexp) \
class name : public std::vector<T> \
{ \
classexp name(); \
classexp size_t Count();

and so on.

This will expand to

class wxBaseArrayInt:public std::vector<int>
{
WXDLLIMPEXP wxBaseArrayInt();
WXDLLIMPEXP size_t Count();

I will prepare a fix and attach it to this ticket.

[hajokirchhoff]

Unfortunately the patch is a little bit more complicated than it might seem at first and also has the potential to break existing code.

The problem is that the original macro

#define  _WX_DECLARE_BASEARRAY(T, name, classexp)

expects the C++ keyword 'class' or 'struct' to be part of classexp. All through the wxWidgets sources you will see statements such as

// and an array of class info objects
WX_DEFINE_USER_EXPORTED_ARRAY_PTR(wxClassInfo *, wxArrayClassInfo,
                                    class WXDLLIMPEXP_BASE);

I personally think, passing 'class WXDLLIMPEXP_BASE' as the classexp parameter isn't a good idea anyway, but this also prevents fixing the duplicate symbol linker error LNK2005.

The fix requires changing
   size_t Count(); to something    classexp size_t Count();
If the keyword 'class' is part of classexp, the fix won't work.

Unless someone has a good idea how to remove   'class' from a parameter macro 'class __declspec(dllexport)'   and do this in a macro definition, the only way to fix this problem is to get rid of the 'class' keyword in the macro parameter.

That means changing all instances of WX_DEFINE_USER_EXPORTED_ARRAY*


But without fixing this problem, wxUSE_STL prevents _all_ use of std::vector<int, short, float, double> in modules other than wxWidgets.

I've attached a patch for dynarray.h showing the fix in principle.

[vadz]

AFAIR the macro was written like this because passing just WXDLLIMPEXP_BASE resulted in warnings in non-DLL build when this macro is empty and so the macro was called without enough parameters. OTOH I see that now we have wxARRAY_EMPTY which seems to take care of this (and also is redundant with wxEMPTY_PARAMETER_VALUE from wx/cpp.h...) so maybe it's not a problem any more.

In any case breaking the existing code using WX_DEFINE_USER_EXPORTED_ARRAY doesn't seem like a good idea as the error messages will undoubtedly be very unclear. This also prevents people from writing code compatible with both 2.8 and 3.0 which is bad as well. So the only solution I can see is to add another macro which would use expmode only before the methods and use it for wxArray{Short,Int,Double,Long,PtrVoid} definitions. I'd be glad to apply such a patch, of course, but the current one can't be used unfortunately, sorry.

[hajokirchhoff]

Replying to vadz:

WX_DEFINE_USER_EXPORTED_ARRAY doesn't seem like a good idea as the error messages will undoubtedly be very unclear. This also prevents people from writing code compatible with both 2.8 and 3.0 which is bad as well. So the only solution I can see is to add another macro which would use expmode only before the methods and use it for wxArray{Short,Int,Double,Long,PtrVoid} definitions. I'd be glad to apply such a patch, of course, but the current one can't be used unfortunately, sorry.

Hm, not nice, but I don't see a better solution. I'll prepare a patch. Thanks.

[hajokirchhoff]

Okay, I did some more experimenting on this and came up with a way to remove the 'class' keyword from the classexp parameter. I prepend the token remove_class_keyword_ to the classexp parameter of the WX_DECLARE_BASE_ARRAY_2 and also define an empty preprocessor macro remove_class_keyword_class.

WX_DECLARE_BASE_ARRAY_2(T, name, predicate, classexp) \

WX_DECLARE_BASE_ARRAY_2_noclasskeyword(t, name, predicate, remove_class_keyword_##classexp)

I changed the original WX_DECLARE_BASE_ARRAY_2 to WX_DECLARE_BASE_ARRAY_2_noclasskeyword and can now change the class definition, so that it no longer exports the entire class, but exports its members only.

The only incompatibility left now is wxARRAY_EMPTY. This macro is used to avoid a preprocessor error when the classexp is empty. Example:

#define WX_DEFINE_USER_EXPORTED_ARRAY(T, name, expmode) \

WX_DEFINE_TYPEARRAY_WITH_DECL(T, name, wxBaseArrayPtrVoid, wxARRAY_EMPTY expmode)

If expmode is empty, wxARRAY_EMPTY will still get passed by the preprocessor. If it weren't, some compilers would complain.

However this construct adds a space between wxARRAY_EMPTY and expmode. This space prevents my workaround above from working.

The incompatibility is this: The order of wxARRAY_EMPTY and expmode needs to be reversed.

Instead of

(T, name, ..., wxARRAY_EMPTY expmode)

you have to use

(T, name, ..., expmode wxARRAY_EMPTY)

I hope that the fact that wxARRAY_EMPTY is used only inside dynarray.h - and nowhere else in wxMSW - means that it is an internal construct only and this incompatibility has no effect in practice.

If accepted this patch would solve the original problem in a completely backward compatible way with minimum fuss and minimum code repetition.

[vadz]

Thanks for your work on this! I don't think there is any problem with compatibility here.

Looking at the patch I don't understand the #ifdef NOT part however -- does it really need to be kept? It looks like the patch could be simplified further by just removing it and always using the new version of the macro, am I missing something?

[hajokirchhoff]

Oops, sorry. #ifdef NOT is just my way of excluding a code block. I left the original code block intact for a while to compare it against the new version.

Yes, it can and should be removed. It does nothing.

Thanks

[vadz]

Looking at this again, while "remove class keyword" hack is ingenious, I'd like to be able to somehow avoid it... Would it be possible to modify the macros to take just the "expmode" without "class" in it in any case? I think I originally inserted "class" into this argument just to avoid the problems in static build (when "expmode" itself was empty) but as we had to use wxARRAY_EMPTY hack anyhow later, I don't see why do we need to keep it.

IOW would it be a problem to replace all occurrences of class expmode with wxARRAY_EMPTY expmode (or maybe something like wxARG_OR_EMPTY(expmode) where we'd have #define wxARE_OR_EMPTY(x) x wxEMPTY in wx/cpp.h) and insert class in the macros that need it? It looks to me like the code would be significantly simpler like this, even if it does demand more changes.

[hajokirchhoff]

That's what I did in my first patch 14 months ago: replace all occurrences of class expmode. But it would break WX_DEFINE_USER_EXPORTED_ARRAY, which might be used elsewhere. You thought it wouldn't be a good idea to break that macro (see your first comment to my ticket).

I agree that this would be a cleaner and IMHO better way, but, as I said, it would either break WX_DEFINE_USER_EXPORTED_ARRAY or we would need to introduce another macro (lots of actually) WX_DEFINE_USER_EXPORTED_ARRAY_WITHOUT_CLASS, which would make the code even less readable than the "remove class keyword" hack.

[vadz]

Sorry for yet another question about this but while preparing to apply the patch I couldn't help wondering if we need to export anything in wxUSE_STL case at all. I.e. I think that all methods of these classes are defined inline so would it be possible to just not use WXDLLIMPEXP_BASE in WX_DECLARE_USER_EXPORTED_BASEARRAY calls in wx/dynarray.h? AFAICS everything should still work then and nothing should be implicitly exported.

Am I missing anything here? If I had a test case I'd be able to try it myself but unfortunately I don't...

Thanks again!

[vadz]

We probably should just apply the patch but if anybody can test if removing the dllexport keyword at all could be enough to fix it, it would be great.

[troelsk]

Will gladly test this, but, remove dllexport from where exactly?

PS: Related ticket #14990

[hajokirchhoff]

Perhaps also related: #14741

[vadz]

Replying to troelsk:

Will gladly test this, but, remove dllexport from where exactly?

I don't remember what was I thinking when I wrote it any more so sorry in advance if it's something stupid, but the comment:11 proposed removing it from WX_DECLARE_USER_EXPORTED_BASEARRAY. And as the fix for #14741 demonstrates, not exporting the classes at all does seem to solve a similar problem elsewhere, so I think we should remove them, perhaps only keeping them for the old (<= 7?) versions of MSVC.

[troelsk]

I tried removing dllexport thus, dynarray.2.patch, and it did not work for me, again got the linker error reported in ticket #14990, below. The original dynarray.patch fix it.

1>Linking...
1>wxmsw29ud_core.lib(wxmsw295ud_core_vc_custom.dll) : error LNK2005: "protected: __thiscall std::_Container_base_aux_alloc_empty<class std::allocator<class wxWindow *> >::~_Container_base_aux_alloc_empty<class std::allocator<class wxWindow *> >(void)" (??1?$_Container_base_aux_alloc_empty@V?$allocator@PAVwxWindow@@@std@@@std@@IAE@XZ) already defined in docview.obj
1>wxmsw29ud_core.lib(wxmsw295ud_core_vc_custom.dll) : error LNK2005: "protected: __thiscall std::_Container_base_aux_alloc_empty<class std::allocator<class wxWindow *> >::_Container_base_aux_alloc_empty<class std::allocator<class wxWindow *> >(class std::allocator<class wxWindow *>)" (??0?$_Container_base_aux_alloc_empty@V?$allocator@PAVwxWindow@@@std@@@std@@IAE@V?$allocator@PAVwxWindow@@@1@@Z) already defined in mysource.obj

[troelsk]

I got that completely wrong sorry, please ignore comment above. Still do not know where to find that dllexport

[vadz]

How exactly can I reproduce the problem? I've tried the example from #14990 and also just using vector<int> but I don't see any problems, everything links just fine in release DLL build:

samples/minimal/minimal.cpp

@@ -183 +183 @@
     Close(true);
 }
 
+#include <vector>
+
 void MyFrame::OnAbout(wxCommandEvent& WXUNUSED(event))
 {
+#if wxUSE_STL
+    const wxWindowList wndList = GetChildren();
+    std::vector<int> nums(10);
+#else
+#error !wxUSE_STL
+#endif
+
     wxMessageBox(wxString::Format
                  (
                     "Welcome to %s!\n"

So what problem are we actually trying to solve here?

[hajokirchhoff]

Hi Vadim,
we recently had a fix that essentially removed the _declspec(dllexport) from the STD compatible wxArray classes. The (old) wxArray classes inherit from std::vector/std::list, if wxUSE_STL is defined. Until recently the resulting classes would be exported, but that fix removed dllexport and made them template classes only. The ticket is #14741 (I believe).

It is quite possible that that fix also resolves this problem. This problem here is caused by the fact that wxDECLARE_ARRAY will create a class derived from vector<int> and export that class. This will at the same time instantiate vector<int> itself and also export it. That causes the linker error I described in this ticket.

My guess is that since #14741 removes the dllexport from the wxDECLARE_ARRAY, the resulting class is still a template class, will not be exported and will no longer clash with other instances of vector<int>.

To verify that you could undo #14741 and see it the problem occurs then.

Or try to reproduce the bug as follows:
Compile wxWidgets DLLs with wxUSE_STL
Modify a sample to use vector<int> somewhere.
Try to link the sample with the wxWidgets DLLs.

[vadz]

Well, this is exactly what the patch in comment:17 does, or am I missing something?

So I'm tentatively closing it, please reopen if the problem can still be reproduced.

Thanks!

[troelsk]

please reopen if the problem can still be reproduced.

It still can, in a project of mine. I have been using the dynarray.patch since.
Just tried un-applying it: then again I get the error reported in #14990.
(using Visual Studio 2008, DLL target, wxUSE_STL + wxUSE_STD_CONTAINERS + WXUSINGDLL)

[vadz]

Are you still getting the same errors as in comment:15?

Any idea about how can I reproduce this?

[troelsk]

Any idea about how can I reproduce this?

No. Because I see it only in a dll compile here I tried sprinkling the dll sample with wxWindowList's, instantiating, exporting, without luck so far. Will try again if I get an idea.

Are you still getting the same errors as in comment:15?

Almost,

1>wxmsw29ud_core.lib(wxmsw295ud_core_vc_custom.dll) : error LNK2005: "protected: __thiscall std::_Container_base_aux_alloc_empty<class std::allocator<class wxWindow *> >::~_Container_base_aux_alloc_empty<class std::allocator<class wxWindow *> >(void)" (??1?$_Container_base_aux_alloc_empty@V?$allocator@PAVwxWindow@@@std@@@std@@IAE@XZ) already defined in mysourcefile.obj
1>wxmsw29ud_core.lib(wxmsw295ud_core_vc_custom.dll) : error LNK2005: "protected: __thiscall std::_Container_base_aux_alloc_empty<class std::allocator<class wxWindow *> >::_Container_base_aux_alloc_empty<class std::allocator<class wxWindow *> >(class std::allocator<class wxWindow *>)" (??0?$_Container_base_aux_alloc_empty@V?$allocator@PAVwxWindow@@@std@@@std@@IAE@V?$allocator@PAVwxWindow@@@1@@Z) already defined in mysourcefile.obj

[troelsk]

To summarize:
- I experience the linking problem only with wxWindowList, not with std::vector<int> as the OP does
- not reproducable in the samples

I suspect that this declaration change would fix the problem,

#if wxUSE_STD_CONTAINERS
class wxWindowList : public wxVector<wxWindow*> {};
#else
WX_DECLARE_LIST_3(wxWindow, wxWindowBase, wxWindowList, wxWindowListNode, class WXDLLIMPEXP_CORE);
#endif

BUT this breaks wx build in many places as std::vector has no Find/Append/GetCount/GetFirst/DeleteObject methods, quite a bit of work to STL'ize wx code using wxWindowList.
Is it worth considiring going down this path??

[hajokirchhoff]

If you have this problem only with wxWindowList, the problem may be in your code. Are you using a vector<wxWindow*> or something somewhere?

[troelsk]

No, no vector<wxWindow*> anywhere.
The problem seems to be inside wx, but is not reproducible in the samples :-(

[troelsk]

I narrowed it down to this one line in my dll,

    const wxWindowList wndList(frame.GetChildren()); // make a copy of the window list -> linker error in wx trunk+wxUSE_STL+DLL

Making it a reference (not what I want here) makes it link

    const wxWindowList& wndList(frame.GetChildren()); // no dtor called, linker is happy

Still not reproducible in the sample. Maybe the compiler is broken, closing ticket again.
(Will make myself a workaround using a simple array of wxWindow pointers)