wxWidgets and Windows.h issue

[Dinin]

Hi,
we are developing a software with wxWidget and we found an issue.
I reproduced the error on a small test app that I will attach.

OS: Windows
IDE: Visual Studio 2013
wxWidget: 3.0.2 (compiled with CMake)

As you may know the inclusion of Windows.h can be dangerous as it will define some ugly macros (CreateDialog, OpenMutex, CreateEvent, SetPort ecc).
I know that wxWidget provide an header (wx/msw/winundef.h) that, as written in the wx code, "#undef the macros defined in winsows.h which conflict with code elsewhere".
winundef.h undef only the macros that conflicts witch wxWidgets, other macros as SetPort will remain defined (which is fine).

The main problem is that, with the latest wx implementations, the file wx/dialog.h now include Windows.h (wx/dialog.h include wx/sharedptr.h, that include wx/atomic.h, that include wx/msw/wrapwin.h). This means that you can include Windows.h even if you don't want it as it can create some compiling/linking issue.
I think that this problem does not exists in the older wxWidget versions (we used to compile wxWidget 2.9.3 before the upgrade) but I cannot test it right now.

As I know wxWidget should not include Windows.h, is this inclusion wanted?


I made a small test application to be sure that the problem is not in our main software. I created a simple class that contains a function named SetPort (the same name of a Windows.h macro). When I use this class inside a dialog I get a compiler/linker error (the error depends on the position of the inclusions).
To change your wxWidget path go to "View -> Property Manager", expand "TestIncWin" and "Debug | Win32", open the "Environment" prop and go to "User Macros". There you can change the "wxWidget_PATH" value.


Tnx for your time, any information can be helpful.
Mario

[catalin]

If you look at most of (all?) the samples, you will see a headers sequence where something like this is written:

Code: Select all

// For compilers that support precompilation, includes "wx/wx.h".
#include "wx/wxprec.h"

// for all others, include the necessary headers (this file is usually all you
// need because it includes almost all "standard" wxWidgets headers)
#ifndef WX_PRECOMP
    #include "wx/wx.h"
#endif

Do that before adding explicit headers, unless you really know what you are doing. Doing this will hide any individual header changes that occur from time to time.

p.s. If you went to the trouble of making "a small test app", why did you use 8 files for it ?

[Dinin]

Hi, tnx for the fast reply!

The software I'm currently developing is quite big so I want to use only the minimal includes (this way the code should be easier to read and portable).

Anyway including wx/wxprec.h before the other includes removes the compile error but creates a link error.
Including wx/wxprec.h in every file or with a precompiled header is not a good design in my opinion, especially in a big software.

If the function that creates the error is inside an external library, the issue cannot be removed simply moving the inclusions.

I can hide the problem (for now) changing the function names with something that is not already defined in Windows.h, but someone could find the same problem using external libraries.

Sorry if I'm asking again: is the Windows.h inclusion inside wx/dialog.h really necessary?


As I said, I am able to hide (not remove) the problem, I'm writing to see if someone had the same problem and to improve wx (if necessary).
Tnx for your support,
Mario

[doublemax]

Sorry if I'm asking again: is the Windows.h inclusion inside wx/dialog.h really necessary?

I suggest to ask this in the wx-users mailing list where you can reach the core wx developers.
https://groups.google.com/forum/#!forum/wx-users

Before you do that, try to create a simpler sample. It should be possible to demonstrate the issue by adding just a view lines to the "minimal" sample.

[catalin]

I want to use only the minimal includes (this way the code should be easier to read and portable).

Very well, but first, you have to include wxWidgets' header files, of course.

Including wx/wxprec.h in every file or with a precompiled header is not a good design in my opinion, especially in a big software.

I don't know if you need to include them in every file, but why not? Those are headers of a toolkit, and they will not change (unless you upgrade to a new version, but that should be rare enough).
I for example try to keep them in .cpp files and if possible avoid including them in my headers, which in turn are included after wxW headers in cpp files.
People could come up with new questions here, like: after decided to use wxW in the new big software, the decision to not follow wxW guidelines was it part of a good design? Polemics will hardly solve your build issues...

