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]

[patch, fortran] Further improve -Wconversion


Hi all.

After the changes with -Wconversion, I found a source file that generates 
about 50.000 lines of conversion warnings if compiled with -Wall. A tad 
excessive.

Attached patch includes the changes required for same type range checking, in 
particular it avoids warnings for constructs like

      integer*2 n
      data n / 42 /

but generates range errors for

      integer*2 n
      data n / 42424242 /

With -fno-range-check but -Wconversion, the warning "conversion from 
INTEGER(4) to INTEGER(2)" is given for both.

Tobias, as the one who requested -Wconversion-extra, please double-check the 
changes I made for gfortran.dg/warn_conversion_2.f90 :)


gcc/fortran/:
2010-05-19  Daniel Franke  <franke.daniel@gmail.com>

	* intrinsic.c (gfc_convert_type_warn): Further improve checking if
	a warning is required.

gcc/testsuite/:
2010-05-19  Daniel Franke  <franke.daniel@gmail.com>

	* gfortran.dg/warn_conversion.f90: Removed check for redundant
	warning.
	* gfortran.dg/warn_conversion_2.f90: Use non-constant expression to
	check for warning.


Regression tested on i686-pc-linux-gnu.

Ok for trunk?

	Daniel
Index: fortran/intrinsic.c
===================================================================
--- fortran/intrinsic.c	(revision 159586)
+++ fortran/intrinsic.c	(working copy)
@@ -4022,58 +4022,67 @@ gfc_convert_type_warn (gfc_expr *expr, g
     }
   else if (wflag)
     {
-      /* Two modes of warning:
-	  - gfc_option.warn_conversion tries to be more intelligent
-	    about the warnings raised and omits those where smaller
-	    kinds are promoted to larger ones without change in the
-	    value
-	  - gfc_option.warn_conversion_extra does not take the kinds
-	    into account and also warns for coversions like
-	    REAL(4) -> REAL(8)
-
-	 NOTE: Possible enhancement for warn_conversion
-	 If converting from a smaller to a larger kind, check if the
-	 value is constant and if yes, whether the value still fits
-	 in the smaller kind. If yes, omit the warning.
-      */
-
-      /* If the types are the same (but not LOGICAL), and if from-kind
-	 is larger than to-kind, this may indicate a loss of precision.
-	 The same holds for conversions from REAL to COMPLEX.  */
-      if (((from_ts.type == ts->type && from_ts.type != BT_LOGICAL)
-           && ((gfc_option.warn_conversion && from_ts.kind > ts->kind)
-	       || gfc_option.warn_conversion_extra))
-	  || ((from_ts.type == BT_REAL && ts->type == BT_COMPLEX)
-	      && ((gfc_option.warn_conversion && from_ts.kind > ts->kind)
-		  || gfc_option.warn_conversion_extra)))
-	gfc_warning_now ("Possible change of value in conversion "
-			 "from %s to %s at %L", gfc_typename (&from_ts),
-			 gfc_typename (ts), &expr->where);
-
-      /* If INTEGER is converted to REAL/COMPLEX, this is generally ok if
-	 the kind of the INTEGER value is less or equal to the kind of the
-	 REAL/COMPLEX one. Otherwise the value may not fit.
-	 Assignment of an overly large integer constant also generates
-	 an overflow error with range checking. */
-      else if (from_ts.type == BT_INTEGER
-	       && (ts->type == BT_REAL || ts->type == BT_COMPLEX)
-	       && ((gfc_option.warn_conversion && from_ts.kind > ts->kind)
-		   || gfc_option.warn_conversion_extra))
-	gfc_warning_now ("Possible change of value in conversion "
-			 "from %s to %s at %L", gfc_typename (&from_ts),
-			 gfc_typename (ts), &expr->where);
-
-      /* If REAL/COMPLEX is converted to INTEGER, or COMPLEX is converted
-        to REAL we almost certainly have a loss of digits, regardless of
-        the respective kinds.  */
-      else if ((((from_ts.type == BT_REAL || from_ts.type == BT_COMPLEX)
-		  && ts->type == BT_INTEGER)
-		|| (from_ts.type == BT_COMPLEX && ts->type == BT_REAL))
-	       && (gfc_option.warn_conversion
-	           || gfc_option.warn_conversion_extra))
-	gfc_warning_now ("Possible change of value in conversion from "
-			"%s to %s at %L", gfc_typename (&from_ts),
-			gfc_typename (ts), &expr->where);
+      if (gfc_option.flag_range_check
+	  && expr->expr_type == EXPR_CONSTANT
+	  && from_ts.type == ts->type)
+	{
+	  /* Do nothing. Constants of the same type are range-checked
+	     elsewhere. If a value too large for the target type is
+	     assigned, an error is generated. Not checking here avoids
+	     duplications of warnings/errors.
+	     If range checking was disabled, but -Wconversion enabled,
+	     a non range checked warning is generated below.  */
+	}
+      else if (from_ts.type == BT_LOGICAL || ts->type == BT_LOGICAL)
+	{
+	  /* Do nothing. This block exists only to simplify the other
+	     else-if expressions.
+	       LOGICAL <> LOGICAL    no warning, independent of kind values
+	       LOGICAL <> INTEGER    extension, warned elsewhere
+	       LOGICAL <> REAL       invalid, error generated elsewhere
+	       LOGICAL <> COMPLEX    invalid, error generated elsewhere  */
+	}
+      else if (from_ts.type == ts->type
+	       || (from_ts.type == BT_INTEGER && ts->type == BT_REAL)
+	       || (from_ts.type == BT_INTEGER && ts->type == BT_COMPLEX)
+	       || (from_ts.type == BT_REAL && ts->type == BT_COMPLEX))
+	{
+	  /* Larger kinds can hold values of smaller kinds without problems.
+	     Hence, only warn if target kind is smaller than the source
+	     kind - or if -Wconversion-extra is specified.  */
+	  if (gfc_option.warn_conversion_extra)
+	    gfc_warning_now ("Conversion from %s to %s at %L",
+			     gfc_typename (&from_ts), gfc_typename (ts),
+			     &expr->where);
+	  else if (gfc_option.warn_conversion
+		   && from_ts.kind > ts->kind)
+	    gfc_warning_now ("Possible change of value in conversion "
+			     "from %s to %s at %L", gfc_typename (&from_ts),
+			     gfc_typename (ts), &expr->where);
+	}
+      else if ((from_ts.type == BT_REAL && ts->type == BT_INTEGER)
+	       || (from_ts.type == BT_COMPLEX && ts->type == BT_INTEGER)
+	       || (from_ts.type == BT_COMPLEX && ts->type == BT_REAL))
+	{
+	  /* Conversion from REAL/COMPLEX to INTEGER or COMPLEX to REAL
+	     usually comes with a loss of information, regardless of kinds.  */
+	  if (gfc_option.warn_conversion_extra
+	      || gfc_option.warn_conversion)
+	    gfc_warning_now ("Possible change of value in conversion "
+			     "from %s to %s at %L", gfc_typename (&from_ts),
+			     gfc_typename (ts), &expr->where);
+	}
+      else if (from_ts.type == BT_HOLLERITH || ts->type == BT_HOLLERITH)
+	{
+	  /* If HOLLERITH is involved, all bets are off.  */
+	  if (gfc_option.warn_conversion_extra
+	      || gfc_option.warn_conversion)
+	    gfc_warning_now ("Conversion from %s to %s at %L",
+			     gfc_typename (&from_ts), gfc_typename (ts),
+			     &expr->where);
+	}
+      else
+        gcc_unreachable ();
     }
 
   /* Insert a pre-resolved function call to the right function.  */
