-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Since part of it was to go into c-family (as per Joseph S. Myers's email) and parts are not in c-family, I split the changelog into three parts. I also changed the formatting of the changelog and patch as per Dodji's comments. The attached patch is the same as before, but the diff for Wfloat-conversion.c is as a new file, not as a copy of Wconversion-real.c. Thanks for reviewing it. If there is anything else that needs changing, please tell me.
For gcc/c-family/ChangeLog:
PR c/53001
Splitting out a -Wfloat-conversion from -Wconversion for
conversions that lower floating point number precision
or conversion from floating point numbers to integers
* c-common.c (unsafe_conversion_p): Make this function
return an enumeration with more detail.
(conversion_warning): Use the new return type of
unsafe_conversion_p to separately warn either about conversions
that lower floating point number precision or about the other
kinds of conversions.
* c-common.h: Adding conversion_safety enumeration.
(unsafe_conversion_p): switching return type to
conversion_safety enumeration.
* c.opt: Adding new warning float-conversion and
enabling it -Wconversion
For gcc/ChangeLog:
PR c/53001
Splitting out a -Wfloat-conversion from -Wconversion for
conversions that lower floating point number precision
or conversion from floating point numbers to integers
* doc/invoke.texi: Adding documentation about
-Wfloat-conversion
For gcc/testsuite/ChangeLog
PR c/53001
Splitting out a -Wfloat-conversion from -Wconversion for
conversions that lower floating point number precision
or conversion from floating point numbers to integers
* c-c++-common/Wfloat-conversion.c: Copies relevant
tests from c-c++-common/Wconversion-real.c,
gcc.dg/Wconversion-real-integer.c and gcc.dg/pr35635.c into
new testcase for conversions that are warned about by
-Wfloat-conversion
On 10/28/2013 02:50 AM, Dodji Seketeli wrote:
> Hello Joshua,
>
> Joshua J Cogliati <jrinc...@yahoo.com> writes:
>
>> I am not certain that c.opt was modified correctly.
>
> I don't see any problem with the c.opt part. So unless another
> maintainer says otherwise, I'd say this is OK.
>
>> 1. warn_float_patch_and_new_testcase.diff This adds a new
>> testcase and checks for float-conversion in the warning. This
>> will add somewhat more time for running the testcases compared to
>> version 1 while still testing more or less the same code paths.
>> This does however check that the warning occurs when -Wconversion
>> is not used.
>
> As said earlier, I'd really prefer this approach because it leaves
> the existing tests alone and just adds new one. I guess we cannot
> do much at this point for the speed concern (okay, we could try and
> make the test harness run more tests in parallel but that is a
> separate discussion) and I think Joseph agrees too. So please
> let's stick to this approach.
>
>
>> Changelog for warn_float_patch_and_new_testcase.diff and
>> warn_float_patch_and_new_testcase2.diff:
>>
>> Splitting out a -Wfloat-conversion from -Wconversion for
>> conversions that lower floating point number precision or
>> conversion from floating point numbers to integers *
>> c-family/c-common.c Switching unsafe_conversion_p to return an
>> enumeration with more detail, and conversion_warning to use this
>> information.
>
> Please, strictly follow the same format as the others entries in
> the ChangeLog file. That is, explicitly write the names of the
> functions you changed, in parenthesis, followed by a colon. That
> would make the entry look like:
>
> * c-family/c-common.c (unsafe_conversion_p): Make this function
> return an enumeration with more detail. (conversion_warning): Use
> the new return type of unsafe_conversion_p to separately warn
> either about conversions that lower floating point number precision
> or about the other kinds of conversions.
Thanks, done.
>
>> * c-family/c-common.h Adding conversion_safety enumeration and
>> switching return type of unsafe_conversion_p
>
> Likewise.
Changed.
>
>> * c-family/c.opt Adding new warning float-conversion and enabling
>> it -Wconversion * doc/invoke.texi Adding documentation about
>> -Wfloat-conversion
>
> Likewise (colon missing at the end of the file name).
>
>
>> * testsuite/c-c++-common/Wfloat-conversion.c Copies relevant
>> tests from c-c++-common/Wconversion-real.c,
>> gcc.dg/Wconversion-real-integer.c and gcc.dg/pr35635.c into new
>> testcase for ones that are warned about by -Wfloat-conversion
Changed.
>
> You need to add the above ChangeLog entry to the ChangeLog file in
> gcc/testsuite/ChangeLog. Thus, the entry would look like (note how
> the prefix testsuite/ is removed from the path name):
>
> * c-c++-common/Wfloat-conversion.c: New test file. Its content
> started as a copy of c-c++-common/Wconversion-real.c,
> gcc.dg/Wconversion-real-integer.c and gcc.dg/pr35635.c. It has
> been augmented by new tests to exercise the -Wfloat-conversion
> option.
I have included it in the email as per
http://gcc.gnu.org/contribute.html#patches but if you mean I should
include the changelog in the patch, I can do that.
>
>> Index: gcc/testsuite/c-c++-common/Wfloat-conversion.c
>> ===================================================================
>>
>>
- --- gcc/testsuite/c-c++-common/Wfloat-conversion.c (working copy)
>> +++ gcc/testsuite/c-c++-common/Wfloat-conversion.c (working
>> copy) @@ -1,85 +1,58 @@ /* Test for diagnostics for Wconversion
>> for floating-point. */
>
> Hmmh. I think this diff hunk should say that this is a new file.
> Here what it says is that it's a modification of an existing file.
Changed, I switch it to diff as a new file, not as a copy of a file.
> Thank you for your time on this.
You are welcome, thank you for reviewing this.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQEcBAEBAgAGBQJSc9DeAAoJEFeOW05LP1faDgwIALJUexPHFFWZqkX5qrbBgmVa
bGf52Q53hlLNPh8LW4ChEqfM/wSQA40SOXEqj0MTMKF+7O8AaJ473o3NLdAyKfZ2
+4ocrham/c11n4fHfVecqx7heSKvYHHutNMJ7JtBhKOYBok51jy5n0MoQ/MwJPJw
ikllXaRAsS9Mev+PUxIVYNXPhtyyZekLq6aWdxV5sXOtSbUgxrWq1C2qylX0U09t
B2w4iG7QWakPvW/mlCHH+fTFf5SFXvJKWJLX8oeUpq32NX/J996050SpB1M13WWl
RGPzhoNOvYc+eZRM4SSjlbtMz7NgFrff0jJB0qWpuENjNbuAGYhp5XYbtv5i0Dk=
=bjFz
-----END PGP SIGNATURE-----
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c (revision 203640)
+++ gcc/c-family/c-common.c (working copy)
@@ -2518,7 +2518,7 @@ shorten_binary_op (tree result_type, tre
}
/* Checks if expression EXPR of real/integer type cannot be converted
- to the real/integer type TYPE. Function returns true when:
+ to the real/integer type TYPE. Function returns non-zero when:
* EXPR is a constant which cannot be exactly converted to TYPE
* EXPR is not a constant and size of EXPR's type > than size of TYPE,
for EXPR type and TYPE being both integers or both real.
@@ -2526,12 +2526,12 @@ shorten_binary_op (tree result_type, tre
* EXPR is not a constant of integer type which cannot be
exactly converted to real type.
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
+ can return SAFE_CONVERSION (zero) in that case. Function can produce
+ signedness warnings if PRODUCE_WARNS is true. */
+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);
@@ -2543,7 +2543,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
@@ -2564,7 +2564,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)
{
@@ -2573,7 +2573,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. */
@@ -2582,7 +2582,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;
}
}
}
@@ -2591,7 +2591,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)
@@ -2629,7 +2629,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
@@ -2638,12 +2638,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. */
@@ -2679,14 +2679,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;
@@ -2700,8 +2700,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))
@@ -2728,7 +2729,12 @@ conversion_warning (tree type, tree expr
case REAL_CST:
case INTEGER_CST:
- if (unsafe_conversion_p (type, expr, true))
+ 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);
@@ -2747,7 +2753,12 @@ conversion_warning (tree type, tree expr
}
default: /* 'expr' is not a constant. */
- if (unsafe_conversion_p (type, expr, true))
+ 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 203640)
+++ 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 enumerators 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 203640)
+++ 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 203640)
+++ 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/Wfloat-conversion.c
===================================================================
--- gcc/testsuite/c-c++-common/Wfloat-conversion.c (revision 0)
+++ gcc/testsuite/c-c++-common/Wfloat-conversion.c (working copy)
@@ -0,0 +1,58 @@
+/* Test for diagnostics for Wconversion for floating-point. */
+
+/* { dg-do compile } */
+/* { dg-options "-std=c99 -Wfloat-conversion" { target c } } */
+/* { dg-options "-Wfloat-conversion" { target c++ } } */
+/* { dg-require-effective-target large_double } */
+/* { dg-require-effective-target int32plus } */
+/* { dg-require-effective-target double64plus } */
+#include <limits.h>
+
+float vfloat;
+double vdouble;
+long double vlongdouble;
+int bar;
+
+void fsi (signed int x);
+void fui (unsigned int x);
+void ffloat (float f);
+void fdouble (double d);
+void flongdouble (long double ld);
+
+void h (void)
+{
+ unsigned int ui = 3;
+ int si = 3;
+ unsigned char uc = 3;
+ signed char sc = 3;
+ float f = 0;
+ double d = 0;
+ long double ld = 0;
+
+ ffloat (3.1); /* { dg-warning "conversion to 'float' alters 'double' constant value" } */
+ vfloat = 3.1; /* { dg-warning "conversion to 'float' alters 'double' constant value" } */
+ ffloat (3.1L); /* { dg-warning "conversion to 'float' alters 'long double' constant value" } */
+ vfloat = 3.1L; /* { dg-warning "conversion to 'float' alters 'long double' constant value" } */
+ fdouble (3.1L); /* { dg-warning "conversion to 'double' alters 'long double' constant value" "" { target large_long_double } } */
+ vdouble = 3.1L; /* { dg-warning "conversion to 'double' alters 'long double' constant value" "" { target large_long_double } } */
+ ffloat (vdouble); /* { dg-warning "conversion to 'float' from 'double' may alter its value" } */
+ vfloat = vdouble; /* { dg-warning "conversion to 'float' from 'double' may alter its value" } */
+ ffloat (vlongdouble); /* { dg-warning "conversion to 'float' from 'long double' may alter its value" } */
+ vfloat = vlongdouble; /* { dg-warning "conversion to 'float' from 'long double' may alter its value" } */
+ fdouble (vlongdouble); /* { dg-warning "conversion to 'double' from 'long double' may alter its value" "" { target large_long_double } } */
+ vdouble = vlongdouble; /* { dg-warning "conversion to 'double' from 'long double' may alter its value" "" { target large_long_double } } */
+
+ fsi (3.1f); /* { dg-warning "conversion to 'int' alters 'float' constant value" } */
+ si = 3.1f; /* { dg-warning "conversion to 'int' alters 'float' constant value" } */
+ fsi (3.1); /* { dg-warning "conversion to 'int' alters 'double' constant value" } */
+ si = 3.1; /* { dg-warning "conversion to 'int' alters 'double' constant value" } */
+ fsi (d); /* { dg-warning "conversion to 'int' from 'double' may alter its value" } */
+ si = d; /* { dg-warning "conversion to 'int' from 'double' may alter its value" } */
+ ffloat (INT_MAX); /* { dg-warning "conversion to 'float' alters 'int' constant value" } */
+ vfloat = INT_MAX; /* { dg-warning "conversion to 'float' alters 'int' constant value" } */
+ ffloat (16777217); /* { dg-warning "conversion to 'float' alters 'int' constant value" } */
+ vfloat = 16777217; /* { dg-warning "conversion to 'float' alters 'int' constant value" } */
+
+ sc = bar != 0 ? 2.1 : 10; /* { dg-warning "conversion to 'signed char' alters 'double' constant value" } */
+ uc = bar != 0 ? 2.1 : 10; /* { dg-warning "conversion to 'unsigned char' alters 'double' constant value" } */
+}
warn_float_patch_and_new_testcase3.diff.sig
Description: PGP signature
