diff mbox

[fortran] PR 47359 - warnings for constant conversion

Message ID 556B7ED1.4020304@netcologne.de
State New
Headers show

Commit Message

Thomas Koenig May 31, 2015, 9:36 p.m. UTC
Hello world,

please find attached a rather lengthy patch, which attempts to deal
with warnings for conversions in Fortran once and for all (one can
hope ... :-)

The patch has two parts.

One part moves all the checking for conversion into the
gfc_<type>2<type> functions.  The checks are made much more
extensive; for example, integer conversions to real are also
checked because they can change the value.

There is one change in behavior:  Previosly, when a value
which did not change on a narrowing conversion was detected
with -Wconversion, it was not reported with -Wconversion-extra,
but there was a warning if only -Wconversion-extra was specified.

This patch changes that behavior so that -Wconversion and
-Wconversion-extra emit the same warning if the value is changed,
but that -Wconversion-extra, if specified alone, will emit
a warning anyway.

The second part catches the case when the user supplies more
digits than appropriate for the number.  More often than not,
this is a KIND error.

For the second part (which can also be committed separately)
I am not sure if the -Wconversion-extra flag is the right one.
I shied away from specifying YANF (Yet Another New Flag), but
I could do so if the consensus is that this is better.  It
might also be possible to invent another name for this option,
and maybe enable this with -Wsurprising.

Regression-tested.  Comments?  OK for trunk?

	Thomas

2015-05-31  Thomas Koenig  <tkoenig@netcologne.de>

        PR fortran/47359
        * arith.c (eval_intrinsic_op): Set warn flag for
        gfc_type_convert_binary if -Wconversion or -Wconversion-extra
        are set.
        (wprecision_real_real): New function.
        (wprecision_int_real): New function.
        (gfc_int2int): If -fno-range-check and -Wconversion are specified
        and it is a narrowing conversion, warn.
        (gfc_int2real): If there is a change in value for the conversion,
        warn.
        (gfc_int2complex):  Likewise.
        (gfc_real2int): If there is a fractional part to the real number,
        warn with -Wconversion, otherwise warn with -Wconversion-extra.
        (gfc_real2real): Emit warning if the constant was changed by
        conversion with either -Wconversion or -Wconversion-extra.  With
        -Wconversion-extra, warn if no warning was issued earlier.
        (gfc_real2complex):  Likewise.
        (gfc_complex2int): For -Wconversion or -Wconversion-extra, if
        there was an imaginary part, warn; otherwise, warn for change in
        value.  Warn with -Wconversion-extra if no other warning was
        issued.
        (gfc_complex2real): For -Wconversion or -Wconversion-extra, if
        there was an imaginary part, warn; otherwise, warn for change in
        value. Warn with -Wconversion-extra if no other warning was
        issued.
        (gfc_complex2complex):  For -Wconversion, warn if the value of
        either the real or the imaginary part was changed.  Warn for
        -Wconversion-extra if no prior warning was issued.
        * expr.c (gfc_check_assign):  Remove check for change in value.
        * primary.c (match_real_constant): For -Wconversion-extra, check
        against a number in which the last non-zero digit has been
        replaced with a zero.  If the number compares equal, warn.

2015-05-31  Thomas Koenig  <tkoenig@netcologne.de>

        PR fortran/47359
        * gfortran.dg/array_constructor_type_17.f03: Adjust error message.
        * gfortran.dg/warn_conversion.f90: Add warning for change in value
        for assignment.
        * gfortran.dg/warn_conversion_3.f90: Add warnings.
        * gfortran.dg/warn_conversion_5.f90: New test.
        * gfortran.dg/warn_conversion_6.f90: New test.
        * gfortran.dg/warn_conversion_7.f90: New test.

Comments

Steve Kargl May 31, 2015, 10:14 p.m. UTC | #1
On Sun, May 31, 2015 at 11:36:17PM +0200, Thomas Koenig wrote:
> 
> The second part catches the case when the user supplies more
> digits than appropriate for the number.  More often than not,
> this is a KIND error.
> 

Does the above issue warnings for

cat consts.def

  real(knd), parameter :: pi = 3.1415926535897932384626433832795029_knd