Index: testsuite/gfortran.dg/warn_conversion.f90
===================================================================
--- testsuite/gfortran.dg/warn_conversion.f90	(revision 159549)
+++ testsuite/gfortran.dg/warn_conversion.f90	(working copy)
@@ -18,7 +18,6 @@ SUBROUTINE pr27866c4
   integer(kind=4) :: i4
   i4 = 2.3              ! { dg-warning "conversion" }
   i1 = 500              ! { dg-error "overflow" }
-                        ! { dg-warning "conversion" "" { target *-*-* } 20 }
   a = 2**26-1           ! assignment INTEGER(4) to REAL(4) - no warning
   b = 1d999             ! { dg-error "overflow" }
 
Index: testsuite/gfortran.dg/warn_conversion_2.f90
===================================================================
--- testsuite/gfortran.dg/warn_conversion_2.f90	(revision 159586)
+++ testsuite/gfortran.dg/warn_conversion_2.f90	(working copy)
@@ -2,5 +2,10 @@
 ! { dg-options "-Wconversion-extra" }
 
   real(8) :: sqrt2
-  sqrt2 = sqrt(2.0)      ! { dg-warning "conversion" }
+  real x
+
+  x = 2.0
+  sqrt2 = sqrt(x)      ! { dg-warning "Conversion" }
+
+  sqrt2 = sqrt(2.0)    ! no warning; simplified to a constant and range checked
 end

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