Patchwork [fortran] PR 46659 - extend conversion warnings to assignments

login
register
mail settings
Submitter Thomas Koenig
Date Aug. 14, 2011, 3:36 p.m.
Message ID <4E47EB84.6080006@netcologne.de>
Download mbox | patch
Permalink /patch/109960/
State New
Headers show

Comments

Thomas Koenig - Aug. 14, 2011, 3:36 p.m.
Hello world,

the attached patch extends conversion warnings to assignments.

OK for trunk?

	Thomas

011-08-14  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/46659
         * expr.c (gfc_check_assign): Check for type conversions when the
         right-hand side is a constant REAL cor COMPLEX contstant the
         left-hand side is also REAL or COMPLEX.  Don't warn when a
         narrowing conversion does not change the value of the constant.

2011-08-14  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/46659
         * gfortran.dg/warn_conversion_2.f90:  Also warn about conversion
         of a constant resulting from simplification.
         * gfortran.dg/warn_conversion_3.f90:  New test.
! { dg-do compile }
! { dg-options "-Wconversion -Wconversion-extra" }
! PR 47659 - warning about conversions on assignment
! Based on a test case by Thomas Henlich
program main
  double precision d1, d2
  real :: r1, r2
  r1 = 2.3d0 ! { dg-warning "Change of value in conversion" }
  r2 = 2.5d0 ! No warning because the value does not change
  d1 = .13 ! { dg-warning "Conversion" }
  d2 = .13d0
end program main
Thomas Koenig - Aug. 14, 2011, 3:39 p.m.
I wrote:
> Hello world,
>
> the attached patch extends conversion warnings to assignments.
>
> OK for trunk?

... and forgot to say that this has been regression-tested.

	Thomas
Tobias Burnus - Aug. 14, 2011, 8:54 p.m.
Thomas Koenig wrote:
> the attached patch extends conversion warnings to assignments.
> OK for trunk?

I get now two warnings for:

complex(8), parameter :: z = cmplx (0.5, 0.5)
r = z
end


Untested: Instead of checking

   if (rvalue->expr_type == EXPR_CONSTANT
&& (lvalue->ts.type == BT_REAL || lvalue->ts.type == BT_COMPLEX)
&& (rvalue->ts.type == BT_REAL || rvalue->ts.type == BT_COMPLEX))

one might check for:

   if (rvalue->expr_type == EXPR_CONSTANT
&& (lvalue->ts.type == BT_REAL || lvalue->ts.type == BT_COMPLEX)
&& (rvalue->ts.type ==rvalue->ts.type)

Tobias
Thomas Koenig - Aug. 15, 2011, 5:49 p.m.
Am 14.08.2011 22:54, schrieb Tobias Burnus:
> Thomas Koenig wrote:
>> the attached patch extends conversion warnings to assignments.
>> OK for trunk?
>
> I get now two warnings for:
>
> complex(8), parameter :: z = cmplx (0.5, 0.5)
> r = z
> end

The problem is that gfc_check_assign is called twice for this, from
different code paths:


Breakpoint 1, gfc_check_assign (lvalue=0x7fffffffd8d0, rvalue=0x15bb020, 
conform=1)
     at ../../trunk/gcc/fortran/expr.c:3045 

3045    { 

(gdb) bt 

#0  gfc_check_assign (lvalue=0x7fffffffd8d0, rvalue=0x15bb020, 
conform=1)
     at ../../trunk/gcc/fortran/expr.c:3045 

#1  0x00000000004f70bb in gfc_check_assign_symbol (sym=0x15bb680, 
rvalue=0x15bb020)
     at ../../trunk/gcc/fortran/expr.c:3649 

#2  0x00000000004e0670 in add_init_expr_to_sym (name=Unhandled dwarf 
expression opcode 0xf3
) at ../../trunk/gcc/fortran/decl.c:1346 

#3  0x00000000004e87d0 in variable_decl (elem=<value optimized out>) 

     at ../../trunk/gcc/fortran/decl.c:2047 

#4  gfc_match_data_decl (elem=<value optimized out>) at 
../../trunk/gcc/fortran/decl.c:4124
#5  0x000000000052cb8a in match_word (subr=Unhandled dwarf expression 
opcode 0xf3
) at ../../trunk/gcc/fortran/parse.c:65 

