This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, PR 53001] Re: Patch to split out new warning flag for floating point conversion


Hello Joshua,

Joshua J Cogliati <jrincayc@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.


> 	* c-family/c-common.h Adding conversion_safety enumeration
> 	and switching return type of unsafe_conversion_p

Likewise.

> 	* 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


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.


> 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.

-- 
		Dodji


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]