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

[Albert Astals Cid]

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.

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