cat foo.f90

module foomod

  interface foo
     module procedure foo4, foo8, foo10, foo16
  end interface foo

  contains

  subroutine foo4()
  integer, parameter :: knd = 4
  include 'consts.def'
  ! do something with pi
  end subroutine

  subroutine foo8()
  integer, parameter :: knd = 8
  include 'consts.def'
  ! do something with pi
  end subroutine

  subroutine foo10()
  integer, parameter :: knd = 10
  include 'consts.def'
  ! do something with pi
  end subroutine

  subroutine foo16()
  integer, parameter :: knd = 16 
  include 'consts.def'
  ! do something with pi
  end subroutine foo16

end module foomod

If warnings occur, then IMHO the patch cannot be committed
in its current form.
Thomas Koenig June 1, 2015, 6:34 a.m. UTC | #2
Hi Steve,

>> The second part catches the case when the user supplies more
>> digits than appropriate for the number.  More often than not,
>> this is a KIND error.
>>
> 
> Does the above issue warnings for
> 
>   real(knd), parameter :: pi = 3.1415926535897932384626433832795029_knd
> 
> module foomod
> 
>   interface foo
>      module procedure foo4, foo8, foo10, foo16
>   end interface foo
> 
>   contains
> 
>   subroutine foo4()
>   integer, parameter :: knd = 4
>   include 'consts.def'
>   ! do something with pi
>   end subroutine 
> end module foomod

Yes, it does, if -Wconversion-extra is specified.

> If warnings occur, then IMHO the patch cannot be committed
> in its current form.

What would be the peferred alternative?

I can make this warning conditional on a new option, let's call it
-Wconversion-constant, and have this set by default by -Wsurprising
or by -Wconversion.  The new options machinery is nice enough to
specify the option name with the warning, so it is easy for a user
to turn this particular warning off.

It would also be possible to grade this warning for really fine-grained
control:

- Introduce -Wconversion-constant, enabled by -Wconversion, to warn
  about the most common error of a constant with excess precision
  without KIND, so print *,3.1415829535 would get a warning and
  print *,3.1415926535_4 would not.

- Introduce -Wconversion-constant-extra, enabled by -Wconversion-extra,
  to warn about any excess precision regardless whether a KIND number
  is present in the constant or not.

Comments?

	Thomas
Steve Kargl June 1, 2015, 1:40 p.m. UTC | #3
On Mon, Jun 01, 2015 at 08:34:24AM +0200, Thomas Koenig wrote:
> 
> >> The second part catches the case when the user supplies more
> >> digits than appropriate for the number.  More often than not,
> >> this is a KIND error.
> >>
> > 
> > Does the above issue warnings for
> > 
> >   real(knd), parameter :: pi = 3.1415926535897932384626433832795029_knd
> > 
> > module foomod
> > 
> >   interface foo
> >      module procedure foo4, foo8, foo10, foo16
> >   end interface foo
> > 
> >   contains
> > 
> >   subroutine foo4()
> >   integer, parameter :: knd = 4
> >   include 'consts.def'
> >   ! do something with pi
> >   end subroutine 
> > end module foomod
> 
> Yes, it does, if -Wconversion-extra is specified.
> 
> > If warnings occur, then IMHO the patch cannot be committed
> > in its current form.
> 
> What would be the peferred alternative?
> 

Is it possible to detect the _knd suffix?  If so, no
warning is my preference as it is never incorrect to
specify more digits than required for conversion from
ASCII to an internal representation.  This, of course,
assumes that the compiler doesn't have a bug.

> 
> Comments?
> 

I'm not a big fan of a proliferation of options.  As long
as the warning isn't triggered under -Wall, I suppose
I can live with -Wconversion-extra.
Thomas Koenig June 1, 2015, 6:01 p.m. UTC | #4
Am 01.06.2015 um 15:40 schrieb Steve Kargl:
> On Mon, Jun 01, 2015 at 08:34:24AM +0200, Thomas Koenig wrote:
>> What would be the peferred alternative?

> Is it possible to detect the _knd suffix?

Yes, this is possible.

> If so, no
> warning is my preference as it is never incorrect to
> specify more digits than required for conversion from
> ASCII to an internal representation.  This, of course,
> assumes that the compiler doesn't have a bug.

