This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add a warning for suspicious use of conditional expressions in boolean context


On 09/12/16 22:08, Jeff Law wrote:
> On 09/12/2016 02:00 PM, Bernd Edlinger wrote:
>
>> Yes, I will update the patch accordingly.
>>
>> I agree, a statement like "if (x ? 0 : 2)" can be called suspicious as
>> well, even if the result of x is not ignored as in "if (x ? 1 : 2)".
>>
>> After I posted this patch, I extended the patch to cover also
>> suspicious int shift-left operations, at
>> https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00155.html
>> [PATCHv2] Add a warning for suspicious use of conditional expressions in
>> boolean context
>>
>> Therefore, I would like to rename the warning to -Wint-in-bool-context
>> so that the other patch can extend that instead of creating even more
>> warning names with much longer names.
>>
>>
>> Is renaming the warning OK as well, or could you have a look at the
>> follow-up patch please?
> Renaming is OK as well.
>

Hmm, well.  There is a problem.

If I change the warning to warn if either of the integer constants
is not [0|1] the boot-strap fails here:
./../gcc-trunk/gcc/gensupport.c: In function 'void 
compute_test_codes(rtx, file_location, char*)':
../../gcc-trunk/gcc/gensupport.c:202:25: error: ?: using integer 
constants in boolean context [-Werror=int-in-bool-context]
    ((a) == I ? ((b) == N ? N : I) :  \
                ~~~~~~~~~~^~~~~~~~
../../gcc-trunk/gcc/gensupport.c:209:5: note: in definition of macro 
'TRISTATE_OR'
     (a) || (b))
      ^
../../gcc-trunk/gcc/gensupport.c:256:26: note: in expansion of macro 
'TRISTATE_AND'
   codes[i] = TRISTATE_OR (TRISTATE_AND (op0_codes[i], op1_codes[i]),
                           ^~~~~~~~~~~~
../../gcc-trunk/gcc/gensupport.c:203:25: error: ?: using integer 
constants in boolean context [-Werror=int-in-bool-context]
     (b) == I ? ((a) == N ? N : I) :  \
                ~~~~~~~~~~^~~~~~~~
../../gcc-trunk/gcc/gensupport.c:209:5: note: in definition of macro 
'TRISTATE_OR'
     (a) || (b))
      ^
