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

[Richard Trieu]

Arthur,

I have included a note that includes a fix-it to silence the warning.  This
seems to be the best way to handle this as the compiler cannot infer the
safe use of the comma operator in your example.  Casts to void will be
silence -Wcomma.

Richard


On Fri, May 30, 2014 at 10:22 PM, Richard Trieu <rtrieu at google.com> wrote:

> -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/20140612/703dc148/attachment.html>