[cfe-dev] Explicit warning diagnostic flags for groups

[David Blaikie]

On Mon, Aug 12, 2013 at 10:30 PM, Peter N Lewis <peter at stairways.com.au> wrote:
> Hi,
>
> I'm new to this group, so forgive me if this is the wrong place.
>
> I use clang with Xcode and like to run with all warnings (-Weverything -Werror) and then disable whatever warnings/enable whatever features I feel I can justify.  Unfortunately, sometimes there are features like GNU's ?: expression extension, eliding middle term which do not have their own flags, so then the choice is either enable all the GNU extensions or do without the extension.
>
> I've hit this in the past with clang, for things like vararg macros and such.  So I'd be interested in seeing this remedied.  I checked out the clang source and it appears that all that is needed to resolve this (for the eliding middle term case) is a simple patch like this:
>
> Index: include/clang/Basic/DiagnosticGroups.td
> ===================================================================
> --- include/clang/Basic/DiagnosticGroups.td     (revision 188139)
> +++ include/clang/Basic/DiagnosticGroups.td     (working copy)
> @@ -52,6 +52,7 @@
>  def ConfigMacros : DiagGroup<"config-macros">;
>  def : DiagGroup<"ctor-dtor-privacy">;
>  def GNUDesignator : DiagGroup<"gnu-designator">;
> +def GNUTernaryElidingExpression : DiagGroup<"gnu-ternary-eliding-expression">;
>
>  def DeleteNonVirtualDtor : DiagGroup<"delete-non-virtual-dtor">;
>  def AbstractFinalClass : DiagGroup<"abstract-final-class">;
> @@ -524,7 +525,8 @@
>
>  // A warning group for warnings about GCC extensions.
>  def GNU : DiagGroup<"gnu", [GNUDesignator, VLAExtension,
> -                            ZeroLengthArray, GNUStaticFloatInit]>;
> +                            ZeroLengthArray, GNUStaticFloatInit,
> +                            GNUTernaryElidingExpression]>;
>  // A warning group for warnings about code that clang accepts but gcc doesn't.
>  def GccCompat : DiagGroup<"gcc-compat">;
>
> Index: include/clang/Basic/DiagnosticParseKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticParseKinds.td (revision 188139)
> +++ include/clang/Basic/DiagnosticParseKinds.td (working copy)
> @@ -127,7 +127,7 @@
>  def ext_gnu_statement_expr : Extension<
>    "use of GNU statement expression extension">, InGroup<GNU>;
>  def ext_gnu_conditional_expr : Extension<
> -  "use of GNU ?: expression extension, eliding middle term">, InGroup<GNU>;
> +  "use of GNU ?: expression extension, eliding middle term">, InGroup<GNUTernaryElidingExpression>;
>  def ext_gnu_empty_initializer : Extension<
>    "use of GNU empty initializer extension">, InGroup<GNU>;
>  def ext_gnu_array_range : Extension<"use of GNU array range extension">,
>
> But I'm not familiar with clang's code, and so there may be more, or there may be tests that need doing, or documentation, or whatever else that goes along with this sort of thing.
>
> I'm happy to put some effort in to the grunge work of doing this if someone more familiar with clang can set me on the right path as to what else needs to be done and what process is needed for submitting the patches, and presuming this is something that will be accepted into clang, and thus eventually work its way into Xcode.

You're on the right track - adding warning flags is a desired thing
(in fact, while we haven't gone to the effort of adding flags for
every warning, we have a check that ensures we don't add more
unflagged warnings (& that if we add flags to a warning we don't
regress this by removing those flags later)).

If you're interested in actually going through the whole process of
contributing a patch, you'll find the basics here:
http://llvm.org/docs/DeveloperPolicy.html (don't treat it all as
gospel - yeah, you don't need to be reading everything on the mailing
list to contribute one standalone patch).
http://llvm.org/docs/GettingStarted.html will get you through
compiling/building - once you've done that & made your change, if you
run the tests ("make check-clang" should do it) you'll probably notice
a test or two failing (the one that ensures we don't regress flags
being one I would expect - you'll need to tweak that list so that
these warnings no longer appear in it, this way we'll not regress this
by accident later on).

The only other thing would be to add a more specific test that
demonstrates this flag working - turning it on & off, etc. I'm not
sure exactly where those tests are, but you might look around for
something similar in tools/clang/test (perhaps if you look in the
version history for the last time the diagtool test was changed you'll
see the last new flag that was added and how other testing was
handled)

>
> Any help on making this happen would be appreciated.
>
> Thanks,
>    Peter.
>
> --
> Keyboard Maestro 6.1 now out - set web checkboxes & radio buttons, exit from loops, and more.
>
> Keyboard Maestro <http://www.keyboardmaestro.com/> Macros for your Mac
> <http://www.stairways.com/>           <http://download.keyboardmaestro.com/>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev