[PING] PR 21438 : Warning about division by zero depends on lexical form

[PING] PR 21438 : Warning about division by zero depends on lexical form

[Manuel López-Ibáñez]

The discussion didn't reach a definitive conclusion:
http://gcc.gnu.org/ml/gcc-patches/2007-01/msg01166.html

It seems that the consensus is that:

* Context matters, so we should warn for int i = int(1.0/0.0); (not
sure how to implement this, but I will try).

* Lexical form should not matter, so warnings for 1.0/0.0 and 1.0/0
should be consistent.

* C and C++ front-ends should agree.

So the only question is:

Do we warn for 1.0/0.0 (like C++ front-end does) or we consider that
it is a legitimate way to obtain infinities so we don't warn (like C
front-end does) ?

Thanks,

Manuel.

[PING^2] PR 21438 : Warning about division by zero depends on lexical form

[Manuel López-Ibáñez]

Still pending resolution.

Thanks,

Manuel.

On 10/02/07, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> The discussion didn't reach a definitive conclusion:
> http://gcc.gnu.org/ml/gcc-patches/2007-01/msg01166.html
>
> It seems that the consensus is that:
>
> * Context matters, so we should warn for int i = int(1.0/0.0); (not
> sure how to implement this, but I will try).
>
> * Lexical form should not matter, so warnings for 1.0/0.0 and 1.0/0
> should be consistent.
>
> * C and C++ front-ends should agree.
>
> So the only question is:
>
> Do we warn for 1.0/0.0 (like C++ front-end does) or we consider that
> it is a legitimate way to obtain infinities so we don't warn (like C
> front-end does) ?
>
> Thanks,
>
> Manuel.
>

[PING^3] PR 21438 : Warning about division by zero depends on lexical form

[Manuel López-Ibáñez]

Still pending resolution:
http://gcc.gnu.org/ml/gcc-patches/2007-02/msg00925.html

Thanks,

Manuel.

Re: [PING^3] PR 21438 : Warning about division by zero depends on lexical form

[Joseph S. Myers]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 791 bytes --]

On Thu, 1 Mar 2007, Manuel López-Ibáñez wrote:

> Still pending resolution:
> http://gcc.gnu.org/ml/gcc-patches/2007-02/msg00925.html

I don't see what there is supposed to be here for me to resolve.  If the 
patch at <http://gcc.gnu.org/ml/gcc-patches/2007-01/msg01166.html> is 
still awaiting approval rather than withdrawn or revised it is OK.

The int i = int(1.0/0.0); is a C++ matter, but is not a matter of a 
division-by-zero warning; rather a matter that we should warn for 
conversion of any constant infinity or NaN to an integer type, whether 
that infinity or NaN is obtained by division by zero or by using built-in 
functions.  This should go with other warnings for out-of-range 
floating-to-integer conversions.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PING^3] PR 21438 : Warning about division by zero depends on lexical form

[Manuel López-Ibáñez]

On 01/03/07, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Thu, 1 Mar 2007, Manuel López-Ibáñez wrote:
>
> > Still pending resolution:
> > http://gcc.gnu.org/ml/gcc-patches/2007-02/msg00925.html
>
> I don't see what there is supposed to be here for me to resolve.  If the
> patch at <http://gcc.gnu.org/ml/gcc-patches/2007-01/msg01166.html> is
> still awaiting approval rather than withdrawn or revised it is OK.
>

That is what I wanted to hear. Thanks.

> The int i = int(1.0/0.0); is a C++ matter, but is not a matter of a
> division-by-zero warning; rather a matter that we should warn for
> conversion of any constant infinity or NaN to an integer type, whether
> that infinity or NaN is obtained by division by zero or by using built-in
> functions.  This should go with other warnings for out-of-range
> floating-to-integer conversions.

Hum, that is a valid point. But ignoring that particular example for a
moment, the C front-end does not warn for 1.0/0.0 and with my patch
neither for 1.0/0 while the C++ front-end warns for both. So my
question is whether I should prepare a patch for the C++ front-end to
warn for both. But, I guess that, yes, that is not for you to decide,
so I should wait for a C++ front-end maintainer to speak clearly.

I was just trying to solve the inconsistency before making it worse
with the above patch. I guess things don't work that way.

Cheers,

Manuel.

Re: [PING^3] PR 21438 : Warning about division by zero depends on lexical form

[Mark Mitchell]

Joseph S. Myers wrote:

> The int i = int(1.0/0.0); is a C++ matter, but is not a matter of a 
> division-by-zero warning; rather a matter that we should warn for 
> conversion of any constant infinity or NaN to an integer type, whether 
> that infinity or NaN is obtained by division by zero or by using built-in 
> functions.  This should go with other warnings for out-of-range 
> floating-to-integer conversions.

I don't know that I entirely agree that we should not warn about
1.0/0.0.  In particular, if doing that division would generate a SIGFPE
on the target, were it not folded, then it seems reasonable to warn
about it.

