[Bug c++/51625] New: -Wconversion should be on by default, or at least included in -Wall

[Bug c++/51625] New: -Wconversion should be on by default, or at least included in -Wall

[pnf at podsnap dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51625

             Bug #: 51625
           Summary: -Wconversion should be on by default, or at least
                    included in -Wall
    Classification: Unclassified
           Product: gcc
           Version: 4.4.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: pnf@podsnap.com


Sometime after 4.1.2, conversion errors stopped being reported by default, and
in fact don't even show up with -Wall and -Wextra.  My opinion is that they
should be reported by default unless explicitly suppressed, but failing that,
at least they should be included in -Wall.

% cat boffo.C
#include <math.h>
#include <stdlib.h>
#include <stdio.h>
#include <cmath>
//using namespace std;
int main() {
  double x = abs(3.5), y= fabs(3.5);
  printf("%g %g\n",x,y);
  return 0;
}
% # No warning in 4.2.2 or 4.4.2
% /sbcimp/run/pd/gcc/32-bit/4.2.2/bin/g++ -Wextra -Wall  boffo.C
% /sbcimp/run/pd/gcc/32-bit/4.4.2/bin/g++ -Wextra -Wall  boffo.C
% # 4.1.2 warns, even without any option -W flags
% /sbcimp/run/pd/gcc/32-bit/4.1.2/bin/g++ -Wextra -Wall  boffo.C
boffo.C: In function 'int main()':
boffo.C:7: warning: passing 'double' for argument 1 to 'int abs(int)'
% /sbcimp/run/pd/gcc/32-bit/4.1.2/bin/g++ boffo.C
boffo.C: In function 'int main()':
boffo.C:7: warning: passing 'double' for argument 1 to 'int abs(int)'
# 4.2.2 and 4.4.2 warn correctly when -Wconversion turned on
% /sbcimp/run/pd/gcc/32-bit/4.2.2/bin/g++ -Wextra -Wall -Wconversion  boffo.C
boffo.C: In function 'int main()':
boffo.C:7: warning: passing 'double' for argument 1 to 'int abs(int)'
% /sbcimp/run/pd/gcc/32-bit/4.4.2/bin/g++ -Wextra -Wall -Wconversion  boffo.C
boffo.C: In function 'int main()':
boffo.C:7: warning: conversion to 'int' alters 'double' constant value

[Bug c++/51625] -Wconversion should be on by default, or at least included in -Wall

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51625

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-12-30 06:16:22 UTC ---
I don't we (GCC) want -Wconversion turned on by -Wall or by default.

See http://gcc.gnu.org/wiki/NewWconversion .

[Bug c++/51625] -Wconversion should be on by default, or at least included in -Wall

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51625

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-12-30 06:17:21 UTC ---
Actually we document why we don't want this by default in that wiki page:
Why Wconversion is not enabled by -Wall or at least by -Wextra?

Implicit conversions are very common in C. This tied with the fact that there
is no data-flow in front-ends (see next question) results in hard to avoid
warnings for perfectly working and valid code. Wconversion is designed for a
niche of uses (security audits, porting 32 bit code to 64 bit, etc.) where the
programmer is willing to accept and workaround invalid warnings. Therefore, it
shouldn't be enabled if it is not explicitly requested.

[Bug c++/51625] -Wconversion should be on by default, or at least included in -Wall

[pnf at podsnap dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51625

--- Comment #3 from Peter Fraenkel <pnf at podsnap dot com> 2011-12-30 17:14:38 UTC ---
Passing a double to a function expecting an int is very likely a bug and
definitely bad style.  The fact that cmath contains overloaded math functions
with the same names as int math function in stdlib.h makes inadvertent misuse a
real danger.  Ideally, this particular conversion would cause a separate
warning, but absent that feature it is safer to warn on any conversion.

[Bug c++/51625] -Wconversion should be on by default, or at least included in -Wall

[broken.zhou at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51625

Yichao Zhou <broken.zhou at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |broken.zhou at gmail dot
                   |                            |com

--- Comment #4 from Yichao Zhou <broken.zhou at gmail dot com> 2012-04-29 12:16:54 UTC ---
I run into this problem today too.  Why gcc do not warning by default when we
assign a double to a int?  This is even not warned by Wall and Wextra.  If you
google the internet, you will find there are many users asking this question.

> This tied with the fact that there is no data-flow in front-ends (see next question) results in hard to avoid warnings for perfectly working and valid code.

This is not convincing for me.  Since I think you should do these conversion
explicitly to tell others you know what you are doing.

I hope that there is a day that the developer can change their mind on this
question:)

[Bug c++/51625] -Wconversion should be on by default, or at least included in -Wall

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51625

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |manu at gcc dot gnu.org

--- Comment #5 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-04-29 21:56:23 UTC ---
(In reply to comment #4)
> I run into this problem today too.  Why gcc do not warning by default when we
> assign a double to a int?  This is even not warned by Wall and Wextra.  If you
> google the internet, you will find there are many users asking this question.

So, what do you think should happen in this case?
 double d = 1.0;
  int i;
  i = d; 

Should we warn?

> > This tied with the fact that there is no data-flow in front-ends (see next question) results in hard to avoid warnings for perfectly working and valid code.
> 
> This is not convincing for me.  Since I think you should do these conversion
> explicitly to tell others you know what you are doing.

-Wconversion warns for a lot of things that are perfectly ok. If you compile
any large project with it (like the Linux kernel), there are lots of invalid
warnings. Moreover, there are cases where it is difficult to avoid the warning.
See PR 39170.

> I hope that there is a day that the developer can change their mind on this
> question:)

If the accuracy of Wconversion improves enough... One prerequisite is to find a
fix for PR 39170. Another would be to find a way to avoid warning for the case
above. There are more PRs related to Wconversion. You can help to fix them.
Please, see: http://gcc.gnu.org/contribute.html

[Bug c++/51625] -Wconversion should be on by default, or at least included in -Wall

[pnf at podsnap dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51625

Peter Fraenkel <pnf at podsnap dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #6 from Peter Fraenkel <pnf at podsnap dot com> 2012-04-30 09:11:17 UTC ---
I, personally, would want to be warned on any implicit down-conversion between
native types. In cases where "down" is ambiguous (ie. with non-native classes),
the warning should be specially requested, as it could be generally annoying. 
I would actually argue that xxx(int)/fxxx(double) is such a minefield that it
deserves special case warnings.

[Bug c++/51625] -Wconversion should be on by default, or at least included in -Wall

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51625

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-04-30 10:02:24 UTC ---
FWIW int{1.0} is an invalid narrowing conversion in C++11, even when the double
value is known to be exactly representable as int. Without -pedantic-errors or
-Werror=narrowing G++ only issues a warning, not an error.

N.B. we don't use the CLOSED status in GCC's bugzilla, so setting it just makes
it less likely people will find the bug if they search for it with
status=RESOLVED and don't think to include CLOSED reports.