This is the mail archive of the
gcc-bugs@gcc.gnu.org
mailing list for the GCC project.
[Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
- From: "manu at gcc dot gnu.org" <gcc-bugzilla at gcc dot gnu dot org>
- To: gcc-bugs at gcc dot gnu dot org
- Date: Fri, 20 Sep 2013 20:27:06 +0000
- Subject: [Bug c/53001] -Wfloat-conversion should be available to warn about floating point errors
- Auto-submitted: auto-generated
- References: <bug-53001-4 at http dot gcc dot gnu dot org/bugzilla/>
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-patches@gcc.gnu.org. When
submitting CC dodji@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.c 2013-05-14 14:52:27.000000000
-0600
+++ gcc-4.8.1/gcc/c-family/c-common.c 2013-09-02 13:14:01.000000000 -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).