GNOME Bugzilla – Bug 611690
AG_GST_SET_ERROR_{C|CXX}FLAGS: allow custom warning flags
Last modified: 2010-03-23 11:33:26 UTC
The following patches add the possibility for modules to add more warning flags if they so desire. The intent is to use this argument to allow: - different anality of warning flags for different modules. (In particular allow me to have my favorite flags in gstreamer-cairo) - a step-by-step cleanup of GStreamer modules before adding new warning flags to the common module. This way we can clean up modules one-by-one instead of requiring one big swoosh. In the end, the important warning flags should be added to the commmon module, so that there's generally (lmost) no need to use this new argument anymore.
Created attachment 155110 [details] [review] Add AS_COMPILER_FLAGS from Swfdec And use it to simplify code.
Created attachment 155111 [details] [review] Add additional-flags argument to AG_GST_SET_ERROR_CFLAGS This allows applications to specify additional warning flags to AG_GST_SET_ERROR_CFLAGS() and AG_GST_SET_ERROR_CXXFLAGS() that will be added to the ERROR_CFLAGS.
> Created an attachment (id=155110) [details] [review] > Add AS_COMPILER_FLAGS from Swfdec > > And use it to simplify code. Not a big fan of this patch. Maybe it's me, but I don't think it actually makes things much clearer or cleaner (apart from that, is the use of $CFLAGS in AS_COMPILER_FLAGS correct for C++ as well?) I'd prefer it if we could do without this. > Created an attachment (id=155111) [details] [review] > Add additional-flags argument to AG_GST_SET_ERROR_CFLAGS > > This allows applications to specify additional warning flags to > AG_GST_SET_ERROR_CFLAGS() and AG_GST_SET_ERROR_CXXFLAGS() that will be > added to the ERROR_CFLAGS. This looks fine to me, and a good idea, but preferably without relying on the previous patch (IMHO). Costs us but a foreach..
Created attachment 155723 [details] [review] Add a MORE_FLAGS argument to AG_GST_SET_ERROR_C(XX)FLAGS This allows modules to specify additional warning flags to AG_GST_SET_ERROR_CFLAGS() and AG_GST_SET_ERROR_CXXFLAGS() that will be added to the ERROR_CFLAGS.
Created attachment 155724 [details] [review] Add a MORE_FLAGS argument to AG_GST_SET_ERROR_{C|XX}FLAGS This allows modules to specify additional warning flags to AG_GST_SET_ERROR_CFLAGS() and AG_GST_SET_ERROR_CXXFLAGS() that will be added to the ERROR_CFLAGS.
The only problem I see with this patch is that it spams 30 lines of warning flag checks to stdout when running configure and people might not like that.
I'd say let's make it work first, we can always improve the internal workings later. I don't think a few more lines of configure output are a problem. Non-gcc users might get annoyed by the 'warning' about unsupported flags though (this is going to be pretty gcc centric..) (Do you agree that there may be a $CFLAGS / C++ issue with your original patch, or is that not a problem?)
I can remove the warning, but I think it's useful for the huge amount of people that do use gcc in older version. We can of course only warn if the compiler is gcc. And yeah, I think there are C/C++ issues with the previous approach. I wanted to be able to do this whole trickery with just one macro. This copy/pasting between the two cases is so error-prone. :/
I'd demote the warning to a notice maybe, but I'm happy with it otherwise. Don't see much point in special-casing for different compilers just to avoid some ultimately harmless configure output.