:-)

>> > 
>> > Comments?
>> > 
> I'm not a big fan of a proliferation of options.  As long
> as the warning isn't triggered under -Wall, I suppose
> I can live with -Wconversion-extra.

OK, so we have a few options.

a) Warn for

  print *,3.1415926535897932 with -Wconversion

  and warn for

  print *,3.1415926535_4 only with -Wconversion-extra

b) Like a) but supply two options to switch off the respective
    warnings.

c) Warn for

  print *,3.1415926535 with -Wconversoin-extra

  and don't warn for

  print *,3.141592653589_4

d) Like now: Warn with -Wconversion-extra for both

   print *,3.1415926535

   and

   print *,3.14159265358979_4

What are people's prefrences on this?  Should we maybe ask on c.l.f
(where we will get more opinions, certainly also differing)?

	Thomas
Steve Kargl June 1, 2015, 8:10 p.m. UTC | #5
On Mon, Jun 01, 2015 at 08:01:53PM +0200, Thomas Koenig wrote:
> Am 01.06.2015 um 15:40 schrieb Steve Kargl:
> > On Mon, Jun 01, 2015 at 08:34:24AM +0200, Thomas Koenig wrote:
> >> What would be the peferred alternative?
> 
> > Is it possible to detect the _knd suffix?
> 
> Yes, this is possible.
> 
> > If so, no
> > warning is my preference as it is never incorrect to
> > specify more digits than required for conversion from
> > ASCII to an internal representation.  This, of course,
> > assumes that the compiler doesn't have a bug.
> 
> :-)
> 
> >> > 
> >> > Comments?
> >> > 
> > I'm not a big fan of a proliferation of options.  As long
> > as the warning isn't triggered under -Wall, I suppose
> > I can live with -Wconversion-extra.
> 
> OK, so we have a few options.
> 
> a) Warn for
> 
>   print *,3.1415926535897932 with -Wconversion
> 
>   and warn for
> 
>   print *,3.1415926535_4 only with -Wconversion-extra
> 
> b) Like a) but supply two options to switch off the respective
>     warnings.
> 
> c) Warn for
> 
>   print *,3.1415926535 with -Wconversion-extra
> 
>   and don't warn for
> 
>   print *,3.141592653589_4
> 

This would be my first choice.  If a user actually specifies
a suffix, I assume that the user has given some thought 
to the preceding digits.

> d) Like now: Warn with -Wconversion-extra for both
> 
>    print *,3.1415926535
> 
>    and
> 
>    print *,3.14159265358979_4

This would be my second choice.

> What are people's prefrences on this?  Should we maybe ask on c.l.f
> (where we will get more opinions, certainly also differing)?

If you ask on c.l.f, you'll get differing opinions and most likely
a history lesson on what compilers did 40 years ago and how
PL/1 hands the issue. :-)

Don't let my person opinion be the sole driver/impediment.  
I do have a few comments on the patch itself.  I'll send those
later.
Thomas Koenig June 6, 2015, 10:22 a.m. UTC | #6
Am 01.06.2015 um 22:10 schrieb Steve Kargl:

>> c) Warn for
>>
>>   print *,3.1415926535 with -Wconversion-extra
>>
>>   and don't warn for
>>
>>   print *,3.141592653589_4
>>
> 
> This would be my first choice.  If a user actually specifies
> a suffix, I assume that the user has given some thought 
> to the preceding digits.


>> d) Like now: Warn with -Wconversion-extra for both
>>
>>    print *,3.1415926535
>>
>>    and
>>
>>    print *,3.14159265358979_4
> 
> This would be my second choice.

I would actually prefer d).  -Wconversion-extra is about warning
for things that are often correct.  I would like to have a chance
to warn about this kind of construct.

Any other comments?  OK to commit?

	Thomas
Steve Kargl June 6, 2015, 2:15 p.m. UTC | #7
On Sat, Jun 06, 2015 at 12:22:56PM +0200, Thomas Koenig wrote:
> 
> Any other comments?  OK to commit?
> 

No. Yes.
diff mbox

Patch