#6  0x000000000052d3a0 in decode_statement () at 
../../trunk/gcc/fortran/parse.c:283
#7  decode_statement () at ../../trunk/gcc/fortran/parse.c:232 

#8  0x000000000052ea05 in next_free () at 
../../trunk/gcc/fortran/parse.c:731
#9  next_statement () at ../../trunk/gcc/fortran/parse.c:923 

#10 0x00000000005320de in gfc_parse_file () at 
../../trunk/gcc/fortran/parse.c:4382
#11 0x000000000056b486 in gfc_be_parse_file () at 
../../trunk/gcc/fortran/f95-lang.c:250
#12 0x00000000008fa978 in compile_file () at ../../trunk/gcc/toplev.c:548
#13 do_compile () at ../../trunk/gcc/toplev.c:1886
#14 toplev_main () at ../../trunk/gcc/toplev.c:1962
#15 0x00007ffff6f8aa7d in __libc_start_main () from /lib64/libc.so.6
#16 0x00000000004cade9 in _start () at ../sysdeps/x86_64/elf/start.S:113
(gdb) c
Continuing.

Breakpoint 1, gfc_check_assign (lvalue=0x7fffffffd9e0, rvalue=0x15bb020, 
conform=1)
     at ../../trunk/gcc/fortran/expr.c:3045
3045    {
(gdb) bt
#0  gfc_check_assign (lvalue=0x7fffffffd9e0, rvalue=0x15bb020, conform=1)
     at ../../trunk/gcc/fortran/expr.c:3045
#1  0x00000000004f70bb in gfc_check_assign_symbol (sym=0x15bb680, 
rvalue=0x15bb020)
     at ../../trunk/gcc/fortran/expr.c:3649
#2  0x000000000055a697 in traverse_ns (st=Unhandled dwarf expression 
opcode 0xf3
) at ../../trunk/gcc/fortran/symbol.c:3333
#3  0x000000000055fd8c in gfc_traverse_ns (ns=0x15ba590,
     func=0x548200 <resolve_values(gfc_symbol*)>) at 
../../trunk/gcc/fortran/symbol.c:3349
#4  0x00000000005475e1 in resolve_types (ns=0x15ba590) at 
../../trunk/gcc/fortran/resolve.c:13511
#5  0x000000000053c4e4 in gfc_resolve (ns=0x15ba590) at 
../../trunk/gcc/fortran/resolve.c:13593
#6  gfc_resolve (ns=0x15ba590) at ../../trunk/gcc/fortran/resolve.c:13581
#7  0x00000000005325cb in resolve_all_program_units 
(gfc_global_ns_list=<value optimized out>)
     at ../../trunk/gcc/fortran/parse.c:4247
#8  gfc_parse_file (gfc_global_ns_list=<value optimized out>)
     at ../../trunk/gcc/fortran/parse.c:4513
#9  0x000000000056b486 in gfc_be_parse_file () at 
../../trunk/gcc/fortran/f95-lang.c:250
#10 0x00000000008fa978 in compile_file () at ../../trunk/gcc/toplev.c:548
#11 do_compile () at ../../trunk/gcc/toplev.c:1886
#12 toplev_main () at ../../trunk/gcc/toplev.c:1962
#13 0x00007ffff6f8aa7d in __libc_start_main () from /lib64/libc.so.6
#14 0x00000000004cade9 in _start () at ../sysdeps/x86_64/elf/start.S:113


What's the best way here?  Add an "already checked" flag?  I don't want
to disable the test for complex.
Tobias Burnus - Aug. 17, 2011, 5:53 p.m.
On 08/15/2011 07:49 PM, Thomas Koenig wrote:
> Am 14.08.2011 22:54, schrieb Tobias Burnus:
>> Thomas Koenig wrote:
>>> the attached patch extends conversion warnings to assignments.
>>> OK for trunk?
>>
>> I get now two warnings for:
>>
>> complex(8), parameter :: z = cmplx (0.5, 0.5)
>> r = z
>> end
>
> The problem is that gfc_check_assign is called twice for this, from
> different code paths

Well, I don't think that this is the problem - the actual warning comes 
from the calling of the same code path:

a) Call to gfc_check_assign_symbol from add_init_expr_to_sym: This does 
not print a warning for "z = cmplx(0.5,0.5)"  (Side note: There is a 
conversion from kind=4 to kind=8)