However, what I do agree about is that the C and C++ front-ends should
agree.  Also, as a C++ maintainer, I am happy for the C++ front-end to
be modified to match the C front-end; that consistency is more important
to me than whether or not we warn.  So, I would look favorably on a
patch to factor out whatever logic is used in C and share it with C++.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [PING^3] PR 21438 : Warning about division by zero depends on lexical form

[Manuel López-Ibáñez]

On 02/03/07, Mark Mitchell <mark@codesourcery.com> wrote:
> Joseph S. Myers wrote:
>
> I don't know that I entirely agree that we should not warn about
> 1.0/0.0.  In particular, if doing that division would generate a SIGFPE
> on the target, were it not folded, then it seems reasonable to warn
> about it.
>

I think that if it generates a SIGFPE, then it is not a legitimate way
to generate infinities and NaNs.

Can I detect that from the front-end? How?

Cheers,

Manuel.

Re: [PING^3] PR 21438 : Warning about division by zero depends on lexical form

[Mark Mitchell]

Manuel López-Ibáñez wrote:
> On 02/03/07, Mark Mitchell <mark@codesourcery.com> wrote:
>> Joseph S. Myers wrote:
>>
>> I don't know that I entirely agree that we should not warn about
>> 1.0/0.0.  In particular, if doing that division would generate a SIGFPE
>> on the target, were it not folded, then it seems reasonable to warn
>> about it.
>> 
> I think that if it generates a SIGFPE, then it is not a legitimate way
> to generate infinities and NaNs.

The problem is that whether or not it generates SIGFPE may depend on the
dynamic state of the running program.  In particular, there are usually
functions that you can call that will enable/disable getting SIGFPE for
these kinds of operations.  So, you can't tell at compile-time.

The point of my statement is that it seems odd to me not to warn about
an operation that might trap if it were not expressed with constants.
While writing "1.0/0.0" is more sensible that writing "1/0", it's only a
question of degree, and I'd imagine that some programmers might want a
warning.

But, the first thing to do is to make the C++ front end behave like the
C front end.  Then, we can decide what, if anything to do next.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [PING^3] PR 21438 : Warning about division by zero depends on lexical form

[Andrew Haley]

Manuel López-Ibáñez writes:
 > On 02/03/07, Mark Mitchell <mark@codesourcery.com> wrote:
 > > Joseph S. Myers wrote:
 > >
 > > I don't know that I entirely agree that we should not warn about
 > > 1.0/0.0.  In particular, if doing that division would generate a SIGFPE
 > > on the target, were it not folded, then it seems reasonable to warn
 > > about it.
 > >
 > 
 > I think that if it generates a SIGFPE, then it is not a legitimate way
 > to generate infinities and NaNs.
 > 
 > Can I detect that from the front-end? How?

I doubt it.  Is a target that generates SIGFPE from 1.0/0.0
IEEE-754(etc) compliant?  I would not have thought so, but it can be
hard to be certain.

Andrew.

Re: [PING^3] PR 21438 : Warning about division by zero depends on lexical form

[Manuel López-Ibáñez]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

On 02/03/07, Mark Mitchell <mark@codesourcery.com> wrote:
>
> But, the first thing to do is to make the C++ front end behave like the
> C front end.  Then, we can decide what, if anything to do next.
>

Done. Bootstrapped and regression tested. Funny enough, no C++
testcase needs adjustment.

:ADDPATCH C/C++:

2007-03-12  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>

  PR 21438
  * c-common.h (warn_for_div_by_zero): Declare.
  * c-common.c (warn_for_div_by_zero): Define.
  * c-typeck.c (build_binary_op): Call warn_for_div_zero instead of warning.

cp/
  * typeck.c (build_binary_op): Call warn_for_div_zero instead of warning.

