Bug 51625 - -Wconversion should be on by default, or at least included in -Wall
Summary: -Wconversion should be on by default, or at least included in -Wall
Status: CLOSED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.4.2
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-19 17:09 UTC by Peter Fraenkel
Modified: 2012-04-30 10:02 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Fraenkel 2011-12-19 17:09:02 UTC
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
Comment 1 Andrew Pinski 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 .
Comment 2 Andrew Pinski 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.
Comment 3 Peter Fraenkel 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.
Comment 4 Yichao Zhou 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:)
Comment 5 Manuel López-Ibáñez 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
Comment 6 Peter Fraenkel 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.
Comment 7 Jonathan Wakely 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.