Index: fortran/arith.c
===================================================================
--- fortran/arith.c	(Revision 223876)
+++ fortran/arith.c	(Arbeitskopie)
@@ -1521,7 +1521,7 @@  eval_intrinsic (gfc_intrinsic_op op,
       temp.value.op.op1 = op1;
       temp.value.op.op2 = op2;
 
-      gfc_type_convert_binary (&temp, 0);
+      gfc_type_convert_binary (&temp, warn_conversion || warn_conversion_extra);
 
       if (op == INTRINSIC_EQ || op == INTRINSIC_NE
 	  || op == INTRINSIC_GE || op == INTRINSIC_GT
@@ -1949,7 +1949,43 @@  arith_error (arith rc, gfc_typespec *from, gfc_typ
      NaN, etc.  */
 }
 
+/* Returns true if significant bits were lost when converting real
+   constant r from from_kind to to_kind.  */
 
+static bool
+wprecision_real_real (mpfr_t r, int from_kind, int to_kind)
+{
+  mpfr_t rv, diff;
+  bool ret;
+
+  gfc_set_model_kind (to_kind);
+  mpfr_init (rv);
+  gfc_set_model_kind (from_kind);
+  mpfr_init (diff);
+
+  mpfr_set (rv, r, GFC_RND_MODE);
+  mpfr_sub (diff, rv, r, GFC_RND_MODE);
+
+  ret = ! mpfr_zero_p (diff);
+  mpfr_clear (rv);
+  mpfr_clear (diff);
+  return ret;
+}
+
+/* Return true if conversion from an integer to a real loses precision.  */
+
+static bool
+wprecision_int_real (mpz_t n, mpfr_t r)
+{
+  mpz_t i;
+  mpz_init (i);
+  mpfr_get_z (i, r, GFC_RND_MODE);
+  mpz_sub (i, i, n);
+  return mpz_cmp_si (i, 0) != 0;
+  mpz_clear (i);
+
+}
+
 /* Convert integers to integers.  */
 
 gfc_expr *
@@ -1985,8 +2021,12 @@  gfc_int2int (gfc_expr *src, int kind)
       k = gfc_validate_kind (BT_INTEGER, kind, false);
       gfc_convert_mpz_to_signed (result->value.integer,
 				 gfc_integer_kinds[k].bit_size);
+
+      if (warn_conversion && kind < src->ts.kind)
+	gfc_warning_now (OPT_Wconversion, "Conversion from %qs to %qs at %L",
+			 gfc_typename (&src->ts), gfc_typename (&result->ts),
+			 &src->where);
     }
-
   return result;
 }
 
@@ -2010,6 +2050,14 @@  gfc_int2real (gfc_expr *src, int kind)
       return NULL;
     }
 
+  if (warn_conversion
+      && wprecision_int_real (src->value.integer, result->value.real))
+    gfc_warning_now (OPT_Wconversion, "Change of value in conversion "
+		     "from %qs to %qs at %L",
+		     gfc_typename (&src->ts),
+		     gfc_typename (&result->ts),
+		     &src->where);
+
   return result;
 }
 
@@ -2034,6 +2082,15 @@  gfc_int2complex (gfc_expr *src, int kind)
       return NULL;
     }
 
+  if (warn_conversion
+      && wprecision_int_real (src->value.integer,
+			      mpc_realref (result->value.complex)))
+      gfc_warning_now (OPT_Wconversion, "Change of value in conversion "
+		       "from %qs to %qs at %L",
+		       gfc_typename (&src->ts),
+		       gfc_typename (&result->ts),
+		       &src->where);
+
   return result;
 }
 
@@ -2045,6 +2102,7 @@  gfc_real2int (gfc_expr *src, int kind)
 {
   gfc_expr *result;
   arith rc;
+  bool did_warn = false;
 
   result = gfc_get_constant_expr (BT_INTEGER, kind, &src->where);
 
@@ -2057,6 +2115,28 @@  gfc_real2int (gfc_expr *src, int kind)
       return NULL;
     }
 