../../gcc-trunk/gcc/gensupport.c:256:26: note: in expansion of macro 
'TRISTATE_AND'
   codes[i] = TRISTATE_OR (TRISTATE_AND (op0_codes[i], op1_codes[i]),
                           ^~~~~~~~~~~~
../../gcc-trunk/gcc/gensupport.c:202:25: error: ?: using integer 
constants in boolean context [-Werror=int-in-bool-context]
    ((a) == I ? ((b) == N ? N : I) :  \
                ~~~~~~~~~~^~~~~~~~
../../gcc-trunk/gcc/gensupport.c:209:5: note: in definition of macro 
'TRISTATE_OR'
     (a) || (b))
      ^
../../gcc-trunk/gcc/gensupport.c:256:26: note: in expansion of macro 
'TRISTATE_AND'
   codes[i] = TRISTATE_OR (TRISTATE_AND (op0_codes[i], op1_codes[i]),
                           ^~~~~~~~~~~~
../../gcc-trunk/gcc/gensupport.c:203:25: error: ?: using integer 
constants in boolean context [-Werror=int-in-bool-context]
     (b) == I ? ((a) == N ? N : I) :  \
                ~~~~~~~~~~^~~~~~~~
../../gcc-trunk/gcc/gensupport.c:209:5: note: in definition of macro 
'TRISTATE_OR'
     (a) || (b))
      ^
../../gcc-trunk/gcc/gensupport.c:256:26: note: in expansion of macro 
'TRISTATE_AND'
   codes[i] = TRISTATE_OR (TRISTATE_AND (op0_codes[i], op1_codes[i]),
                           ^~~~~~~~~~~~
../../gcc-trunk/gcc/gensupport.c:202:25: error: ?: using integer 
constants in boolean context [-Werror=int-in-bool-context]
    ((a) == I ? ((b) == N ? N : I) :  \
                ~~~~~~~~~~^~~~~~~~
../../gcc-trunk/gcc/gensupport.c:209:12: note: in definition of macro 
'TRISTATE_OR'
     (a) || (b))
             ^
../../gcc-trunk/gcc/gensupport.c:257:5: note: in expansion of macro 
'TRISTATE_AND'
      TRISTATE_AND (TRISTATE_NOT (op0_codes[i]),
      ^~~~~~~~~~~~
../../gcc-trunk/gcc/gensupport.c:203:25: error: ?: using integer 
constants in boolean context [-Werror=int-in-bool-context]
     (b) == I ? ((a) == N ? N : I) :  \
                ~~~~~~~~~~^~~~~~~~
../../gcc-trunk/gcc/gensupport.c:209:12: note: in definition of macro 
'TRISTATE_OR'
     (a) || (b))
             ^
../../gcc-trunk/gcc/gensupport.c:257:5: note: in expansion of macro 
'TRISTATE_AND'
      TRISTATE_AND (TRISTATE_NOT (op0_codes[i]),
      ^~~~~~~~~~~~
../../gcc-trunk/gcc/gensupport.c:202:25: error: ?: using integer 
constants in boolean context [-Werror=int-in-bool-context]
    ((a) == I ? ((b) == N ? N : I) :  \
                ~~~~~~~~~~^~~~~~~~
../../gcc-trunk/gcc/gensupport.c:209:12: note: in definition of macro 
'TRISTATE_OR'
     (a) || (b))
             ^
../../gcc-trunk/gcc/gensupport.c:257:5: note: in expansion of macro 
'TRISTATE_AND'
      TRISTATE_AND (TRISTATE_NOT (op0_codes[i]),
      ^~~~~~~~~~~~
../../gcc-trunk/gcc/gensupport.c:203:25: error: ?: using integer 
constants in boolean context [-Werror=int-in-bool-context]
     (b) == I ? ((a) == N ? N : I) :  \
                ~~~~~~~~~~^~~~~~~~
../../gcc-trunk/gcc/gensupport.c:209:12: note: in definition of macro 
'TRISTATE_OR'
     (a) || (b))
             ^
../../gcc-trunk/gcc/gensupport.c:257:5: note: in expansion of macro 
'TRISTATE_AND'
      TRISTATE_AND (TRISTATE_NOT (op0_codes[i]),
      ^~~~~~~~~~~~
cc1plus: all warnings being treated as errors


I had not seen that with the un-symmetric version of the warning.
But that is of course possible because it warns on a few more
cases than before.

At first sight that code looks bogus.

But I think it could fix the warning:

Index: gensupport.c
===================================================================
--- gensupport.c	(revision 240097)
+++ gensupport.c	(working copy)
@@ -201,15 +201,15 @@
  #define TRISTATE_AND(a,b)			\
    ((a) == I ? ((b) == N ? N : I) :		\
     (b) == I ? ((a) == N ? N : I) :		\
-   (a) && (b))
+   (a) == Y && (b) == Y ? Y : N)

  #define TRISTATE_OR(a,b)			\
    ((a) == I ? ((b) == Y ? Y : I) :		\
     (b) == I ? ((a) == Y ? Y : I) :		\
-   (a) || (b))
+   (a) == Y || (b) == Y ? Y : N)

  #define TRISTATE_NOT(a)				\
-  ((a) == I ? I : !(a))
+  ((a) == I ? I : (a) == Y ? N : Y)

  /* 0 means no warning about that code yet, 1 means warned.  */
  static char did_you_mean_codes[NUM_RTX_CODE];

I think the warning is justified on code like that,
what do you think?


Bernd.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]