Re: [patch, fortran] Warning for feal / complex equality / inequality comparisons

2012-08-19 Thread Tobias Burnus

Thomas Koenig wrote:

the attached patch warns about comparisions for equality and inequality
of real and complex values if -Wcompare-reals is given. The new
compiler option is included in -Wall.

Regression-tested, tested with "make info" and "make dvi".
OK for trunk?


Thanks for the patch. It's okay, after fixing the nits below.


+ if (gfc_option.warn_compare_reals)
+   {
+ gfc_intrinsic_op op = e->value.op.op;
+   
+ if ((op1->ts.type == BT_REAL || op1->ts.type == BT_COMPLEX)
+ && (op == INTRINSIC_EQ || op == INTRINSIC_EQ_OS
+ || op == INTRINSIC_NE || op == INTRINSIC_NE_OS))
+   {
+ bool equality;
+
+ equality = op == INTRINSIC_EQ || op == INTRINSIC_EQ_OS;
+   
+ /* Type conversion has made sure that the types
+of op1 and op2 agree.  */
+ gfc_warning ("%s comparison for %s at %L",
+  equality ? "Equality" : "Inequality",
+  gfc_typename (&op1->ts), &op1->where);


Can you move up the comment before the "(op1->ts.type"? When reading the 
patch, I stumbled over that, before reading 8 lines later that checking 
op1 is sufficient.


Additionally, your gfc_warning is rather unfriendly to translators (try 
yourself to translate it, taking into account that the "Equality" string 
might be re-used elsewhere). I think it is better to have two separate 
strings, one for equality and one for inequality.


Tobias


[patch, fortran] Warning for feal / complex equality / inequality comparisons

2012-08-19 Thread Thomas Koenig

Hello world,

the attached patch warns about comparisions for equality and inequality
of real and complex values if -Wcompare-reals is given. The new
compiler option is included in -Wall.

Regression-tested, tested with "make info" and "make dvi".

OK for trunk?

Thomas

2012-08-19  Thomas König  

PR fortran/54298
* gfortran.h (struct gfc_option_t): Add warn_compare_reals.
* lang.opt:  Add Wcompare-reals.
* invoke.texi:  Document -Wcompare-reals.
* resolve.c (resolve_operator):  If -Wcompare-reals is in effect,
warn about equality/inequality comparisions for REAL and COMPLEX.
* options.c (gfc_init_options):  Set warn_compare_reals.
(set_Wall):  Include warn_compare_reals in Wall.
(gfc_handle_option):  Handle Wcompare_reals.

2012-08-19  Thomas König  

PR fortran/54298
* gfortran.dg/real_compare_1.f90:  New test case.
* gfortran.dg/bessel_5.f90  Add -Wno-compare-reals to options.
Index: testsuite/gfortran.dg/bessel_5.f90
===
--- testsuite/gfortran.dg/bessel_5.f90	(Revision 190442)
+++ testsuite/gfortran.dg/bessel_5.f90	(Arbeitskopie)
@@ -1,5 +1,5 @@
 ! { dg-do run }
-! { dg-options "-Wall -fno-range-check" }
+! { dg-options "-Wall -fno-range-check -Wno-compare-reals" }
 !
 ! PR fortran/36158 - Transformational BESSEL_JN/YN
 ! PR fortran/33197 - F2008 math functions
Index: fortran/gfortran.h
===
--- fortran/gfortran.h	(Revision 190442)
+++ fortran/gfortran.h	(Arbeitskopie)
@@ -2225,6 +2225,7 @@ typedef struct
   int warn_unused_dummy_argument;
   int warn_realloc_lhs;
   int warn_realloc_lhs_all;
+  int warn_compare_reals;
   int max_errors;
 
   int flag_all_intrinsics;
Index: fortran/lang.opt
===
--- fortran/lang.opt	(Revision 190442)
+++ fortran/lang.opt	(Arbeitskopie)
@@ -218,6 +218,10 @@ Wcharacter-truncation
 Fortran Warning
 Warn about truncated character expressions
 
+Wcompare-reals
+Fortran Warning
+Warn about equality comparisons involving REAL or COMPLEX expressions
+
 Wconversion
 Fortran Warning
 ; Documented in C
Index: fortran/invoke.texi
===
--- fortran/invoke.texi	(Revision 190442)
+++ fortran/invoke.texi	(Arbeitskopie)
@@ -726,10 +726,11 @@ warnings.
 @cindex warnings, all
 Enables commonly used warning options pertaining to usage that
 we recommend avoiding and that we believe are easy to avoid.
-This currently includes @option{-Waliasing}, @option{-Wampersand}, 
-@option{-Wconversion}, @option{-Wsurprising}, @option{-Wintrinsics-std},
-@option{-Wno-tabs}, @option{-Wintrinsic-shadow}, @option{-Wline-truncation},
-@option{-Wreal-q-constant} and @option{-Wunused}.
+This currently includes @option{-Waliasing}, @option{-Wampersand},
+@option{-Wconversion}, @option{-Wcompare-reals}, @option{-Wsurprising},
+@option{-Wintrinsics-std}, @option{-Wno-tabs}, @option{-Wintrinsic-shadow},
+@option{-Wline-truncation}, @option{-Wreal-q-constant} and
+@option{-Wunused}.
 
 @item -Waliasing
 @opindex @code{Waliasing}
@@ -935,6 +936,11 @@ a scalar.  See also @option{-frealloc-lhs}.
 Warn when the compiler inserts code to for allocation or reallocation of an
 allocatable variable; this includes scalars and derived types.
 
+@item -Wcompare-reals
+@opindex @code{Wcompare-reals}
+Warn when comparing real or complex types for equality or inequality.
+Enabled by @option{-Wall}.
+
 @item -Werror
 @opindex @code{Werror}
 @cindex warnings, to errors
Index: fortran/resolve.c
===
--- fortran/resolve.c	(Revision 190442)
+++ fortran/resolve.c	(Arbeitskopie)
@@ -4034,6 +4034,27 @@ resolve_operator (gfc_expr *e)
 
 	  e->ts.type = BT_LOGICAL;
 	  e->ts.kind = gfc_default_logical_kind;
+
+	  if (gfc_option.warn_compare_reals)
+	{
+	  gfc_intrinsic_op op = e->value.op.op;
+	  
+	  if ((op1->ts.type == BT_REAL || op1->ts.type == BT_COMPLEX)
+		  && (op == INTRINSIC_EQ || op == INTRINSIC_EQ_OS
+		  || op == INTRINSIC_NE || op == INTRINSIC_NE_OS))
+		{
+		  bool equality;
+
+		  equality = op == INTRINSIC_EQ || op == INTRINSIC_EQ_OS;
+		  
+		  /* Type conversion has made sure that the types
+		 of op1 and op2 agree.  */
+		  gfc_warning ("%s comparison for %s at %L",
+			   equality ? "Equality" : "Inequality",
+			   gfc_typename (&op1->ts), &op1->where);
+		}
+	}
+
 	  break;
 	}
 
Index: fortran/options.c
===
--- fortran/options.c	(Revision 190442)
+++ fortran/options.c	(Arbeitskopie)
@@ -113,6 +113,7 @@ gfc_init_options (unsigned int decoded_options_cou
   gfc_option.warn_unused_dummy_argument = 0;
   gfc_option.warn_realloc_lhs = 0;
   gfc_option.warn_realloc_lhs_all = 0;
+  gfc_option.warn_compare_rea