+  /* If there was a fractional part, warn about this.  */
+
+  if (warn_conversion)
+    {
+      mpfr_t f;
+      mpfr_init (f);
+      mpfr_frac (f, src->value.real, GFC_RND_MODE);
+      if (mpfr_cmp_si (f, 0) != 0)
+	{
+	  gfc_warning_now (OPT_Wconversion, "Change of value in conversion "
+			   "from %qs to %qs at %L", gfc_typename (&src->ts),
+			   gfc_typename (&result->ts), &src->where);
+	  did_warn = true;
+	}
+    }
+  if (!did_warn && warn_conversion_extra)
+    {
+      gfc_warning_now (OPT_Wconversion_extra, "Conversion from %qs to %qs "
+		       "at %L", gfc_typename (&src->ts),
+		       gfc_typename (&result->ts), &src->where);
+    }
+
   return result;
 }
 
@@ -2068,6 +2148,7 @@  gfc_real2real (gfc_expr *src, int kind)
 {
   gfc_expr *result;
   arith rc;
+  bool did_warn = false;
 
   result = gfc_get_constant_expr (BT_REAL, kind, &src->where);
 
@@ -2088,6 +2169,33 @@  gfc_real2real (gfc_expr *src, int kind)
       return NULL;
     }
 
+  /* As a special bonus, don't warn about REAL values which are not changed by
+     the conversion if -Wconversion is specified and -Wconversion-extra is
+     not.  */
+
+  if ((warn_conversion || warn_conversion_extra) && src->ts.kind > kind)
+    {
+      int w = warn_conversion ? OPT_Wconversion : OPT_Wconversion_extra;
+      
+      /* Calculate the difference between the constant and the rounded
+	 value and check it against zero.  */
+
+      if (wprecision_real_real (src->value.real, src->ts.kind, kind))
+	{
+	  gfc_warning_now (w, "Change of value in conversion from "
+			   "%qs to %qs at %L",
+			   gfc_typename (&src->ts), gfc_typename (&result->ts),
+			   &src->where);
+	  /* Make sure the conversion warning is not emitted again.  */
+	  did_warn = true;
+	}
+    }
+
+    if (!did_warn && warn_conversion_extra)
+      gfc_warning_now (OPT_Wconversion_extra, "Conversion from %qs to %qs "
+		       "at %L", gfc_typename(&src->ts),
+		       gfc_typename(&result->ts), &src->where);
+
   return result;
 }
 
@@ -2099,6 +2207,7 @@  gfc_real2complex (gfc_expr *src, int kind)
 {
   gfc_expr *result;
   arith rc;
+  bool did_warn = false;
 
   result = gfc_get_constant_expr (BT_COMPLEX, kind, &src->where);
 
@@ -2119,6 +2228,26 @@  gfc_real2complex (gfc_expr *src, int kind)
       return NULL;
     }
 
+  if ((warn_conversion || warn_conversion_extra) && src->ts.kind > kind)
+    {
+      int w = warn_conversion ? OPT_Wconversion : OPT_Wconversion_extra;
+
+      if (wprecision_real_real (src->value.real, src->ts.kind, kind))
+	{
+	  gfc_warning_now (w, "Change of value in conversion from "
+			   "%qs to %qs at %L",
+			   gfc_typename (&src->ts), gfc_typename (&result->ts),
+			   &src->where);
+	  /* Make sure the conversion warning is not emitted again.  */
+	  did_warn = true;
+	}
+    }
+
+  if (!did_warn && warn_conversion_extra)
+    gfc_warning_now (OPT_Wconversion_extra, "Conversion from %qs to %qs "
+		     "at %L", gfc_typename(&src->ts),
+		     gfc_typename(&result->ts), &src->where);
+
   return result;
 }
 
@@ -2130,6 +2259,7 @@  gfc_complex2int (gfc_expr *src, int kind)
 {
   gfc_expr *result;
   arith rc;
+  bool did_warn = false;
 
   result = gfc_get_constant_expr (BT_INTEGER, kind, &src->where);
 
@@ -2143,6 +2273,43 @@  gfc_complex2int (gfc_expr *src, int kind)
       return NULL;
     }
 
+  if (warn_conversion || warn_conversion_extra)
+    {
+      int w = warn_conversion ? OPT_Wconversion : OPT_Wconversion_extra;
+
+      /* See if we discarded an imaginary part.  */
+      if (mpfr_cmp_si (mpc_imagref (src->value.complex), 0) != 0)
+	{
+	  gfc_warning_now (w, "Non-zero imaginary part discarded "
+			   "in conversion from %qs to %qs at %L",
+			   gfc_typename(&src->ts), gfc_typename (&result->ts),
+			   &src->where);
+	  did_warn = true;
+	}
+
+      else {
+	mpfr_t f;
+
+	mpfr_init (f);
+	mpfr_frac (f, src->value.real, GFC_RND_MODE);
+	if (mpfr_cmp_si (f, 0) != 0)
+	  {
+	    gfc_warning_now (w, "Change of value in conversion from "
+			     "%qs to %qs at %L", gfc_typename (&src->ts),
+			     gfc_typename (&result->ts), &src->where);
+	    did_warn = true;
+	  }
+	mpfr_clear (f);
+      }
+
+      if (!did_warn && warn_conversion_extra)
+	{
+	  gfc_warning_now (OPT_Wconversion_extra, "Conversion from %qs to %qs "
+			   "at %L", gfc_typename (&src->ts),
+			   gfc_typename (&result->ts), &src->where);
+	}
+    }
+
   return result;
 }
 
@@ -2154,6 +2321,7 @@  gfc_complex2real (gfc_expr *src, int kind)
 {
   gfc_expr *result;
   arith rc;
+  bool did_warn = false;
 
   result = gfc_get_constant_expr (BT_REAL, kind, &src->where);
 
@@ -2174,6 +2342,41 @@  gfc_complex2real (gfc_expr *src, int kind)
       return NULL;
     }
 
+  if (warn_conversion || warn_conversion_extra)
+    {
+      int w = warn_conversion ? OPT_Wconversion : OPT_Wconversion_extra;
+
+      /* See if we discarded an imaginary part.  */
+      if (mpfr_cmp_si (mpc_imagref (src->value.complex), 0) != 0)
+	{
+	  gfc_warning_now (w, "Non-zero imaginary part discarded "
+			   "in conversion from %qs to %qs at %L",
+			   gfc_typename(&src->ts), gfc_typename (&result->ts),
+			   &src->where);
+	  did_warn = true;
+	}
+
+      /* Calculate the difference between the real constant and the rounded
+	 value and check it against zero.  */
+      
+      if (kind > src->ts.kind
+	  && wprecision_real_real (mpc_realref (src->value.complex),
+				   src->ts.kind, kind))
+	{
+	  gfc_warning_now (w, "Change of value in conversion from "
+			   "%qs to %qs at %L",
+			   gfc_typename (&src->ts), gfc_typename (&result->ts),
+			   &src->where);
+	  /* Make sure the conversion warning is not emitted again.  */
+	  did_warn = true;
+	}
+    }
+
+  if (!did_warn && warn_conversion_extra)
+    gfc_warning_now (OPT_Wconversion, "Conversion from %qs to %qs at %L",
+		     gfc_typename(&src->ts), gfc_typename (&result->ts),
+		     &src->where);
+
   return result;
 }
 
@@ -2185,6 +2388,7 @@  gfc_complex2complex (gfc_expr *src, int kind)
 {
   gfc_expr *result;
   arith rc;
+  bool did_warn = false;
 
   result = gfc_get_constant_expr (BT_COMPLEX, kind, &src->where);
 
@@ -2220,6 +2424,26 @@  gfc_complex2complex (gfc_expr *src, int kind)
       return NULL;
     }
 
+  if ((warn_conversion || warn_conversion_extra) && src->ts.kind > kind
+      && (wprecision_real_real (mpc_realref (src->value.complex),
+				src->ts.kind, kind)
+	  || wprecision_real_real (mpc_imagref (src->value.complex),
+				   src->ts.kind, kind)))
+    {
+      int w = warn_conversion ? OPT_Wconversion : OPT_Wconversion_extra;
+
+      gfc_warning_now (w, "Change of value in conversion from "
+		       " %qs to %qs at %L",
+		       gfc_typename (&src->ts), gfc_typename (&result->ts),
+		       &src->where);
+      did_warn = true;
+    }
+
+  if (!did_warn && warn_conversion_extra && src->ts.kind != kind)
+    gfc_warning_now (OPT_Wconversion_extra, "Conversion from %qs to %qs "
+		     "at %L", gfc_typename(&src->ts),
+		     gfc_typename (&result->ts), &src->where);
+
   return result;
 }
 
