Re: [lmi] [PATCH] Avoid harmless but annoying arg[cv] shadowing warnings

[Vadim Zeitlin]

On Sat, 4 Jun 2016 17:19:42 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2016-06-04 14:30, Vadim Zeitlin wrote:
GC> > 
GC> >  After fixing the 2 "possibly uninitialized variables used" warnings in
GC> > a3e46591c01cfc970a715b14b8764002d7eca013, there are just two more 
(related)
GC> > warnings remaining when building lmi with MSVC 2015 and so, even if it's a
GC> > minor problem, I couldn't prevent myself from fixing it in this patch:
GC> > 
GC> >   https://github.com/vadz/lmi/pull/37
GC> > 
GC> >  As you can see there, it just uses wxApp::arg[cv] directly in
GC> > Skeleton::ProcessCommandLine() instead of passing them to it from
GC> > Skeleton::OnInit() as parameters.
GC> 
GC> Actually, I can't see it

 Sorry, I should have been more precise, I meant "as you can see in the
patch, arg[cv] are now used directly in ProcessCommandLine()".

GC> (but I'll apply the patch anyway).

 Thanks, I now have an entirely warning-free MSVC build (at least for
lmi_wx, I expect there are still warnings when building the tests etc) for
the first time ever!

GC> I guess wxApp knows what argc and argv are, and supplies them silently
GC> if they aren't explicitly given.

 Actually wxApp::arg[cv] just store the copies of the main() parameters or,
in WinMain() case, the result of parsing the command line string.


GC> >  It's not a problem for me to keep this change in a local branch, as I 
need
GC> > to use it to build with MSVC anyhow, but it's always better to keep the
GC> > differences between it and master to a minimum, so it would be nice if 
this
GC> > could be applied. It could also allow us to enable -Wshadow (currently
GC> > commented out in workhorse.make) in the future for the gcc builds.
GC> 
GC> Last sentence--not quite, e.g.:
GC> 
GC> /opt/lmi/third_party/include/xmlwrapp/node.h: In constructor 
'xml::node::text::text(const char*)':
GC> 
GC> /opt/lmi/third_party/include/xmlwrapp/node.h:163:42: error: declaration of 
'text' shadows a member of 'this' [-Werror=shadow]
GC> 
GC>          explicit text (const char *text) : t(text) {}
GC> 
GC>                                           ^
GC> And a zillion boost errors, and more.

 Yes, this is why I wrote "in the future"... I didn't look at this in
details, but the simplest solution would be to ignore the warnings in these
headers using either -isystem gcc option (probably preferable) or #pragmas
disabling/reenabling warnings before/after including third party headers.
Unlike the other recently discussed warnings, I think it would be worth
enabling this one because -Wshadow does find real problems relatively often
in average C++ code. lmi quality is higher than that, of course, but I
still think it could be useful even for it.

 I do wonder why I don't see any MSVC warnings of this kind for boost
though (I don't see them for xmlwrapp because I don't use xml_lmi.cpp in
MSVC projects and just link with a separately built xmlwrapp library)...

VZ