[poppler] warning sets - was - Re: cmake CXXFLAGS

[Albert Astals Cid]

El dissabte, 16 de setembre de 2017, a les 11:45:15 CEST, Albert Astals Cid va 
escriure:
> El dissabte, 16 de setembre de 2017, a les 9:55:09 CEST, Adrian Johnson va
> 
> escriure:
> > On 11/09/17 20:30, Albert Astals Cid wrote:
> > > El diumenge, 10 de setembre de 2017, a les 20:39:18 CEST, Adrian Johnson
> > > va
> > > 
> > > escriure:
> > >> Also in PopplerMacros.cmake there are three levels of warning options:
> > >> "no", "yes", and "kde". Is there any reason we can't have one standard
> > >> set of warning options? I can't think of any reason for wanting to turn
> > >> off warnings. If anyone needs some extra warnings they can be added
> > >> with
> > >> CXXFLAGS.
> > > 
> > > We have the same in configure.ac:1154
> > > 
> > > I'm more than happy to have a single good set of warnings we agree on.
> > 
> > I've had a go at coming up with a list of warnings. I have made the
> > following warnings default as the compile is reasonably clean.
> > 
> >   -Wall -Wextra -Wpedantic
> >   -Wno-unused-parameter -Wno-missing-field-initializers
> >   -Wcast-align
> >   -Wformat-security
> >   -Wframe-larger-than=65536
> >   -Wlogical-op
> >   -Wmissing-format-attribute
> >   -Wnon-virtual-dtor
> >   -Woverloaded-virtual
> >   -Wsuggest-override
> > 
> > -Wextra and -Wpedantic produced a fairly clean compile after I disabled
> > -Wunused-parameter and -Wmissing-field-initializers. Most of the other
> > warnings are existing warnings (either "yes" or "kde" options). I added
> > -Wlogical-op as it helps avoid mixing logical and bitwise ops. I added
> > -Wframe-larger-than=65536 because if the stack frame is larger than this
> > you are doing something wrong (largest currently is 52K in splash).
> > 
> > I've replaced the COMPILE_WARNINGS option with an EXTRA_WARN boolean
> > 
> > option default to off. When on it enables:
> >   -Wconversion
> >   -Wmissing-declarations
> >   -Wshadow
> >   -Wundef
> >   -Wuseless-cast
> >   -Wzero-as-null-pointer-constant
> > 
> > These produce a lot of warnings. Most of them probably should be fixed.
> > 
> > All of the "kde" warnings except -Wlong-long are included in the above
> > two sets of warnings. Some of them are part of "-Wall -Wextra
> > -Wpedantic" so I have removed them from the list. The
> > "-D_XOPEN_SOURCE=600 -D_BSD_SOURCE" is deprecated by "-D_DEFAULT_SOURCE".
> > 
> > If we can either fix the extra warnings or decide not to care about some
> > of them, they can be moved to default or removed and the EXTRA_WARN
> > option can be removed.
> 
> Looks good to me. Please commit.

Wait, one thing i forgot, you've raised the gcc requirement to gcc 5 by always 
adding suggest-override, please leave that wrapped in the if so that it works 
with gcc 4.x too.

Cheers,
  Albert

> 
> Cheers,
>   Albert
> 
> 
> P.S: -Wmissing-declarations
> seems like it should be easy to promote, just needs defining those functions
> without declaration as static and it seems to makes the warning go away.
> > > Cheers,
> > > 
> > >   Albert
> > > 
> > > _______________________________________________
> > > poppler mailing list
> > > poppler at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/poppler
> 
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/poppler