Index: fortran/expr.c
===================================================================
--- fortran/expr.c	(Revision 223876)
+++ fortran/expr.c	(Arbeitskopie)
@@ -3247,55 +3247,6 @@  gfc_check_assign (gfc_expr *lvalue, gfc_expr *rval
 	}
     }
 
-  /*  Warn about type-changing conversions for REAL or COMPLEX constants.
-      If lvalue and rvalue are mixed REAL and complex, gfc_compare_types
-      will warn anyway, so there is no need to to so here.  */
-
-  if (rvalue->expr_type == EXPR_CONSTANT && lvalue->ts.type == rvalue->ts.type
-      && (lvalue->ts.type == BT_REAL || lvalue->ts.type == BT_COMPLEX))
-    {
-      if (lvalue->ts.kind < rvalue->ts.kind && 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 (OPT_Wconversion, 
-			     "Change of value in conversion from "
-			     " %qs to %qs at %L", gfc_typename (&rvalue->ts),
-			     gfc_typename (&lvalue->ts), &rvalue->where);
-
-	      mpfr_clear (rv);
-	      mpfr_clear (diff);
-	    }
-	  else
-	    gfc_warning (OPT_Wconversion,
-			 "Possible change of value in conversion from %qs "
-			 "to %qs at %L", gfc_typename (&rvalue->ts),
-			 gfc_typename (&lvalue->ts), &rvalue->where);
-
-	}
-      else if (warn_conversion_extra && lvalue->ts.kind > rvalue->ts.kind)
-	{
-	  gfc_warning (OPT_Wconversion_extra,
-		       "Conversion from %qs to %qs at %L",
-		       gfc_typename (&rvalue->ts),
-		       gfc_typename (&lvalue->ts), &rvalue->where);
-	}
-    }
-
   if (gfc_compare_types (&lvalue->ts, &rvalue->ts))
     return true;
 
