On 22-3-2014 5:42, Krzysztof Kosiński wrote:
2014-03-21 23:25 GMT+01:00 Johan Engelen <jbc.engelen@...2592...>:
Ouch..... It'd be great if some people could work on at least getting configure to work with -Werror.
-Werror should NEVER be enabled by default, as it is equivalent to explicitly disabling forward compatibility with compilers and libraries.
I think it should definitely be default during development. And then you simply disable it for release.
- The compiler might add a new warning, and it so happens that
existing code triggers it. Build broken.
Great! Compiler builders are very careful when adding new warnings to -Wall. When a new warning is triggered, it very likely means our code is bad and needs improvement. Breaking the build is (in our project) the only reliable way to get people to fix things.
- An external library we're using might deprecate a function. Build broken.
Again, great! We don't want to release code that will be broken in the future because it uses a deprecated function, right?
- An external library might add warning-generating code to one of its
headers. Build broken.
This is a valid concern, and is why I added -Wno-error=... flags. For example, libcroco (our copy in any case) is broken as it uses guchar* where it calls external lib functions that expect gchar*. And our devlibs version of boost::make_shared also contains a cast-alignment warning. Disabling erroring on specific warnings is easy, and much better in my opinion than not errorring on warnings at all. I'm pretty sure that the devs of libraries worth depending on try hard to resolve any warnings. In lib2geom we *really* should build with -Werror.
In other words, -Werror should only be enabled when someone explicitly requests it in configure. Only specific warnings should be converted to errors.
In my opinion, -Werror should be default for our development. Only specific warnings should be allowed to not error. (this set is much smaller than it's inverse)
-Johan