Anyway including wx/wxprec.h before the other includes removes the compile error but creates a link error.

That may or may not be related to the topic. Did you try a clean build?

If the function that creates the error is inside an external library, the issue cannot be removed simply moving the inclusions.

I can hide the problem (for now) changing the function names with something that is not already defined in Windows.h, but someone could find the same problem using external libraries.

I don't think I understand what you are trying to say here.

is the Windows.h inclusion inside wx/dialog.h really necessary?

I'm pretty sure it is, but to avoid searching mail archives for dialog topics I'll advise you to ask again on the mailing list.

[Dinin]

Hi,
to make things easier I created the same Test App with just 3 files:
- "TestIncWinApp.cpp" contains everything wx needs to work. wx/wxprec.h, wxApp and the dialog.
- "TestIncClass.h/.cpp" contains a simple class with a method "SetPort". To create the error this class must be in a separate file.

If the function that creates the error is inside an external library, the issue cannot be removed simply moving the inclusions.

I meant that the class TestIncClass could be defined in an external library that don't use wx (in this case the only "visible" file should be TestIncClass.h).


This code (that you can find as attachment) uses the basic wx guidelines.
In this example the TestIncClass.h file is included after wx/wxprec.h and a TestIncClass object is used inside the App.

Here there is the link error:

Code: Select all

1>TestIncWinApp.obj : error LNK2019: unresolved external symbol "public: void __thiscall TestIncClass::SetPortW(int)" (?SetPortW@TestIncClass@@QAEXH@Z) referenced in function "public: virtual bool __thiscall TestIncWinApp::OnInit(void)" (?OnInit@TestIncWinApp@@UAE_NXZ)

As you can see there is an unresolved external symbol for the function SetPortW. The function SetPort is translated in SetPortW in the files with the Windows.h inclusion (TestIncWinApp.cpp and TestIncClass.h).

Tnx for the advice and for your time, I'll probably ask the same question in the mailing list!
Mario

[magras]

Hello. Sorry for necroposting, but i can't find continued discussion of this topic in the mailing lists (neither wx-users, nor wx-dev, nor wx-discuss). Am i missing something, or it just didn't happened?

[doublemax]

Hello. Sorry for necroposting, but i can't find continued discussion of this topic in the mailing lists (neither wx-users, nor wx-dev, nor wx-discuss). Am i missing something, or it just didn't happened?

AFAIR this was never posted to the mailing list. And it's also possible that it has been fixed in the meantime.

Do you have the same problem?

[ONEEYEMAN]

Hi,
I vaguely remember Vadim pushing some fixes in that area so that windows.h can be included anywhere in the project.
However, I think you should grab the latest HEAD for that to be true.

But I'm wondering if the dialogs sample or widgets sample works for you? Because this would be the first question I'd ask the OP...

Thank you.

[stahta01]

There is two clear ways to fix this issue.

1. Create your own macro to define the macro needed.

2. Create an C++ source file with the functions/methods that need to use the macro without any wxWidget code/headers used in it.
Inside it include the windows.h header.

Tim S.

[magras]

Do you have the same problem?

Not exactly the same, but the root of the problem is as always windows.h. In my case, it is a conflict with boost.asio about winsock2. It is solvable with right include order or keeping wx and asio in different translation units. But I agree with Dinin that it would be good to get rid of windows.h in public wx headers. I don't want to touch a nerve, but i was curios about this topic, because wx isn't header-only lib, so it should be possible to hide it. I think technically it's possible even for header-only lib, but it's tedious and hard to support. So you can view this post just as «+1» to Dinin opinion.

I vaguely remember Vadim pushing some fixes in that area so that windows.h can be included anywhere in the project.
However, I think you should grab the latest HEAD for that to be true.

Thank you for suggestion.

[ONEEYEMAN]

Hi,
You could also try to send an e-mail to wx-dev asking about that and I'm sure Vadim will try to fix it.
This forum is for users and no core wx-devs are coming here - except maybe tierra. But he is more an infrastructure person for the library.

Thank you.