Index: fortran/intrinsic.c
===================================================================
--- fortran/intrinsic.c	(Revision 223876)
+++ fortran/intrinsic.c	(Arbeitskopie)
@@ -4695,15 +4695,18 @@  gfc_convert_type_warn (gfc_expr *expr, gfc_typespe
 	  /* 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 (warn_conversion && from_ts.kind > ts->kind)
-	    gfc_warning_now (OPT_Wconversion, "Possible change of value in "
-			     "conversion from %s to %s at %L",
-			     gfc_typename (&from_ts), gfc_typename (ts),
-			     &expr->where);
-	  else if (warn_conversion_extra)
-	    gfc_warning_now (OPT_Wconversion_extra, "Conversion from %s to %s "
-			     "at %L", gfc_typename (&from_ts),
-			     gfc_typename (ts), &expr->where);
+	  if (expr->expr_type != EXPR_CONSTANT)
+	    {
+	      if (warn_conversion && from_ts.kind > ts->kind)
+		gfc_warning_now (OPT_Wconversion, "Possible change of value in "
+				 "conversion from %s to %s at %L",
+				 gfc_typename (&from_ts), gfc_typename (ts),
+				 &expr->where);
+	      else if (warn_conversion_extra)
+		gfc_warning_now (OPT_Wconversion_extra, "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)
@@ -4711,7 +4714,7 @@  gfc_convert_type_warn (gfc_expr *expr, gfc_typespe
 	{
 	  /* Conversion from REAL/COMPLEX to INTEGER or COMPLEX to REAL
 	     usually comes with a loss of information, regardless of kinds.  */
-	  if (warn_conversion)
+	  if (warn_conversion && expr->expr_type != EXPR_CONSTANT)
 	    gfc_warning_now (OPT_Wconversion, "Possible change of value in "
 			     "conversion from %s to %s at %L",
 			     gfc_typename (&from_ts), gfc_typename (ts),
Index: fortran/primary.c
===================================================================
--- fortran/primary.c	(Revision 223876)
+++ fortran/primary.c	(Arbeitskopie)
@@ -736,6 +736,58 @@  done:
       gfc_internal_error ("gfc_range_check() returned bad value");
     }
 
+  /* Warn about trailing digits which suggest the user added too many
+     trailing digits, which may cause the appearance of higher pecision
+     than the kind kan support.
+
+     This is done by replacing the rightmost non-zero digit with zero
+     and comparing with the original value.  If these are equal, we
+     assume the user supplied more digits than intended (or forgot to
+     convert to the correct kind).
+  */
+
+  if (warn_conversion_extra)
+    {
+      mpfr_t r;
+      char *c, *p;
+      bool did_break;
+
+      c = strchr (buffer, 'e');
+      if (c == NULL)
+	c = buffer + strlen(buffer);
+
+      did_break = false;
+      for (p = c - 1; p >= buffer; p--)
+	{
+	  if (*p == '.')
+	    continue;
+	  
+	  if (*p != '0')
+	    {
+	      *p = '0';
+	      did_break = true;
+	      break;
+	    }
+	}
+
+      if (did_break)
+	{
+	  mpfr_init (r);
+	  mpfr_set_str (r, buffer, 10, GFC_RND_MODE);
+	  if (negate)
+	    mpfr_neg (r, r, GFC_RND_MODE);
+
+	  mpfr_sub (r, r, e->value.real, GFC_RND_MODE);
+
+	  if (mpfr_cmp_ui (r, 0) == 0)
+	    gfc_warning (OPT_Wconversion_extra, "Non-significant digits "
+			 "in %qs number at %C, maybe incorrect KIND",
+			 gfc_typename (&e->ts));
+
+	  mpfr_clear (r);
+	}
+    }
+
   *result = e;
   return MATCH_YES;
 
Index: testsuite/gfortran.dg/array_constructor_type_17.f03
===================================================================
--- testsuite/gfortran.dg/array_constructor_type_17.f03	(Revision 223876)
+++ testsuite/gfortran.dg/array_constructor_type_17.f03	(Arbeitskopie)
@@ -8,5 +8,5 @@  PROGRAM test
   IMPLICIT NONE
 
   INTEGER(KIND=4) :: arr(1)
-  arr = (/ INTEGER(KIND=4) :: HUGE(0_8) /) ! { dg-warning "conversion from" }
+  arr = (/ INTEGER(KIND=4) :: HUGE(0_8) /) ! { dg-warning "Conversion" }
 END PROGRAM test
Index: testsuite/gfortran.dg/warn_conversion.f90
===================================================================
--- testsuite/gfortran.dg/warn_conversion.f90	(Revision 223876)
+++ testsuite/gfortran.dg/warn_conversion.f90	(Arbeitskopie)
@@ -18,7 +18,7 @@  SUBROUTINE pr27866c4
   integer(kind=4) :: i4
   i4 = 2.3              ! { dg-warning "conversion" }
   i1 = 500              ! { dg-error "overflow" }
-  a = 2**26-1           ! assignment INTEGER(4) to REAL(4) - no warning
+  a = 2**26-1           ! { dg-warning "Change of value in conversion" }
   b = 1d999             ! { dg-error "overflow" }
 
   a = i4                ! assignment INTEGER(4) to REAL(4) - no warning
Index: testsuite/gfortran.dg/warn_conversion_3.f90
===================================================================
--- testsuite/gfortran.dg/warn_conversion_3.f90	(Revision 223876)
+++ testsuite/gfortran.dg/warn_conversion_3.f90	(Arbeitskopie)
@@ -7,8 +7,8 @@  program main
   complex(8), parameter :: z = cmplx (0.5, 0.5)  ! { dg-warning "Conversion" }
   real :: r1, r2
   r1 = 2.3d0 ! { dg-warning "Change of value in conversion" }
-  r2 = 2.5d0 ! No warning because the value does not change
+  r2 = 2.5d0 ! { dg-warning "Conversion" }
   d1 = .13 ! { dg-warning "Conversion" }
   d2 = .13d0
-  d1 = z     ! { dg-warning "change of value in conversion" }
+  d1 = z     ! { dg-warning "Non-zero imaginary part" }
 end program main