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 to split out new warning flag for floating point conversion


== Administrivia ==

This is my first patch.  I have emailed in the signed copyright transfer
documents already.

== Description ==

This patch is a fix for Bug 53001

As required by the C and C++ standards, gcc automatically converts
floating point numbers to lower precision or integer values.  Silently
converting these values is a problem for numerical programs.  GCC
already has a flag -Wconversion which does warn about these conversions,
but -Wconversion also warns about integer conversion which means for
many programs the number of warnings will be large.

This patch adds a -Wfloat-conversion that only warns on float
conversions.  Here are three examples that are warned by this new flag:

int main(int argc, char ** argv) {
  int i = 3.14;
  return i;
}

int foo(double x)
{
  return x;
}

float foo2(double x) {
  return x;
}

The -Wfloat-conversion is enabled by both -Wconversion (since it is a
subset) and -Wextra (as suggested on the Bug 53001 discussion)

Because this changes -Wextra, when compiling with -Werror and -Wextra,
some code will not compile now.  The code in gcc that this occurred in
was changed to use explicit casts.  The patch would be shorter if
-Wextra did not enable -Wfloat-conversion, and if that is desired I can
change the patch.

Because this patch enables it for -Wextra, gcc/c-family/c-cppbuiltin.c,
gcc/mcf.c, gcc/predict.c, gcc/real.c and libcpp/symtab.c need to be
changed, otherwise they would be unmodified.

I am not certain that c.opt was modified correctly.

== Testcases ==

This patch has passes the existing -Wconversion testcases.
gcc/testsuite/c-c++-common/Wconversion-real.c and other testcases
possibly could be modified.

== Changelog ==

2013-10-08  Joshua Cogliati <jrincayc@yahoo.com>

        Splitting out a -Wfloat-conversion from -Wconversion for
        conversions that lower floating point number precision
        or conversion from floating point numbers to integers
        * gcc/c-family/c-common.c Switching unsafe_conversion_p to
        return an enumeration with more detail, and conversion_warning
        to use this information.
        * gcc/c-family/c-common.h Adding conversion_safety enumeration
	and switching return type of unsafe_conversion_p
        * gcc/c-family/c-cppbuiltin.c Making explicit casts
        * gcc/c-family/c.opt Adding new warning float-conversion and
        enabling it with -Wextra and -Wconversion
        * gcc/doc/invoke.texi Adding documentation about
        -Wfloat-conversion
        * gcc/mcf.c Making explicit casts
        * gcc/predict.c Making explicit casts
        * gcc/real.c Making explicit casts
        * libcpp/symtab.c Avoiding float to int conversion

== Bootstrapping and testing ==

Tested bootstrap on x86_64-unknown-linux-gnu for
--enable-languages=c,c++,fortran,java,lto,objc with trunk on r203051 and
r203112


Thank you for consideration of this patch.

