Changes compared to previous version of patch: This patch only includes -Wfloat-conversion in -Wconversion, it does not include it in -Wextra It also changes several testcases to use -Wfloat-conversion instead of -Wconversion A split out patch to make the floating conversions explicit is attached at the PR 53001 page, but is not included in this patch.
On 10/09/2013 02:38 PM, Joseph S. Myers wrote: > Also note that this patch needs to add testcases to the testsuite > (gcc/testsuite/c-c++-common/, probably) testing what cases generate > warnings with the new option and what cases don't. > === Description === 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 -Wconversion (since it is a subset) It was suggested on the discussion for bug 53001 that this be enabled by -Wextra, but this version of the patch does not. I am not certain that c.opt was modified correctly. == Testcases == This patch has passes the existing -Wconversion testcases. It modifies Wconversion-real.c, Wconversion-real-integer.c and pr35635.c to be more specific == Changelog == 2013-10-13 Joshua Cogliati <jrinc...@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.opt Adding new warning float-conversion and enabling it -Wconversion * gcc/doc/invoke.texi Adding documentation about -Wfloat-conversion * gcc/testsuite/c-c++-common/Wconversion-real.c Switching tests to use float-conversion * gcc/testsuite/gcc.dg/Wconversion-real-integer.c Switching tests to use float-conversion * gcc/testsuite/gcc.dg/pr35635.c Switching tests to use float-conversion == Bootstrapping and testing == Tested bootstrap on x86_64-unknown-linux-gnu for --enable-languages=c,c++,fortran,java,lto,objc with trunk on r203112 Thank you for consideration of this patch.
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.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) +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 @@ -4570,6 +4571,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}. + @item -Wsizeof-pointer-memaccess @opindex Wsizeof-pointer-memaccess @opindex Wno-sizeof-pointer-memaccess Index: gcc/testsuite/c-c++-common/Wconversion-real.c =================================================================== --- gcc/testsuite/c-c++-common/Wconversion-real.c (revision 203112) +++ gcc/testsuite/c-c++-common/Wconversion-real.c (working copy) @@ -2,8 +2,8 @@ /* { dg-do compile } */ /* { dg-skip-if "doubles are floats" { "avr-*-*" } { "*" } { "" } } */ -/* { dg-options "-std=c99 -Wconversion" { target c } } */ -/* { dg-options "-Wconversion" { target c++ } } */ +/* { dg-options "-std=c99 -Wfloat-conversion" { target c } } */ +/* { dg-options "-Wfloat-conversion" { target c++ } } */ /* { dg-require-effective-target large_double } */ float vfloat; @@ -20,18 +20,18 @@ void h (void) double d = 0; long double ld = 0; - ffloat (3.1); /* { dg-warning "conversion" } */ - vfloat = 3.1; /* { dg-warning "conversion" } */ - ffloat (3.1L); /* { dg-warning "conversion" } */ - vfloat = 3.1L; /* { dg-warning "conversion" } */ - fdouble (3.1L); /* { dg-warning "conversion" "" { target large_long_double } } */ - vdouble = 3.1L; /* { dg-warning "conversion" "" { target large_long_double } } */ - ffloat (vdouble); /* { dg-warning "conversion" } */ - vfloat = vdouble; /* { dg-warning "conversion" } */ - ffloat (vlongdouble); /* { dg-warning "conversion" } */ - vfloat = vlongdouble; /* { dg-warning "conversion" } */ - fdouble (vlongdouble); /* { dg-warning "conversion" "" { target large_long_double } } */ - vdouble = vlongdouble; /* { dg-warning "conversion" "" { target large_long_double } } */ + ffloat (3.1); /* { dg-warning "float-conversion" } */ + vfloat = 3.1; /* { dg-warning "float-conversion" } */ + ffloat (3.1L); /* { dg-warning "float-conversion" } */ + vfloat = 3.1L; /* { dg-warning "float-conversion" } */ + fdouble (3.1L); /* { dg-warning "float-conversion" "" { target large_long_double } } */ + vdouble = 3.1L; /* { dg-warning "float-conversion" "" { target large_long_double } } */ + ffloat (vdouble); /* { dg-warning "float-conversion" } */ + vfloat = vdouble; /* { dg-warning "float-conversion" } */ + ffloat (vlongdouble); /* { dg-warning "float-conversion" } */ + vfloat = vlongdouble; /* { dg-warning "float-conversion" } */ + fdouble (vlongdouble); /* { dg-warning "float-conversion" "" { target large_long_double } } */ + vdouble = vlongdouble; /* { dg-warning "float-conversion" "" { target large_long_double } } */ ffloat ((float) 3.1); Index: gcc/testsuite/gcc.dg/Wconversion-real-integer.c =================================================================== --- gcc/testsuite/gcc.dg/Wconversion-real-integer.c (revision 203112) +++ gcc/testsuite/gcc.dg/Wconversion-real-integer.c (working copy) @@ -25,18 +25,18 @@ void h (void) float f = 3; double d = 3; - fsi (3.1f); /* { dg-warning "conversion" } */ - si = 3.1f; /* { dg-warning "conversion" } */ - fsi (3.1); /* { dg-warning "conversion" } */ - si = 3.1; /* { dg-warning "conversion" } */ - fsi (d); /* { dg-warning "conversion" } */ - si = d; /* { dg-warning "conversion" } */ + fsi (3.1f); /* { dg-warning "float-conversion" } */ + si = 3.1f; /* { dg-warning "float-conversion" } */ + fsi (3.1); /* { dg-warning "float-conversion" } */ + si = 3.1; /* { dg-warning "float-conversion" } */ + fsi (d); /* { dg-warning "float-conversion" } */ + si = d; /* { dg-warning "float-conversion" } */ fui (-1.0); /* { dg-warning "overflow" } */ ui = -1.0; /* { dg-warning "overflow" } */ - ffloat (INT_MAX); /* { dg-warning "conversion" } */ - vfloat = INT_MAX; /* { dg-warning "conversion" } */ - ffloat (16777217); /* { dg-warning "conversion" } */ - vfloat = 16777217; /* { dg-warning "conversion" } */ + ffloat (INT_MAX); /* { dg-warning "float-conversion" } */ + vfloat = INT_MAX; /* { dg-warning "float-conversion" } */ + ffloat (16777217); /* { dg-warning "float-conversion" } */ + vfloat = 16777217; /* { dg-warning "float-conversion" } */ ffloat (si); /* { dg-warning "conversion" } */ vfloat = si; /* { dg-warning "conversion" } */ ffloat (ui); /* { dg-warning "conversion" } */ Index: gcc/testsuite/gcc.dg/pr35635.c =================================================================== --- gcc/testsuite/gcc.dg/pr35635.c (revision 203112) +++ gcc/testsuite/gcc.dg/pr35635.c (working copy) @@ -45,7 +45,7 @@ void func2() /* At least one branch of ? does not fit in the destination, thus warn. */ - schar_x = bar != 0 ? 2.1 : 10; /* { dg-warning "conversion" } */ + schar_x = bar != 0 ? 2.1 : 10; /* { dg-warning "float-conversion" } */ schar_x = bar != 0 ? (signed char) 1024: -1024; /* { dg-warning "conversion" } */ } @@ -61,7 +61,7 @@ void func3() /* At least one branch of ? does not fit in the destination, thus warn. */ - uchar_x = bar != 0 ? 2.1 : 10; /* { dg-warning "conversion" } */ + uchar_x = bar != 0 ? 2.1 : 10; /* { dg-warning "float-conversion" } */ uchar_x = bar != 0 ? (unsigned char) 1024 : -1; /* { dg-warning "negative integer implicitly converted to unsigned type" } */