This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH, PR 53001] Re: Patch to split out new warning flag for floating point conversion
- From: Dodji Seketeli <dodji at redhat dot com>
- To: Joshua J Cogliati <jrincayc at yahoo dot com>
- Cc: "Joseph S. Myers" <joseph at codesourcery dot com>, gcc-patches at gcc dot gnu dot org, jason at redhat dot com, manu at gcc dot gnu dot org
- Date: Mon, 28 Oct 2013 09:50:10 +0100
- Subject: Re: [PATCH, PR 53001] Re: Patch to split out new warning flag for floating point conversion
- Authentication-results: sourceware.org; auth=none
- References: <52554AA2 dot 50706 at yahoo dot com> <Pine dot LNX dot 4 dot 64 dot 1310092037370 dot 12619 at digraph dot polyomino dot org dot uk> <525AA8E5 dot 807 at yahoo dot com> <87zjqcp6jx dot fsf at redhat dot com> <Pine dot LNX dot 4 dot 64 dot 1310142333000 dot 19760 at digraph dot polyomino dot org dot uk> <52613001 dot 9030606 at yahoo dot com> <Pine dot LNX dot 4 dot 64 dot 1310181516380 dot 16441 at digraph dot polyomino dot org dot uk> <52673B3E dot 3070006 at yahoo dot com> <526D1E17 dot 80001 at yahoo dot com>
Joshua J Cogliati <firstname.lastname@example.org> 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
> Changelog for warn_float_patch_and_new_testcase.diff and
> 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.
> * c-family/c-common.h Adding conversion_safety enumeration
> and switching return type of unsafe_conversion_p
> * c-family/c.opt Adding new warning float-conversion and
> enabling it -Wconversion
> * doc/invoke.texi Adding documentation about
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
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
> 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.
Thank you for your time on this.