[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001
--- Comment #30 from Joshua Cogliati jjcogliati-r1 at yahoo dot com ---
Yes, it is fixed so far as I am concerned. I checked out gcc trunk 205109, and
bootstraped it and tried it out on:
int main(int argc, char ** argv) {
int i = 3.14;
return i;
}
int foo(double x)
{
return x;
}
float foo2(double x) {
return x;
}
and without the flag it didn't warn, but with the flag it did:
gcc -Wall -Wfloat-conversion -o float_test float_test.c
float_test.c: In function ‘main’:
float_test.c:2:3: warning: conversion to ‘int’ alters ‘double’ constant value
[-Wfloat-conversion]
int i = 3.14;
^
float_test.c: In function ‘foo’:
float_test.c:8:3: warning: conversion to ‘int’ from ‘double’ may alter its
value [-Wfloat-conversion]
return x;
^
float_test.c: In function ‘foo2’:
float_test.c:12:3: warning: conversion to ‘float’ from ‘double’ may alter its
value [-Wfloat-conversion]
return x;
^
I think there are two things remaining to be done.
1. http://gcc.gnu.org/gcc-4.9/changes.html does not yet list it as a change,
2. for gcc 4.10 (or what ever the next gcc version is) I think it might be
worth considering as a -Wextra or -Wall. That however would be a separate bug
number and would require something like the Patch to fixup gcc float
conversions in GCC and another patch to turn it on with the flag.
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #28 from Manuel López-Ibáñez manu at gcc dot gnu.org --- Author: manu Date: Wed Nov 20 07:15:40 2013 New Revision: 205090 URL: http://gcc.gnu.org/viewcvs?rev=205090root=gccview=rev Log: 2013-11-19 Joshua J Cogliati jrinc...@yahoo.com 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. gcc/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. * c-common.h (enum conversion_safety): New enumeration. (unsafe_conversion_p): switching return type to conversion_safety enumeration. * c.opt: Adding new warning -Wfloat-conversion and enabling it with -Wconversion. gcc/ * doc/invoke.texi: Adding documentation about -Wfloat-conversion. gcc/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 conversions that are warned about by -Wfloat-conversion. Added: trunk/gcc/testsuite/c-c++-common/Wfloat-conversion.c Modified: trunk/gcc/ChangeLog trunk/gcc/c-family/ChangeLog trunk/gcc/c-family/c-common.c trunk/gcc/c-family/c-common.h trunk/gcc/c-family/c.opt trunk/gcc/doc/invoke.texi trunk/gcc/testsuite/ChangeLog
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 Manuel López-Ibáñez manu at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED --- Comment #29 from Manuel López-Ibáñez manu at gcc dot gnu.org --- I guess this is fixed for GCC 4.9.
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #27 from Joshua Cogliati jjcogliati-r1 at yahoo dot com --- Created attachment 31097 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=31097action=edit Patch to add -Wfloat-conversion option against trunk with new testcase and detailed warnings This uses a detailed warnings in the testcase. I am not sure if this is a good idea or not since it might get invalidated if the types change. It bootstraps and the new testcase works on x86_64-unknown-linux-gnu
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #26 from Joshua Cogliati jjcogliati-r1 at yahoo dot com --- Created attachment 31065 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=31065action=edit Patch to add -Wfloat-conversion option against trunk with new testcase This adds a new testcase instead of using existing testcases.
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 Joshua Cogliati jjcogliati-r1 at yahoo dot com changed: What|Removed |Added Attachment #30979|0 |1 is obsolete|| --- Comment #24 from Joshua Cogliati jjcogliati-r1 at yahoo dot com --- Created attachment 30994 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=30994action=edit Patch to add -Wfloat-conversion option against trunk with testcases This version modifies a few more testcases.
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 Joshua Cogliati jjcogliati-r1 at yahoo dot com changed: What|Removed |Added Attachment #30899|0 |1 is obsolete|| --- Comment #22 from Joshua Cogliati jjcogliati-r1 at yahoo dot com --- Created attachment 30979 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=30979action=edit Patch to add -Wfloat-conversion option against trunk This version only does the -Wfloat-conversion. It does not do the -Wextra or the changes needed to get -Wextra to bootstrap. It starts to change the test cases. This work is to satisfy the request on the gcc-patch list.
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #23 from Joshua Cogliati jjcogliati-r1 at yahoo dot com --- Created attachment 30980 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=30980action=edit Patch to fixup gcc float conversions in GCC This changes the float conversions in gcc so that gcc bootstraps if float conversions are warned about with -Wextra. Splitting the patch was requested on gcc-patches. (Note to self: patches were created with svn diff --diff-cmd diff -x -up )
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #21 from Manuel López-Ibáñez manu at gcc dot gnu.org --- (In reply to Joshua Cogliati from comment #20) Created attachment 30937 [details] Patch to add -Wfloat-conversion option against trunk Added one more changed needed to get it to compile (which only occured if I did an rm -Rf in the build directry, the make bootstrap didn't catch the libcpp warning found.) The only thing I have left is to possibly modify the test scripts (to use -Wfloat-conversion instead of -Wconversion for the relevant tests) and recompile against the current trunk. I would humbly suggest that you first recompile against current trunk, then submit to gcc-patches and CC Dodji and Jason Merrill (you can find the emails in MAINTAINERS) asking for comments before spending time and effort on updating the test cases. The maintainers may ask you to change the name, or they might not like that it triggers so often in GCC (so they may ask you to not enable it with -Wextra), or they might simply not like the patch at all. Please mention that it is your first patch to GCC and your copyright assignment is already in order. People are reluctant to review patches unless the papers are in order since there is a high chance that they will never be committed.
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 Joshua Cogliati jjcogliati-r1 at yahoo dot com changed: What|Removed |Added Attachment #30913|0 |1 is obsolete|| --- Comment #20 from Joshua Cogliati jjcogliati-r1 at yahoo dot com --- Created attachment 30937 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=30937action=edit Patch to add -Wfloat-conversion option against trunk Added one more changed needed to get it to compile (which only occured if I did an rm -Rf in the build directry, the make bootstrap didn't catch the libcpp warning found.) The only thing I have left is to possibly modify the test scripts (to use -Wfloat-conversion instead of -Wconversion for the relevant tests) and recompile against the current trunk.
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #19 from Joshua Cogliati jjcogliati-r1 at yahoo dot com --- Created attachment 30913 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=30913action=edit Patch to add -Wfloat-conversion option against trunk This version is against gcc trunk (rev 202818). It now bootstraps. It adds about ten casts so that the existing float conversions in gcc are now explicit instead of implicit so that gcc can bootstrap even with the new warning.
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #17 from Manuel López-Ibáñez manu at gcc dot gnu.org --- (In reply to Joshua Cogliati from comment #16) This does not bootstrap trunk yet, because gcc has floating conversion issues and with this being enabled by -Wextra and with -Werror, gcc fails to build. Are those real bugs or normal code?
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #18 from Joshua Cogliati jjcogliati-r1 at yahoo dot com --- (In reply to Manuel López-Ibáñez from comment #17) (In reply to Joshua Cogliati from comment #16) This does not bootstrap trunk yet, because gcc has floating conversion issues and with this being enabled by -Wextra and with -Werror, gcc fails to build. Are those real bugs or normal code? So far they seem to be normal code. I'll try eliminating the warning on a few, but if there are too many I'll just split the patch into the basic warning and one that occurs on -Wextra.
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 Joshua Cogliati jjcogliati-r1 at yahoo dot com changed: What|Removed |Added Attachment #30873|0 |1 is obsolete|| --- Comment #16 from Joshua Cogliati jjcogliati-r1 at yahoo dot com --- Created attachment 30899 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=30899action=edit Patch to add -Wfloat-conversion option against trunk This version is against gcc trunk (rev 202818). For trunk, I had to unmake unsafe_conversion_p being static (it is now extern again.) This does not bootstrap trunk yet, because gcc has floating conversion issues and with this being enabled by -Wextra and with -Werror, gcc fails to build.
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 Joshua Cogliati jjcogliati-r1 at yahoo dot com changed: What|Removed |Added Attachment #30870|0 |1 is obsolete|| Attachment #30871|0 |1 is obsolete|| --- Comment #15 from Joshua Cogliati jjcogliati-r1 at yahoo dot com --- Created attachment 30882 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=30882action=edit Patch to add -Wfloat-conversion option Adding documentation, formating better, removing assignment from test. Still based on 4.8.1 (which will be fixed later) Still a static function, C++ seems to be bootstraped fine.
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #10 from Richard Neill gcc at richardneill dot org --- Thank you for adding this - I will certainly find it useful! May I suggest that this warning be implicitly enabled by -Wextra, (even if it's not enabled by -Wall) ?
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 Joshua Cogliati jjcogliati-r1 at yahoo dot com changed: What|Removed |Added CC||jjcogliati-r1 at yahoo dot com --- Comment #8 from Joshua Cogliati jjcogliati-r1 at yahoo dot com --- Created attachment 30870 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=30870action=edit Patch to add -Wfloat-conversion option This patch add a -Wfloat-conversion option. It adds an enumeration type called conversion_safety, with a SAFE_CONVERSION being 0, and unsafe ones being non-zero. It changes the unsafe_conversion_p to return this enumeration. This allows the floating point conversions to be handled separately. Someone who understands the c.opt file better than I should check that I did it correctly. -Wconversion implies -Wfloat-conversion. -Wall does not, but it might possibly be worth considering.
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #9 from Joshua Cogliati jjcogliati-r1 at yahoo dot com --- Created attachment 30871 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=30871action=edit some floating conversion issues This tests three of the floating conversion reduction in precision issues. P.S. Copyright assignment paperwork from both myself and my employer for GCC has been sent to copyright-clerk
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #11 from Joshua Cogliati jjcogliati-r1 at yahoo dot com --- Created attachment 30873 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=30873action=edit Patch to add -Wfloat-conversion option This version also is enabled with -Wextra. $ gcc -o float_test float_test.c $ gcc -Wfloat-conversion -o float_test float_test.c float_test.c: In function ‘main’: float_test.c:2:3: warning: conversion to ‘int’ alters ‘double’ constant value [-Wfloat-conversion] int i = 3.14; ^ float_test.c: In function ‘foo’: float_test.c:8:3: warning: conversion to ‘int’ from ‘double’ may alter its value [-Wfloat-conversion] return x; ^ float_test.c: In function ‘foo2’: float_test.c:12:3: warning: conversion to ‘float’ from ‘double’ may alter its value [-Wfloat-conversion] return x; ^ $ gcc -Wconversion -o float_test float_test.c float_test.c: In function ‘main’: float_test.c:2:3: warning: conversion to ‘int’ alters ‘double’ constant value [-Wfloat-conversion] int i = 3.14; ^ float_test.c: In function ‘foo’: float_test.c:8:3: warning: conversion to ‘int’ from ‘double’ may alter its value [-Wfloat-conversion] return x; ^ float_test.c: In function ‘foo2’: float_test.c:12:3: warning: conversion to ‘float’ from ‘double’ may alter its value [-Wfloat-conversion] return x; ^ $ gcc -Wextra -o float_test float_test.c float_test.c: In function ‘main’: float_test.c:2:3: warning: conversion to ‘int’ alters ‘double’ constant value [-Wfloat-conversion] int i = 3.14; ^ float_test.c: In function ‘foo’: float_test.c:8:3: warning: conversion to ‘int’ from ‘double’ may alter its value [-Wfloat-conversion] return x; ^ float_test.c: In function ‘foo2’: float_test.c:12:3: warning: conversion to ‘float’ from ‘double’ may alter its value [-Wfloat-conversion] return x; ^
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001
--- Comment #12 from Manuel López-Ibáñez manu at gcc dot gnu.org ---
The patch looks mostly correct to me, but you will need a few extra details
before a maintainer (not me!) can accept it.
* You need to bootstrap + run the testsuite. You don't really need to add
testcases, because there are quite a few already in the testsuite for
Wconversion, but perhaps to be sure that your patch is correct, you could check
those and change -Wconversion for -Wfloat-conversion where it should trigger.
* You need to write a Changelog and submit to gcc-patc...@gcc.gnu.org. When
submitting CC do...@redhat.com who is the diagnostics maintainer.
* There are some potential issues in the patch that maintainers may complain
diff -ur gcc-4.8.1_orig/gcc/c-family/c-common.c
gcc-4.8.1/gcc/c-family/c-common.c
--- gcc-4.8.1_orig/gcc/c-family/c-common.c2013-05-14 14:52:27.0
-0600
+++ gcc-4.8.1/gcc/c-family/c-common.c2013-09-02 13:14:01.0 -0600
@@ -294,6 +294,8 @@
{NULL, 0, 0},
};
+enum conversion_safety
{SAFE_CONVERSION=0,UNSAFE_OTHER,UNSAFE_SIGN,UNSAFE_REAL};
+
Perhaps you should add a bit of documentation of what each value means? Also,
you don't seem to use UNSAFE_SIGN.
Also, some extra whitespace around =, after ',' and {, and before }.
-extern bool unsafe_conversion_p (tree, tree, bool);
This function is used in the C++ FE, you cannot make it static. Did you
bootstrap the compiler with the patch?
+ if ((conversion_kind = unsafe_conversion_p (type, expr, true)))
I don't think this is going to be accepted. You should split the assignment and
the test.
+Wfloat-conversion
+C ObjC C++ ObjC++ Var(warn_float_conversion) LangEnabledBy(C ObjC
C++,Wconversion) EnabledBy(Wextra)
This looks ok to me, but you could add ObjC++ in the LangEnabledBy part.
+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}.
and by @option{-Wextra}! You should also add it to the list of things enabled
by -Wextra (look where -Wextra is defined).
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #13 from Manuel López-Ibáñez manu at gcc dot gnu.org --- Another potential issue is that your patch is against 4.8.1, but this is not a regression, so it is very likely that maintainers will ask you to submit a patch with respect to current trunk: http://www.gnu.org/software/gcc/svn.html
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #14 from Joshua Cogliati jjcogliati-r1 at yahoo dot com --- Manuel López-Ibáñez, thank you for the patch review. I will make the changes you requested, and rebase it against svn trunk, and run bootstrap and testsuite. It may be a few weeks before I get this all done (depending on available time). UNSAFE_SIGN is unused, but when I was doing the patch, I noticed that unsafe_conversion_p basically does not report sign conversion errors, instead it does the warning directly inside the function. So there definitely is a distinct unsafe conversion type, but I am not sure if I should remove the constant entirely or not. Either way I will try and document the behavior better.
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #7 from Joshua Cogliati jjcogliati-r1 at yahoo dot com --- I wrote the patch for 4.8.1 (which is the easy part), now I will see if I can get approval to submit it through the bureaucracies (the hard part).
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #5 from Joshua Cogliati jjcogliati-r1 at yahoo dot com 2012-06-22 12:29:14 UTC --- (In reply to comment #3) (In reply to comment #2) I should have time to create a patch for this before 4.8 goes into stage 3. Do you think it needs a copyright assignment and if so what paperwork would you need from my employer? I am afraid so. See: https://www.gnu.org/prep/maintain/maintain.html#Legal-Matters Write to g...@gcc.gnu.org asking for the documents. There are several people there dealing with this matter. Also, let us know if you find any problems. I have written to to g...@gcc.gnu.org, and submitted the Employer disclaimer to my employer, but my employer is reluctant to sign it. I may not be able to create the patch because of that.
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #6 from Manuel López-Ibáñez manu at gcc dot gnu.org 2012-06-22 12:41:05 UTC --- (In reply to comment #5) I have written to to g...@gcc.gnu.org, and submitted the Employer disclaimer to my employer, but my employer is reluctant to sign it. I may not be able to create the patch because of that. Write to both g...@gcc.gnu.org and ass...@gnu.org to explain your case. They may be able to work out an arrangement. Sadly, this situation is fairly common.
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 Manuel López-Ibáñez manu at gcc dot gnu.org changed: What|Removed |Added CC||gcc at richardneill dot org --- Comment #4 from Manuel López-Ibáñez manu at gcc dot gnu.org 2012-06-07 14:03:45 UTC --- *** Bug 53603 has been marked as a duplicate of this bug. ***
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001
Manuel López-Ibáñez manu at gcc dot gnu.org changed:
What|Removed |Added
CC||manu at gcc dot gnu.org
--- Comment #1 from Manuel López-Ibáñez manu at gcc dot gnu.org 2012-04-16
11:41:24 UTC ---
(In reply to comment #0)
This is a request to add a new warning that warns on the subset of
-Wconversion
warnings that involve floating point numbers. For example, with
-Wfloat-conversion this would cause a warning:
Should it also warn for non-literals?
int foo(double x)
{
return x;
}
I think this could mostly be done by modifying gcc/c-family/c-common.c
unsafe_conversion_p to add the ability to only warn on conversions where
REAL_TYPE or REAL_CST are involved.
Yes, I think it should be easy to implement. You will also need to add a new
option to gcc/c.opt and enable -Wfloat-conversion with -Wconversion (grep for
OPT_Wimplict and how it handles its suboptions).
Unfortunately, I don't have time to work on this, and probably nobody else has,
so you could try to submit a patch: http://gcc.gnu.org/contribute.html
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001
--- Comment #2 from Joshua Cogliati jjcogliati-r1 at yahoo dot com 2012-04-16
12:16:45 UTC ---
Yes, it should also warn for non-constants, and also for other floating
decreases in accuracy such as:
float foo(double x) {
return x;
}
I should have time to create a patch for this before 4.8 goes into stage 3. Do
you think it needs a copyright assignment and if so what paperwork would you
need from my employer?
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53001 --- Comment #3 from Manuel López-Ibáñez manu at gcc dot gnu.org 2012-04-16 12:36:59 UTC --- (In reply to comment #2) I should have time to create a patch for this before 4.8 goes into stage 3. Do you think it needs a copyright assignment and if so what paperwork would you need from my employer? I am afraid so. See: https://www.gnu.org/prep/maintain/maintain.html#Legal-Matters Write to g...@gcc.gnu.org asking for the documents. There are several people there dealing with this matter. Also, let us know if you find any problems.
