Patchwork RFA: PATCH: -Wimplicit-double

login
register
mail settings
Submitter Mark Mitchell
Date Sept. 3, 2010, 3:59 p.m.
Message ID <20100903155938.AFAD85664F5@henry1.codesourcery.com>
Download mbox | patch
Permalink /patch/63695/
State New
Headers show

Comments

Mark Mitchell - Sept. 3, 2010, 3:59 p.m.
This patch implements -Wimplicit-double, a new warning.  The goal of
the warning is to warn users when a "float" is implicitly converted to
a "double".  That's useful on a machine with a single-precision FPU;
it's easy to write something like:

  float area(float radius) { return 3.1459 * radius * radius; }

and not realize that that you're going to end up with all your math
being done in software emulation.

This patch implements the warning only for C.  Once that functionality
has been committed, I will also implement the warning for C++.

Tested on arm-none-eabi.

OK to apply?

--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

2010-09-03  Mark Mitchell  <mark@codesourcery.com>

	* c.opt (Wimplicit-double): New.

2010-09-03  Mark Mitchell  <mark@codesourcery.com>

	* doc/invoke.texi: Document it.
	* c-typeck.c (convert_arguments): Check for implicit conversions
	from float to double.
	(do_warn_implicit_double): New function.
	(build_conditional_expr): Use it.
	(build_binary_op): Likewise.

2010-09-03  Mark Mitchell  <mark@codesourcery.com>

	* gcc.dg/Wimplicit-double.c: New.

# gcc diff
pushd /scratch/mitchell/builds/fsf-mainline/src/gcc-mainline
svn diff
popd

Patch

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 163782)
+++ gcc/doc/invoke.texi	(working copy)
@@ -241,8 +241,8 @@  Objective-C and Objective-C++ Dialects}.
 -Wno-format-contains-nul -Wno-format-extra-args -Wformat-nonliteral @gol
 -Wformat-security  -Wformat-y2k @gol
 -Wframe-larger-than=@var{len} -Wjump-misses-init -Wignored-qualifiers @gol
--Wimplicit  -Wimplicit-function-declaration  -Wimplicit-int @gol
--Winit-self  -Winline @gol
+-Wimplicit  -Wimplicit-double -Wimplicit-function-declaration @gol
+-Wimplicit-int -Winit-self  -Winline @gol
 -Wno-int-to-pointer-cast -Wno-invalid-offsetof @gol
 -Winvalid-pch -Wlarger-than=@var{len}  -Wunsafe-loop-optimizations @gol
 -Wlogical-op -Wlong-long @gol
@@ -3198,6 +3198,30 @@  enabled by default and it is made into a
 Same as @option{-Wimplicit-int} and @option{-Wimplicit-function-declaration}.
 This warning is enabled by @option{-Wall}.
 
+@item -Wimplicit-double @r{(C and Objective-C only)}
+@opindex Wimplicit-double
+@opindex Wno-implicit-double
+Give a warning when a value of type @code{float} is implicitly
+promoted to @code{double}.  CPUs with a 32-bit ``single-precision''
+floating-point unit implement @code{float} in hardware, but emulate
+@code{double} in software.  On such a machine, doing computations
+using @code{double} values is much more expensive because of the
+overhead required for software emulation.  
+
+It is easy to accidentally do computations with @code{double} because
+floating-point literals are implicitly of type @code{double}.  For
+example, in:
+@smallexample
+@group
+float area(float radius)
+@{
+   return 3.14159 * radius * radius;        
+@}
+@end group
+@end smallexample
+the compiler will perform the entire computation with @code{double}
+because the floating-point literal is a @code{double}.
+
 @item -Wignored-qualifiers @r{(C and C++ only)}
 @opindex Wignored-qualifiers
 @opindex Wno-ignored-qualifiers
Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 163782)
+++ gcc/c-family/c.opt	(working copy)
@@ -266,6 +266,10 @@  Wimplicit
 C ObjC Var(warn_implicit) Init(-1) Warning
 Warn about implicit declarations
 