b) Call from gfc_resolve for "r = z"
b.1) gfc_warning call in gfc_check_assign at expr.c:3228  -- the code 
which you added
b.2) gfc_warning_now call in gfc_convert_type_warn (intrinsic.c:4349), 
which is called from gfc_check_assign (line 3272).

(b.2) is guarded by
       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))

while (b.1) uses
   if (rvalue->expr_type == EXPR_CONSTANT
&& (lvalue->ts.type == BT_REAL || lvalue->ts.type == BT_COMPLEX)
&& (rvalue->ts.type == BT_REAL || rvalue->ts.type == BT_COMPLEX))

As written, you should consider changing this to, e.g.,

   if (rvalue->expr_type == EXPR_CONSTANT
&& (lvalue->ts.type == BT_REAL || lvalue->ts.type == BT_COMPLEX)
&& (rvalue->ts.type == rvalue->ts.type))

or at least you should exclude rvalue == BT_COMPLX and lvalue == BT_REAL.


Tobias

Patch

Index: fortran/expr.c
===================================================================
--- fortran/expr.c	(Revision 177746)
+++ fortran/expr.c	(Arbeitskopie)
@@ -3190,6 +3190,53 @@  gfc_check_assign (gfc_expr *lvalue, gfc_expr *rval
 	}
     }
 
+  /*  If lvalue is REAL and rvalue is a REAL constant with different,
+      warn about possible errors the user may have done during conversion.  */
+
+  if (rvalue->expr_type == EXPR_CONSTANT
+      && (lvalue->ts.type == BT_REAL || lvalue->ts.type == BT_COMPLEX)
+      && (rvalue->ts.type == BT_REAL || rvalue->ts.type == BT_COMPLEX))
+    {
+      if (lvalue->ts.kind < rvalue->ts.kind && gfc_option.gfc_warn_conversion)
+	{
+	  /* As a special bonus, don't warn about REAL rvalues which are not
+	     changed by the conversion if -Wconversion is specified.  */
+	  if (rvalue->ts.type == BT_REAL && mpfr_number_p (rvalue->value.real))
+	    {
+	      /* Calculate the difference between the constant and the rounded
+		 value and check it against zero.  */
+	      mpfr_t rv, diff;
+	      gfc_set_model_kind (lvalue->ts.kind);
+	      mpfr_init (rv);
+	      gfc_set_model_kind (rvalue->ts.kind);
+	      mpfr_init (diff);
+	      
+	      mpfr_set (rv, rvalue->value.real, GFC_RND_MODE);
+	      mpfr_sub (diff, rv, rvalue->value.real, GFC_RND_MODE);
+	  
+	      if (!mpfr_zero_p (diff))
+		gfc_warning ("Change of value in conversion from "
+			     " %s to %s at %L", gfc_typename (&rvalue->ts),
+			     gfc_typename (&lvalue->ts), &rvalue->where);
+	      
+	      mpfr_clear (rv);
+	      mpfr_clear (diff);
+	    }
+	  else
+	    gfc_warning ("Possible change of value in conversion from %s "
+			 "to %s at %L",gfc_typename (&rvalue->ts),
+			 gfc_typename (&lvalue->ts), &rvalue->where);
+
+	}
+      else if (gfc_option.warn_conversion_extra
+	       && lvalue->ts.kind > rvalue->ts.kind)
+	{
+	  gfc_warning ("Conversion from %s to %s at %L",
+		       gfc_typename (&rvalue->ts), gfc_typename (&lvalue->ts),
+		       &rvalue->where);
+	}
+    }
+
   if (gfc_compare_types (&lvalue->ts, &rvalue->ts))
     return SUCCESS;
 
Index: testsuite/gfortran.dg/warn_conversion_2.f90
===================================================================
--- testsuite/gfortran.dg/warn_conversion_2.f90	(Revision 177746)
+++ testsuite/gfortran.dg/warn_conversion_2.f90	(Arbeitskopie)
@@ -7,5 +7,5 @@ 
   x = 2.0
   sqrt2 = sqrt(x)      ! { dg-warning "Conversion" }
 
-  sqrt2 = sqrt(2.0)    ! no warning; simplified to a constant and range checked
+  sqrt2 = sqrt(2.0)    ! { dg-warning "Conversion" }
 end