Re: wxWidgets 2.9 build - Mailing list pgadmin-hackers
From | Peter Geoghegan |
---|---|
Subject | Re: wxWidgets 2.9 build |
Date | |
Msg-id | AANLkTinNqjN25cZw0eKA1HByh7bTbiXYCTLRru-18=Ba@mail.gmail.com Whole thread Raw |
In response to | Re: wxWidgets 2.9 build (Dave Page <dpage@pgadmin.org>) |
Responses |
Re: wxWidgets 2.9 build
|
List | pgadmin-hackers |
On 29 January 2011 09:30, Dave Page <dpage@pgadmin.org> wrote: > Builds fine on my Mac. I wonder, should we use a macro to improve > readability, eg: > > #define cstr(x) ((const wxChar *)x) > > (can't use c_str - that seems to be in use already on VC++ 2008) c_str() is the member function that std::string clients use when interfacing with C. The convention has inspired the wxWidgets developers, who have a long term goal to move to using std::string, but it has also inspired, for example, libpqxx, where there is a c_str() member function of a proxy class that encapsulates single datums from result sets. I think that this exposes us to problems in the future. We're following the documented way of getting 2.8 and 2.9 compatibility here. I generally dislike using macros for things like this when working in C++. > Also, did you change all of the variadic calls? I still see a > wxLogInfo with a .c_str() parameter in pgadmin/db/pgConn.cpp:169 for > example. No, I didn't. I left it up to my compiler. Note that the wx built-in functions like wxLogInfo are defined using macros to construct each variant in /wx/log.h . The calls I fixed are to variants of our own, similarly defined using macros in sysLogger.h, but with a different function signature to the 2.9 wx ones. Ours (which is behaviourally identical to wx 2.8's) is: extern void wxLog##level(const wxChar *szFormat, ...) COMPAT_ATTRIBUTE_PRINTF_1 // remember I created the compatibility macro COMPAT_ATTRIBUTE_PRINTF_1? In Wx's 2.9's things are more complex. Here is a comment from that header: /* The code below is unreadable because it (unfortunately unavoidably) contains a lot of macro magic but all it does is to define wxLogXXX() such that you can call them as vararg functions to log a message at the corresponding level. More precisely, it defines: - wxLog{FatalError,Error,Warning,Message,Verbose,Debug}() functions taking the format string and additional vararg arguments if needed. - wxLogGeneric(wxLogLevel level, const wxString& format, ...) which takes the log level explicitly. - wxLogSysError(const wxString& format, ...) and wxLogSysError(long err, const wxString& format, ...) which log a wxLOG_Error severity message with the error message corresponding to the system error code err or the last error. - wxLogStatus(const wxString& format, ...) which logs the message into the status bar of the main application window and its overload wxLogStatus(wxFrame *frame, const wxString& format, ...) which logs it into the status bar of the specified frame. - wxLogTrace(Mask mask, const wxString& format, ...) which only logs the message is the specified mask is enabled. This comes in two kinds: Mask can be a wxString or a long. Both are deprecated. In addition, wxVLogXXX() versions of all the functions above are also defined. They take a va_list argument instead of "...". */ Obviously the wx guys were aware of the need to change their implementation to prevent the c_str() proxy ambiguity thing happening to their clients. What do you want to do about it? I'm not aware of there being an official way to extend the wx logger functionality. > On VC++ 2008, I get the following warnings: > > Warning 1 warning C4800: 'const wxChar *' : forcing value to bool > 'true' or 'false' (performance Oh yeah, I know the C4800 warning well. I'm glad to see it, because it has revealed that I've introduced a bug, which nicely answers my earlier question of "why does sysSettings::Write(const wxString&, const wxChar*) take a const wxChar* as its second argument rather than just a wxString?". We're calling the wrong overload where we don't supply a wxString directly, because C++ prefers converting to a built-in type to converting using a class constructor with a wxChar* argument. I'm going to change the bool overloads and perhaps others to functions with distinct names. Things are obviously too brittle as they stand. Objections? >> or would were it not for OGL and Flex/Bison issues with 2.9. > > Whats the plan with OGL? I think we should get a list of all the > committers to it (excluding the bakefile, which we don't need), and > email them, explaining that Julian has given permission to relicence > the code, and asking if they'll be kind enough to do the same. > > Thoughts? Do you want to do that, or should I? I imagined that you wanted to use your legal resources to make sure that it was done the right way, but I don't mind doing the ground work. Will I go ahead with it? The problem with asking me to do it is that I will split hairs to be on the very safe side, which may not be necessary. I suppose I can just consult you if I hit a snag. -- Regards, Peter Geoghegan
pgadmin-hackers by date: