Re[2]: [lmi] product editor patch

[Vadim Zeitlin]

On Thu, 07 Feb 2008 21:34:07 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2008-02-07 19:10Z, Vaclav Slavik wrote:
GC> > 
GC> > https://savannah.nongnu.org/patch/index.php?6411
GC> 
GC> I wonder whether we're using different wx versions. I'm using
GC> wx-2.8.7 as of a few days ago (with only one change:
GC>   http://lists.nongnu.org/archive/html/lmi/2008-02/msg00011.html
GC> which is in wx svn anyway), and MinGW gcc-3.4.4 .
GC> 
GC> After applying the product-editor patch I'm seeing many warnings
GC> (where there were none before with wx-2.8.7), mostly like this:
GC> 
GC> /lmi/src/lmi[0]$<../log grep "declared as dllimport: attribute ignored" |wc 
-l
GC> 2685

 Hello,

 We've just spent an hour or so on collective hair pulling with Vaclav but
finally found the reason for this problem: it's not due to changes in wx
but due to workhorse,make magic: it uses relaxed gcc warnings for the files
which use wx code, and in particular -W is not used for wx as it results in
this warning and there is no way to suppress it with gcc 3.4. However
workhorse.make uses a heuristics to find the files using wx: it just greps
for "include.*<wx". But this fails now for tier_view_editor.cpp (and a few
other files) because Vaclav has corrected the problem indicated by
"// WX !! why include unused wx headers here" comments as part of his
changes. So now this files doesn't include any wx headers -- but it still
uses wx. As you can see, the explanation is rather simple finally but
arriving at it was quite fun :-/

 Anyhow, to solve this I can immediately see 2 solutions:

1. Fool-proof but heavy to implement one: use gcc -M to find the list of
   all dependencies of each file and use this to determine if it uses wx
   (searching for wx/defs.h in the list of dependencies would be enough)

2. Much simpler but potentially fallible way is to just grep for "wx"
   (possible improvements: only at the beginning of the word; only outside
   the comments (which is not so trivial for C comments but should be
   doable with sed... and certainly doable with cpp if we're ready to run
   it for all the files just for this))

What would you prefer? Or maybe you see a better solution?


GC> We saw something similar before:
GC>   http://lists.nongnu.org/archive/html/lmi/2006-04/msg00002.html

 BTW, following this link and your post linked from there (the original URL
doesn't work, but 
http://readlist.com/lists/lists.sourceforge.net/mingw-users/0/327.html
is probably the same post) I see that my explanation about the need of
dllimport for inline functions was apparently wrong. However it still
wouldn't be easy to fix because the warning appears for the inline methods
of a class using WXDLLIMPEXP declaration. And while some classes (such as
wxCharBuffer which is the first one giving this warning) are fully inline,
others have both inline and non-inline methods and if we don't use
WXDLLIMPEXP on the class declaration we need to use it for all non-inline
methods which is not only annoying but also extremely error-prone as it's
awfully easy to forget it on a method. So I'm afraid we have no choice but
to continue to avoid -W with wx code.

 OTOH -Wcast-qual (which currently results in a dozen of warnings in wx
headers) could probably be worked around. And I haven't seen any
occurrences of warnings due to -Wredundant-decls so it should be safe to
enable them for all source files.

 Regards,
VZ