[PATCH] -Wcomma, a new warning for questionable uses of the comma operator

[Richard Trieu]

-Wcomma currently triggers 6 times in your example.  The BinaryOperator's
for the these comma operators does not show up in the AST dump.  I will
take a closer look when I have the chance.

Assert macros will not be a problem as this warning will not trigger inside
macros.


On Fri, May 30, 2014 at 9:26 PM, Arthur O'Dwyer <arthur.j.odwyer at gmail.com>
wrote:

> The name "-Wcomma" is pretty generic; I wonder if you could give it a
> more specific name. (But then again, "-Wquestionable-comma-usage" or
> whatever isn't any more helpful.)
>
> Please make sure the warning doesn't trigger on parameter-pack
> expansions that are being used for their cardinality; for example,
> this case from MPL11:
>
>     template <bool ...> struct bool_seq;
>     template <typename ...xs> using and_ =
>       std::is_same< bool_seq<xs::value...>, bool_seq<(xs::value, true)...>
> >;
>
>     static_assert(and_<std::true_type, std::true_type,
> std::true_type>::value == true, "");
>     static_assert(and_<std::true_type, std::false_type,
> std::true_type>::value == false, "");
>
> The expression "(xs::value, true)" might seem silly, but because it's
> followed by "..." it's actually sensible.
>
> I feel like there are other cases involving assert-esque macros that
> would be problematic, but I can't think of any good examples off the
> top of my head.
>
> HTH,
> Arthur
>
>
> On Fri, May 30, 2014 at 8:03 PM, Richard Trieu <rtrieu at google.com> wrote:
> > -Wcomma emits a warning when there is a questionable use of the comma
> operator.  It does this by only allowing certain expressions on the LHS of
> the comma operator, with all other expressions giving a warning.  The
> current whitelisted expressions are increments, decrements, assignments,
> compound assignments, overloaded versions of these operators, and void
> returning functions.  Some examples of code that will be detected:
> >
> >   int foo();
> >   if (foo(), 5) {} // should be "=="
> >   cout << "foo is " , foo();  // should be "<<"
> >
> >   void bar(int);
> >   void bar(int, int);
> >   bar((foo(), foo()));  // Too many parens, calls the one argument
> function
> >
> > http://reviews.llvm.org/D3976
> >
> > Files:
> >   include/clang/Basic/DiagnosticSemaKinds.td
> >   lib/Sema/SemaExpr.cpp
> >   test/SemaCXX/warn-comma-operator.cpp
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140530/8c47ce51/attachment.html>