Joshua Cogliati
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(revision 203112)
+++ gcc/c-family/c-common.c	(working copy)
@@ -2517,10 +2517,10 @@ shorten_binary_op (tree result_type, tre
    Function allows conversions between types of different signedness and
    does not return true in that case.  Function can produce signedness
    warnings if PRODUCE_WARNS is true.  */
-bool
+enum conversion_safety
 unsafe_conversion_p (tree type, tree expr, bool produce_warns)
 {
-  bool give_warning = false;
+  enum conversion_safety give_warning = SAFE_CONVERSION; /* is 0 or false */
   tree expr_type = TREE_TYPE (expr);
   location_t loc = EXPR_LOC_OR_HERE (expr);
 
@@ -2532,7 +2532,7 @@ unsafe_conversion_p (tree type, tree exp
 	  && TREE_CODE (type) == INTEGER_TYPE)
 	{
 	  if (!real_isinteger (TREE_REAL_CST_PTR (expr), TYPE_MODE (expr_type)))
-	    give_warning = true;
+	    give_warning = UNSAFE_REAL;
 	}
       /* Warn for an integer constant that does not fit into integer type.  */
       else if (TREE_CODE (expr_type) == INTEGER_TYPE
@@ -2553,7 +2553,7 @@ unsafe_conversion_p (tree type, tree exp
 			    " constant value to negative integer");
 	    }
 	  else
-	    give_warning = true;
+	    give_warning = UNSAFE_OTHER;
 	}
       else if (TREE_CODE (type) == REAL_TYPE)
 	{
@@ -2562,7 +2562,7 @@ unsafe_conversion_p (tree type, tree exp
 	    {
 	      REAL_VALUE_TYPE a = real_value_from_int_cst (0, expr);
 	      if (!exact_real_truncate (TYPE_MODE (type), &a))
-		give_warning = true;
+		give_warning = UNSAFE_REAL;
 	    }
 	  /* Warn for a real constant that does not fit into a smaller
 	     real type.  */
@@ -2571,7 +2571,7 @@ unsafe_conversion_p (tree type, tree exp
 	    {
 	      REAL_VALUE_TYPE a = TREE_REAL_CST (expr);
 	      if (!exact_real_truncate (TYPE_MODE (type), &a))
-		give_warning = true;
+		give_warning = UNSAFE_REAL;
 	    }
 	}
     }
@@ -2580,7 +2580,7 @@ unsafe_conversion_p (tree type, tree exp
       /* Warn for real types converted to integer types.  */
       if (TREE_CODE (expr_type) == REAL_TYPE
 	  && TREE_CODE (type) == INTEGER_TYPE)
-	give_warning = true;
+	give_warning = UNSAFE_REAL;
 
       else if (TREE_CODE (expr_type) == INTEGER_TYPE
 	       && TREE_CODE (type) == INTEGER_TYPE)
@@ -2618,7 +2618,7 @@ unsafe_conversion_p (tree type, tree exp
 			  && int_fits_type_p (op1, c_common_signed_type (type))
 			  && int_fits_type_p (op1,
 					      c_common_unsigned_type (type))))
-		    return false;
+		    return SAFE_CONVERSION;
 		  /* If constant is unsigned and fits in the target
 		     type, then the result will also fit.  */
 		  else if ((TREE_CODE (op0) == INTEGER_CST
@@ -2627,12 +2627,12 @@ unsafe_conversion_p (tree type, tree exp
 			   || (TREE_CODE (op1) == INTEGER_CST
 			       && unsigned1
 			       && int_fits_type_p (op1, type)))
-		    return false;
+		    return SAFE_CONVERSION;
 		}
 	    }
 	  /* Warn for integer types converted to smaller integer types.  */
 	  if (TYPE_PRECISION (type) < TYPE_PRECISION (expr_type))
-	    give_warning = true;
+	    give_warning = UNSAFE_OTHER;
 
 	  /* When they are the same width but different signedness,
 	     then the value may change.  */
@@ -2668,14 +2668,14 @@ unsafe_conversion_p (tree type, tree exp
 
 	  if (!exact_real_truncate (TYPE_MODE (type), &real_low_bound)
 	      || !exact_real_truncate (TYPE_MODE (type), &real_high_bound))
-	    give_warning = true;
+	    give_warning = UNSAFE_OTHER;
 	}
 
       /* Warn for real types converted to smaller real types.  */
       else if (TREE_CODE (expr_type) == REAL_TYPE
 	       && TREE_CODE (type) == REAL_TYPE
 	       && TYPE_PRECISION (type) < TYPE_PRECISION (expr_type))
-	give_warning = true;
+	give_warning = UNSAFE_REAL;
     }
 
   return give_warning;
@@ -2689,8 +2689,9 @@ conversion_warning (tree type, tree expr
 {
   tree expr_type = TREE_TYPE (expr);
   location_t loc = EXPR_LOC_OR_HERE (expr);
+  enum conversion_safety conversion_kind;
 
-  if (!warn_conversion && !warn_sign_conversion)
+  if (!warn_conversion && !warn_sign_conversion && !warn_float_conversion)
     return;
 
   switch (TREE_CODE (expr))
@@ -2717,10 +2718,19 @@ conversion_warning (tree type, tree expr
 
     case REAL_CST:
     case INTEGER_CST:
-      if (unsafe_conversion_p (type, expr, true))
-	warning_at (loc, OPT_Wconversion,
-		    "conversion to %qT alters %qT constant value",
-		    type, expr_type);
+      conversion_kind = unsafe_conversion_p (type, expr, true);
+      if(conversion_kind == UNSAFE_REAL) 
+	{
+	  warning_at (loc, OPT_Wfloat_conversion,
+		      "conversion to %qT alters %qT constant value",
+		      type, expr_type);
+	} 
+      else if(conversion_kind)
+	{
+	  warning_at (loc, OPT_Wconversion,
+		      "conversion to %qT alters %qT constant value",
+		      type, expr_type);
+	}
       return;
 
     case COND_EXPR:
@@ -2736,10 +2746,19 @@ conversion_warning (tree type, tree expr
       }
 
     default: /* 'expr' is not a constant.  */
-      if (unsafe_conversion_p (type, expr, true))
-	warning_at (loc, OPT_Wconversion,
-		    "conversion to %qT from %qT may alter its value",
-		    type, expr_type);
+      conversion_kind = unsafe_conversion_p (type, expr, true);
+      if(conversion_kind == UNSAFE_REAL)
+	{
+	  warning_at (loc, OPT_Wfloat_conversion,
+		      "conversion to %qT from %qT may alter its value",
+		      type, expr_type);
+	} 
+      else if(conversion_kind)
+	{
+	  warning_at (loc, OPT_Wconversion,
+		      "conversion to %qT from %qT may alter its value",
+		      type, expr_type);
+	}
     }
 }
 
Index: gcc/c-family/c-common.h
===================================================================
--- gcc/c-family/c-common.h	(revision 203112)
+++ gcc/c-family/c-common.h	(working copy)
@@ -685,6 +685,16 @@ struct visibility_flags
   unsigned inlines_hidden : 1;	/* True when -finlineshidden in effect.  */
 };
 
+/* These variables are possible types of unsafe conversions. 
+   SAFE_CONVERSION The conversion is safe
+   UNSAFE_OTHER Another type of conversion with problems
+   UNSAFE_SIGN Conversion between signed and unsigned integers 
+    which are all warned about immediately, so this is unused
+   UNSAFE_REAL Conversions that reduce the precision of reals 
+    including conversions from reals to integers
+ */
+enum conversion_safety { SAFE_CONVERSION = 0, UNSAFE_OTHER, UNSAFE_SIGN, UNSAFE_REAL };
+
 /* Global visibility options.  */
 extern struct visibility_flags visibility_options;
 
@@ -738,7 +748,7 @@ extern tree c_common_signed_type (tree);
 extern tree c_common_signed_or_unsigned_type (int, tree);
 extern void c_common_init_ts (void);
 extern tree c_build_bitfield_integer_type (unsigned HOST_WIDE_INT, int);
-extern bool unsafe_conversion_p (tree, tree, bool);
+extern enum conversion_safety unsafe_conversion_p (tree, tree, bool);
 extern bool decl_with_nonnull_addr_p (const_tree);
 extern tree c_fully_fold (tree, bool, bool *);
 extern tree decl_constant_value_for_optimization (tree);
Index: gcc/c-family/c-cppbuiltin.c
===================================================================
--- gcc/c-family/c-cppbuiltin.c	(revision 203112)
+++ gcc/c-family/c-cppbuiltin.c	(working copy)
@@ -157,7 +157,7 @@ builtin_define_float_constants (const ch
 	p log10 b			if b is a power of 10
 	floor((p - 1) log10 b)		otherwise
   */
-  dig = (fmt->p - 1) * log10_b;
+  dig = (int)((fmt->p - 1) * log10_b);
   sprintf (name, "__%s_DIG__", name_prefix);
   builtin_define_with_int_value (name, dig);
 
@@ -173,7 +173,7 @@ builtin_define_float_constants (const ch
 
      Recall that emin is negative, so the integer truncation calculates
      the ceiling, not the floor, in this case.  */
-  min_10_exp = (fmt->emin - 1) * log10_b;
+  min_10_exp = (int)((fmt->emin - 1) * log10_b);
   sprintf (name, "__%s_MIN_10_EXP__", name_prefix);
   sprintf (buf, "(%d)", min_10_exp);
   builtin_define_with_value (name, buf, 0);
@@ -208,7 +208,7 @@ builtin_define_float_constants (const ch
      Hand-waving aside, crunching all of the sets of constants above by hand
      does not yield a case for which the first term is significant, which
      in the end is all that matters.  */
-  max_10_exp = fmt->emax * log10_b;
+  max_10_exp = (int)(fmt->emax * log10_b);
   sprintf (name, "__%s_MAX_10_EXP__", name_prefix);
   builtin_define_with_int_value (name, max_10_exp);
 
@@ -224,14 +224,14 @@ builtin_define_float_constants (const ch
   {
     double d_decimal_dig
       = 1 + (fmt->p < ldfmt->p ? ldfmt->p : fmt->p) * log10_b;
-    decimal_dig = d_decimal_dig;
+    decimal_dig = (int)d_decimal_dig;
     if (decimal_dig < d_decimal_dig)
       decimal_dig++;
   }
   /* Similar, for this type rather than long double.  */
   {
     double type_d_decimal_dig = 1 + fmt->p * log10_b;
-    type_decimal_dig = type_d_decimal_dig;
+    type_decimal_dig = (int)type_d_decimal_dig;
     if (type_decimal_dig < type_d_decimal_dig)
       type_decimal_dig++;
   }
Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 203112)
+++ gcc/c-family/c.opt	(working copy)
@@ -387,6 +387,10 @@ Werror-implicit-function-declaration
 C ObjC RejectNegative Warning Alias(Werror=, implicit-function-declaration)
 This switch is deprecated; use -Werror=implicit-function-declaration instead
 
+Wfloat-conversion
+C ObjC C++ ObjC++ Var(warn_float_conversion) LangEnabledBy(C ObjC C++ ObjC++,Wconversion) EnabledBy(Wextra)
+Warn for implicit type conversions that cause loss of floating point precision
+
 Wfloat-equal
 C ObjC C++ ObjC++ Var(warn_float_equal) Warning
 Warn if testing floating point numbers for equality
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 203112)
+++ gcc/doc/invoke.texi	(working copy)
@@ -263,7 +263,8 @@ Objective-C and Objective-C++ Dialects}.
 -Wpointer-arith  -Wno-pointer-to-int-cast @gol
 -Wredundant-decls  -Wno-return-local-addr @gol
 -Wreturn-type  -Wsequence-point  -Wshadow @gol
--Wsign-compare  -Wsign-conversion  -Wsizeof-pointer-memaccess @gol
+-Wsign-compare  -Wsign-conversion -Wfloat-conversion @gol 
+-Wsizeof-pointer-memaccess @gol
 -Wstack-protector -Wstack-usage=@var{len} -Wstrict-aliasing @gol
 -Wstrict-aliasing=n @gol -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
 -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol
@@ -3351,6 +3352,7 @@ name is still supported, but the newer n
 -Wuninitialized  @gol
 -Wunused-parameter @r{(only with} @option{-Wunused} @r{or} @option{-Wall}@r{)} @gol
 -Wunused-but-set-parameter @r{(only with} @option{-Wunused} @r{or} @option{-Wall}@r{)}  @gol
+-Wfloat-conversion @gol
 }
 
 The option @option{-Wextra} also prints warning messages for the
@@ -4570,6 +4572,14 @@ value, like assigning a signed integer e
 integer variable. An explicit cast silences the warning. In C, this
 option is enabled also by @option{-Wconversion}.
 
+@item -Wfloat-conversion
+@opindex Wfloat-conversion
+@opindex Wno-float-conversion
+Warn for implicit conversions that reduce the precision of a real value. 
+This includes conversions from real to integer, and from higher precision
+real to lower precision real values.  This option is also enabled by 
+@option{-Wconversion} or @option{-Wextra}.
+
 @item -Wsizeof-pointer-memaccess
 @opindex Wsizeof-pointer-memaccess
 @opindex Wno-sizeof-pointer-memaccess
Index: gcc/mcf.c
===================================================================
--- gcc/mcf.c	(revision 203112)
+++ gcc/mcf.c	(working copy)
@@ -348,8 +348,8 @@ mcf_sqrt (double x)
 
   gcc_assert (x >= 0);
 
-  convertor.floatPart = x;
-  convertor2.floatPart = x;
+  convertor.floatPart = (float)x;
+  convertor2.floatPart = (float)x;
   convertor.intPart = MAGIC_CONST1 + (convertor.intPart >> 1);
   convertor2.intPart = MAGIC_CONST2 - (convertor2.intPart >> 1);
 
Index: gcc/predict.c
===================================================================
--- gcc/predict.c	(revision 203112)
+++ gcc/predict.c	(working copy)
@@ -792,7 +792,7 @@ combine_predictions_for_insn (rtx insn,
 	  /* If one probability is 0% and one 100%, avoid division by zero.  */
 	  combined_probability = REG_BR_PROB_BASE / 2;
 	else
-	  combined_probability = (((double) combined_probability) * probability
+	  combined_probability = (int)(((double) combined_probability) * probability
 				  * REG_BR_PROB_BASE / d + 0.5);
       }
 
@@ -957,7 +957,7 @@ combine_predictions_for_bb (basic_block
 	    /* If one probability is 0% and one 100%, avoid division by zero.  */
 	    combined_probability = REG_BR_PROB_BASE / 2;
 	  else
-	    combined_probability = (((double) combined_probability)
+	    combined_probability = (int)(((double) combined_probability)
 				    * probability
 		    		    * REG_BR_PROB_BASE / d + 0.5);
 	}
Index: gcc/real.c
===================================================================
--- gcc/real.c	(revision 203112)
+++ gcc/real.c	(working copy)
@@ -1550,14 +1550,14 @@ real_to_decimal_for_mode (char *str, con
     }
 
   /* Bound the number of digits printed by the size of the representation.  */
-  max_digits = SIGNIFICAND_BITS * M_LOG10_2;
+  max_digits = (int)(SIGNIFICAND_BITS * M_LOG10_2);
   if (digits == 0 || digits > max_digits)
     digits = max_digits;
 
   /* Estimate the decimal exponent, and compute the length of the string it
      will print as.  Be conservative and add one to account for possible
      overflow or rounding error.  */
-  dec_exp = REAL_EXP (&r) * M_LOG10_2;
+  dec_exp = (int)(REAL_EXP (&r) * M_LOG10_2);
   for (max_digits = 1; dec_exp ; max_digits++)
     dec_exp /= 10;
 
@@ -2215,7 +2215,7 @@ decimal_integer_string (char *str, const
   sign = r.sign;
   r.sign = 0;
 
-  dec_exp = REAL_EXP (&r) * M_LOG10_2;
+  dec_exp = (int)(REAL_EXP (&r) * M_LOG10_2);
   digits = dec_exp + 1;
   gcc_assert ((digits + 2) < (int)buf_size);
 
@@ -2818,7 +2818,7 @@ significand_size (enum machine_mode mode
 	 than the number of bits required to hold the largest coefficient
 	 of this mode.  */
       double log2_10 = 3.3219281;
-      return fmt->p * log2_10;
+      return (int)(fmt->p * log2_10);
     }
   return fmt->p;
 }
Index: libcpp/symtab.c
===================================================================
--- libcpp/symtab.c	(revision 203112)
+++ libcpp/symtab.c	(working copy)
@@ -287,7 +287,8 @@ ht_dump_statistics (cpp_hash_table *tabl
 		     : (x) / (1024*1024))))
 #define LABEL(x) ((x) < 1024*10 ? ' ' : ((x) < 1024*1024*10 ? 'k' : 'M'))
 
-  total_bytes = longest = sum_of_squares = nids = 0;
+  total_bytes = longest = nids = 0;
+  sum_of_squares = 0.0;
   p = table->entries;
   limit = p + table->nslots;
   do


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