Re: [lmi] Clang fixes

[Greg Chicares]

On 2016-03-26 00:27, Vadim Zeitlin wrote:
> On Fri, 25 Mar 2016 18:26:17 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> On 2016-03-25 13:48, Vadim Zeitlin wrote:
> GC> > On Fri, 25 Mar 2016 13:21:57 +0000 Greg Chicares <address@hidden> wrote:
> GC> [...]
> GC> > GC>   
> https://github.com/vadz/lmi/pull/7/commits/4a87eab6bcf6222e6c2bd64644f4a48b219a443b
> GC> > GC> | Remove unused progress_meter::display_mode_ member
> GC> > GC> | This field is never used and resulted in clang 
> -Wunused-private-field warning.
[...]
> GC> /lmi/src/lmi/progress_meter.cpp:75:25: error: unused parameter 
> 'display_mode' [-Werror=unused-parameter]
> GC>      ,enum_display_mode  display_mode
[...]
> I've fixed
> this now (please see https://github.com/vadz/lmi/pull/14) but, to address
> the warning, I'd just suppress the parameter name in this particular ctor
> definition, IMO it's the simplest and guaranteed correct way to do it and I
> don't see any need for any further changes today, do you?

I'll look at this later. BTW, above you mention:
  https://github.com/vadz/lmi/pull/14
and then later you wrote:
> https://github.com/vadz/lmi/pull/9/commits/a77ace84929150b3694e3ad2be9d3b99777a0fc8
Which set(s) of patches should I examine? 9 and then 14, in numerical order?
Or 9, 10, 11...14? Is there an "8" that I should examine first? Sorry, I just
don't know how github works, and I don't want to spend time doing the wrong
thing, so please give me very specific guidance. I was able to look at the
"7" set easily enough, so I don't need you to specify each commit by sha1sum;
I just don't know what patchsets exist and their order.

>  Also, a question about -Wextra: why are wx_dependent_objects compiled
> without it? With a few trivial changes, I don't see any reason not to use
> it for them too... If you'd like to enable it for the makefile builds too,
> I could test building using the makefiles after the changes of PR14 are
> merged.

I don't remember the specific problems, but 'workhorse.make' says:

# WX !! The wx library triggers many warnings with the following
# 'extra' flags. (Use '-W' for backward compatibility, instead of the
# modern equivalent '-Wextra'.)

Later I'll try building the wx-dependent code without this workaround,
which might no longer be necessary.

As for the option name...

https://gcc.gnu.org/onlinedocs/gcc-4.8.4/gcc/Warning-Options.html
| -Wextra ... This option used to be called -W. The older name is still
| supported, but the newer name is more descriptive.

...we now require gcc-4.9, so I'll replace '-W' with '-Wextra'.

> GC> I kind of thought you'd say that gnu getopt is perfect because it's plain 
> C
[...snip other, more substantial reasons...]
> 
>  Sorry but why should it be an advantage? I'd very much prefer a C++
> library than C one.

Here we only seem to disagree, but don't really. You're addressing getopt's API,
while I'm addressing its CLI.

You're saying the 'C' API of getopt is nasty. I don't disagree.

I'm saying that I want the user-observable behavior to be that of getopt. Thus,
'-abc', '--xyz', '-', and '--' should behave the same way they would for any
gnu utility, as documented in gnu manuals. I want to avoid any command-line
parser that doesn't behave exactly that way, because any other behavior would
be startling. The object code for gnu getopt meets this criterion by definition,
so in that respect it is ideal, though in other respects it's not.

> GC> Of course they answer in the negative, but their reasoning is not clear to
> GC> me. Why do you say it's definitely better? Does it behave the same way as
> GC> getopt?
> 
>  No, not at all, but that's the whole point. getopt is cryptic and *very*
> error prone as you can easily forget to handle some option or forget to
> handle an argument of an option which has one or try to use an argument of
> an option which doesn't define any or make just about any other error in
> the code handling them. The fact that handling the option is very far away
> from defining it doesn't help neither and the fact that you need to
> manually iterate over all options is pretty bad on its own. . I can go into
> the details, of course, but, to turn the question around, what are the
> advantages of getopt?

Its principal advantage--perhaps its only advantage--is that its CLI behavior
is extremely well defined and familiar to anyone who uses GNU utilities.

> GC> I certainly don't want to use 'cmake' to build a "library" that
> GC> seems to contain only one '.cpp' file.
> 
>  Of course there is no need to do it, the file could be easily just
> included in our own makefiles (how is this different from getopt?). And
> there are other, header-only command line parsing libraries.

I'd happily change to a facility that implements the getopt CLI with a
better API.

> GC> > GC> because we already have too many dependencies on boost.)
> GC> > 
> GC> >  We will have only Boost.Filesystem as soon as I submit all my C++11
> GC> > modernizing patches... But as those patches touch a lot of code, they
> GC> > depend on all the previous patches, so we're not quite there just yet.
> GC> 
> GC> If you can lay those previous patches out on github, I can make time
> GC> for them this weekend. De-boostifying is a worthwhile goal.
> 
>  These patches would result in conflicts with many (maybe all) of my
> outstanding patches. I'd really prefer to apply as many of those as
> possible (including the not yet submitted big mortality tables patch)
> first.

Agreed. That's what I was actually trying to say. I'd like to apply your
outstanding patches in their logical order, because that's the most
sensible way and requires the least work. Then we can apply your C++11
modernizing patches, which logically come later.