[-- Attachment #2: wdivbyzero-try3.diff --]
[-- Type: text/plain, Size: 4153 bytes --]

Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c	(revision 122755)
+++ gcc/cp/typeck.c	(working copy)
@@ -3130,14 +3130,11 @@ build_binary_op (enum tree_code code, tr
 	  && (code1 == INTEGER_TYPE || code1 == REAL_TYPE
 	      || code1 == COMPLEX_TYPE || code1 == VECTOR_TYPE))
 	{
 	  enum tree_code tcode0 = code0, tcode1 = code1;
 
-	  if (TREE_CODE (op1) == INTEGER_CST && integer_zerop (op1))
-	    warning (OPT_Wdiv_by_zero, "division by zero in %<%E / 0%>", op0);
-	  else if (TREE_CODE (op1) == REAL_CST && real_zerop (op1))
-	    warning (OPT_Wdiv_by_zero, "division by zero in %<%E / 0.%>", op0);
+	  warn_for_div_by_zero (op0, op1);
 
 	  if (tcode0 == COMPLEX_TYPE || tcode0 == VECTOR_TYPE)
 	    tcode0 = TREE_CODE (TREE_TYPE (TREE_TYPE (op0)));
 	  if (tcode1 == COMPLEX_TYPE || tcode1 == VECTOR_TYPE)
 	    tcode1 = TREE_CODE (TREE_TYPE (TREE_TYPE (op1)));
@@ -3167,14 +3164,11 @@ build_binary_op (enum tree_code code, tr
 	shorten = -1;
       break;
 
     case TRUNC_MOD_EXPR:
     case FLOOR_MOD_EXPR:
-      if (code1 == INTEGER_TYPE && integer_zerop (op1))
-	warning (OPT_Wdiv_by_zero, "division by zero in %<%E %% 0%>", op0);
-      else if (code1 == REAL_TYPE && real_zerop (op1))
-	warning (OPT_Wdiv_by_zero, "division by zero in %<%E %% 0.%>", op0);
+      warn_for_div_by_zero (op0, op1);
 
       if (code0 == INTEGER_TYPE && code1 == INTEGER_TYPE)
 	{
 	  /* Although it would be tempting to shorten always here, that loses
 	     on some targets, since the modulo instruction is undefined if the
Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c	(revision 122755)
+++ gcc/c-typeck.c	(working copy)
@@ -7846,14 +7846,11 @@ build_binary_op (enum tree_code code, tr
     case TRUNC_DIV_EXPR:
     case CEIL_DIV_EXPR:
     case FLOOR_DIV_EXPR:
     case ROUND_DIV_EXPR:
     case EXACT_DIV_EXPR:
-      /* Floating point division by zero is a legitimate way to obtain
-	 infinities and NaNs.  */
-      if (skip_evaluation == 0 && integer_zerop (op1))
-	warning (OPT_Wdiv_by_zero, "division by zero");
+      warn_for_div_by_zero (op0, op1);
 
       if ((code0 == INTEGER_TYPE || code0 == REAL_TYPE
 	   || code0 == COMPLEX_TYPE || code0 == VECTOR_TYPE)
 	  && (code1 == INTEGER_TYPE || code1 == REAL_TYPE
 	      || code1 == COMPLEX_TYPE || code1 == VECTOR_TYPE))
@@ -7889,12 +7886,11 @@ build_binary_op (enum tree_code code, tr
 	common = 1;
       break;
 
     case TRUNC_MOD_EXPR:
     case FLOOR_MOD_EXPR:
-      if (skip_evaluation == 0 && integer_zerop (op1))
-	warning (OPT_Wdiv_by_zero, "division by zero");
+      warn_for_div_by_zero (op0, op1);
 
       if (code0 == INTEGER_TYPE && code1 == INTEGER_TYPE)
 	{
 	  /* Although it would be tempting to shorten always here, that loses
 	     on some targets, since the modulo instruction is undefined if the
Index: gcc/c-common.c
===================================================================
--- gcc/c-common.c	(revision 122755)
+++ gcc/c-common.c	(working copy)
@@ -7032,6 +7032,16 @@ c_build_cdtor_fns (void)
 
 #ifndef TARGET_HAS_TARGETCM
 struct gcc_targetcm targetcm = TARGETCM_INITIALIZER;
 #endif
 
+/* Warn for division by zero.  */
+void
+warn_for_div_by_zero (tree ARG_UNUSED (op0), tree op1)
+{
+  /* Floating point division by zero is a legitimate way to obtain
+     infinities and NaNs.  */
+  if (skip_evaluation == 0 && integer_zerop (op1))
+    warning (OPT_Wdiv_by_zero, "division by zero");
+}
+
 #include "gt-c-common.h"
Index: gcc/c-common.h
===================================================================
--- gcc/c-common.h	(revision 122755)
+++ gcc/c-common.h	(working copy)
@@ -869,10 +869,11 @@ extern tree builtin_type_for_size (int, 
 
 extern void warn_array_subscript_with_type_char (tree);
 extern void warn_about_parentheses (enum tree_code, enum tree_code,
 				    enum tree_code);
 extern void warn_for_unused_label (tree label);
+extern void warn_for_div_by_zero (tree op0, tree op1);
 
 
 /* In c-gimplify.c  */
 extern void c_genericize (tree);
 extern int c_gimplify_expr (tree *, tree *, tree *);

Re: [PING^3] PR 21438 : Warning about division by zero depends on lexical form

[Mark Mitchell]

Manuel López-Ibáñez wrote:

> 2007-03-12  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
> 
>  PR 21438
>  * c-common.h (warn_for_div_by_zero): Declare.
>  * c-common.c (warn_for_div_by_zero): Define.
>  * c-typeck.c (build_binary_op): Call warn_for_div_zero instead of warning.
> 
> cp/
>  * typeck.c (build_binary_op): Call warn_for_div_zero instead of warning.

> +void
> +warn_for_div_by_zero (tree ARG_UNUSED (op0), tree op1)

Leave out op0 until/if you actually need it for something.  Rename OP1
to DIVISOR.  Make the comment mention DIVISOR by name, and document the
logic behind not warning for REAL_CSTs, like so:

/* If DIVISOR is zero, and has integral type, issue a warning about
division by zero.  Do not issue a warning if DIVISOR has a
floating-point type, since we consider 0.0/0.0 a valid way of generating
a NaN.  */

OK with those changes.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713