+Wimplicit-double
+C ObjC C++ ObjC++ Var(warn_implicit_double) Warning
+Warn about implicit conversions from \"float\" to \"double\"
+
 Wimplicit-function-declaration
 C ObjC Var(warn_implicit_function_declaration) Init(-1) Warning
 Warn about implicit function declarations
Index: gcc/testsuite/gcc.dg/Wimplicit-double.c
===================================================================
--- gcc/testsuite/gcc.dg/Wimplicit-double.c	(revision 0)
+++ gcc/testsuite/gcc.dg/Wimplicit-double.c	(revision 0)
@@ -0,0 +1,104 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Wimplicit-double" } */
+
+#include <stddef.h>
+
+/* Some targets do not provide <complex.h> so we define I ourselves.  */
+#define I 1.0iF
+#define ID ((_Complex double)I)
+
+float f;
+double d;
+int i;
+long double ld;
+_Complex float cf;
+_Complex double cd;
+_Complex long double cld;
+size_t s;
+
+extern void unprototyped_fn ();
+extern void varargs_fn (int, ...);
+extern void double_fn (double);
+extern float float_fn (void);
+
+void 
+usual_arithmetic_conversions(void) 
+{
+  float local_f;
+  _Complex float local_cf;
+
+  /* Values of type "float" are implicitly converted to "double" or
+     "long double" due to use in arithmetic with "double" or "long
+     double" operands.  */
+  local_f = f + 1.0;         /* { dg-warning "implicit" } */
+  local_f = f - d;           /* { dg-warning "implicit" } */
+  local_f = 1.0f * 1.0;      /* { dg-warning "implicit" } */
+  local_f = 1.0f / d;        /* { dg-warning "implicit" } */
+
+  local_cf = cf + 1.0;       /* { dg-warning "implicit" } */
+  local_cf = cf - d;         /* { dg-warning "implicit" } */
+  local_cf = cf + 1.0 * ID;  /* { dg-warning "implicit" } */
+  local_cf = cf - cd;        /* { dg-warning "implicit" } */
+  
+  local_f = i ? f : d;       /* { dg-warning "implicit" } */
+  i = f == d;                /* { dg-warning "implicit" } */
+  i = d != f;                /* { dg-warning "implicit" } */
+}
+
+void 
+default_argument_promotion (void) 
+{
+  /* Because there is no prototype, "f" is promoted to "double".  */
+  unprototyped_fn (f); /* { dg-warning "implicit" } */
+  undeclared_fn (f);   /* { dg-warning "implicit" } */
+  /* Because "f" is part of the variable argument list, it is promoted
+     to "double".  */
+  varargs_fn (1, f);   /* { dg-warning "implicit" } */
+}
+
+/* There is no warning when an explicit cast is used to perform the
+   conversion.  */
+
+void
+casts (void) 
+{
+  float local_f;
+  _Complex float local_cf;
+
+  local_f = (double)f + 1.0;                 /* { dg-bogus "implicit" } */
+  local_f = (double)f - d;                   /* { dg-bogus "implicit" } */
+  local_f = (double)1.0f + 1.0;              /* { dg-bogus "implicit" } */
+  local_f = (double)1.0f - d;                /* { dg-bogus "implicit" } */
+
+  local_cf = (_Complex double)cf + 1.0;      /* { dg-bogus "implicit" } */
+  local_cf = (_Complex double)cf - d;        /* { dg-bogus "implicit" } */
+  local_cf = (_Complex double)cf + 1.0 * ID; /* { dg-bogus "implicit" } */
+  local_cf = (_Complex double)cf - cd;       /* { dg-bogus "implicit" } */
+
+  local_f = i ? (double)f : d;               /* { dg-bogus "implicit" } */
+  i = (double)f == d;                        /* { dg-bogus "implicit" } */
+  i = d != (double)f;                        /* { dg-bogus "implicit" } */
+}
+
+/* There is no warning on conversions that occur in assignment (and
+   assignment-like) contexts.  */
+
+void 
+assignments (void)
+{
+  d = f;           /* { dg-bogus "implicit" } */
+  double_fn (f);   /* { dg-bogus "implicit" } */
+  d = float_fn (); /* { dg-bogus "implicit" } */
+}
+
+/* There is no warning in non-evaluated contexts.  */
+
+void
+non_evaluated (void)
+{
+  s = sizeof (f + 1.0);             /* { dg-bogus "implicit" } */
+  s = __alignof__ (f + 1.0);        /* { dg-bogus "implicit" } */
+  d = (__typeof__(f + 1.0))f;       /* { dg-bogus "implicit" } */
+  s = sizeof (i ? f : d);           /* { dg-bogus "implicit" } */
+  s = sizeof (unprototyped_fn (f)); /* { dg-bogus "implicit" } */
+}
Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c	(revision 163782)
+++ gcc/c-typeck.c	(working copy)
@@ -3106,8 +3106,15 @@  convert_arguments (tree typelist, VEC(tr
 	  if (type_generic)
 	    parmval = val;
 	  else
-	    /* Convert `float' to `double'.  */
-	    parmval = convert (double_type_node, val);
+	    {
+	      /* Convert `float' to `double'.  */
+	      if (warn_implicit_double && !c_inhibit_evaluation_warnings)
+		warning (OPT_Wimplicit_double,
+			 "implicit conversion from %qT to %qT when passing "
+			 "argument to function",
+			 valtype, double_type_node);
+	      parmval = convert (double_type_node, val);
+	    }
 	}
       else if (excess_precision && !type_generic)
 	/* A "double" argument with excess precision being passed
@@ -4029,6 +4036,39 @@  ep_convert_and_check (tree type, tree ex
   return convert (type, expr);
 }
 
+/* RESULT_TYPE is the result of converting TYPE1 and TYPE2 to a common
+   type via c_common_type.  If -Wimplicit-double is in use, and the
+   conditions for warning have been met, issue a warning.  MSG is the
+   warning message.  It must have two %T specifiers for the type that
+   was converted (generally "float") and the type to which it was
+   converted (generally "double), respectively.  */
+
+static void
+do_warn_implicit_double (tree result_type, tree type1, tree type2,
+			 const char *msg)
+{
+  tree source_type;
+
+  if (!warn_implicit_double)
+    return;
+  /* If the conversion will not occur at run-time, there is no need to
+     warn about it.  */
+  if (c_inhibit_evaluation_warnings)
+    return;
+  if (TYPE_MAIN_VARIANT (result_type) != double_type_node
+      && TYPE_MAIN_VARIANT (result_type) != complex_double_type_node)
+    return;
+  if (TYPE_MAIN_VARIANT (type1) == float_type_node
+      || TYPE_MAIN_VARIANT (type1) == complex_float_type_node)
+    source_type = type1;
+  else if (TYPE_MAIN_VARIANT (type2) == float_type_node
+	   || TYPE_MAIN_VARIANT (type2) == complex_float_type_node)
+    source_type = type2;
+  else
+    return;
+  warning (OPT_Wimplicit_double, msg, source_type, result_type);
+}
+
 /* Build and return a conditional expression IFEXP ? OP1 : OP2.  If
    IFEXP_BCP then the condition is a call to __builtin_constant_p, and
    if folded to an integer constant then the unselected half may
@@ -4140,6 +4180,9 @@  build_conditional_expr (location_t colon
 	       || code2 == COMPLEX_TYPE))
     {
       result_type = c_common_type (type1, type2);
+      do_warn_implicit_double (result_type, type1, type2,
+			       "implicit conversion from %qT to %qT to "
+			       "match other result of conditional");
 
       /* If -Wsign-compare, warn here if type1 and type2 have
 	 different signedness.  We'll promote the signed to unsigned
@@ -9822,6 +9865,10 @@  build_binary_op (location_t location, en
       if (shorten || common || short_compare)
 	{
 	  result_type = c_common_type (type0, type1);
+	  do_warn_implicit_double (result_type, type0, type1,
+				   "implicit conversion from %qT to %qT "
+				   "to match other operand of binary "
+				   "expression");
 	  if (result_type == error_mark_node)
 	    return error_mark_node;
 	}