| Message ID | 557C3F76.2000901@netcologne.de |
|---|---|
| State | New |
| Headers | show |
*ping* https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00966.html > Hello world, > > the attached patch emits a warning for constant integer division. > While correct according to the standard, I cannot really think > of a legitimate reason why people would want to write 3/5 where > they could have written 0 , so my preference would be to put > this under -Wconversion (like in the attached patch). > > However, I am open to discussion on that. It is easy enough to > change. > > Regression-tested. Opinions? Comments? Would somebody rather > have -Wconversion-extra? OK for trunk?
On Sun, Jun 21, 2015 at 4:57 PM, Thomas Koenig <tkoenig@netcologne.de> wrote: > *ping* > > https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00966.html > > >> Hello world, >> >> the attached patch emits a warning for constant integer division. >> While correct according to the standard, I cannot really think >> of a legitimate reason why people would want to write 3/5 where >> they could have written 0 , so my preference would be to put >> this under -Wconversion (like in the attached patch). >> >> However, I am open to discussion on that. It is easy enough to >> change. >> >> Regression-tested. Opinions? Comments? Would somebody rather >> have -Wconversion-extra? OK for trunk? I'm a bit uncomfortable about this. IIRC I have code where I'm iterating over some kind of grid, and I'm using integer division and relying on truncation to calculate array indices. I can certainly imagine that others have used it as well, and even that it's not a particularly uncommon pattern. Furthermore, I think it's confusing that you have it under -Wconversion, as there is no type conversion going on. -Winteger-truncation maybe? Any other opinions?
On 06/23/2015 01:36 AM, Janne Blomqvist wrote: > On Sun, Jun 21, 2015 at 4:57 PM, Thomas Koenig <tkoenig@netcologne.de> wrote: >> *ping* >> >> https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00966.html >> >> >>> Hello world, >>> >>> the attached patch emits a warning for constant integer division. >>> While correct according to the standard, I cannot really think >>> of a legitimate reason why people would want to write 3/5 where >>> they could have written 0 , so my preference would be to put >>> this under -Wconversion (like in the attached patch). >>> >>> However, I am open to discussion on that. It is easy enough to >>> change. >>> >>> Regression-tested. Opinions? Comments? Would somebody rather >>> have -Wconversion-extra? OK for trunk? > > I'm a bit uncomfortable about this. IIRC I have code where I'm > iterating over some kind of grid, and I'm using integer division and > relying on truncation to calculate array indices. I can certainly > imagine that others have used it as well, and even that it's not a > particularly uncommon pattern. > > Furthermore, I think it's confusing that you have it under > -Wconversion, as there is no type conversion going on. > -Winteger-truncation maybe? > > Any other opinions? > I am not sure it is worth warning about. I don't think it justifies its own compilation warning option. I have no objection to -Wconversion, 3/5 being converted to zero in a sense. It would help users catch a missing decimal point when they meant 3./5 Regards, Jerry Jerry
Index: arith.c =================================================================== --- arith.c (Revision 224450) +++ arith.c (Arbeitskopie) @@ -733,6 +733,15 @@ gfc_arith_divide (gfc_expr *op1, gfc_expr *op2, gf mpz_tdiv_q (result->value.integer, op1->value.integer, op2->value.integer); + + if (warn_conversion) + { + char *p; + p = mpz_get_str (NULL, 10, result->value.integer); + gfc_warning_now (OPT_Wconversion, "Integer division simplifies " + "to constant %qs at %L", p, &op1->where); + free (p); + } break; case BT_REAL: