Bug 40733 - No warning is issued when an implicit conversion can lead to a data loss
Summary: No warning is issued when an implicit conversion can lead to a data loss
Status: RESOLVED WORKSFORME
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.4.0
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-13 15:53 UTC by photon
Modified: 2009-07-14 18:11 UTC (History)
2 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 photon 2009-07-13 15:53:05 UTC
The following code contains an implicit integer conversion involving a data loss. There is no warning issued even when -Wall is specified.

int main()
{
	int i = 0x12345678;
	char c = i;  // No warning is issued here
}
Comment 1 Andrew Pinski 2009-07-13 15:56:05 UTC
-Wconversion needs to be supplied.
Comment 2 photon 2009-07-13 20:57:09 UTC
-Wall has a very misleading name and should probably be changed to match the MSC behaviour (enable all warnings available).
Comment 3 Andrew Pinski 2009-07-13 21:42:38 UTC
(In reply to comment #2)
> -Wall has a very misleading name and should probably be changed to match the
> MSC behaviour (enable all warnings available).

"-Wall
This enables all the warnings about constructions that some users consider questionable, and that are easy to avoid (or modify to prevent the warning), even in conjunction with macros. "

hmm, I don't know a better name for -Wall really.  Since it enables all warnings that are questionable and easy to avoid.  implicit integer conversion is easy to avoid but is used all the place in normal C/C++ code. and with integer promotion happening with simple stuff like a + b, some folks will have a hard time to understand that happens which is why it is not enabled with -Wall.

Plus there are style warnings not included in -Wall.  I really doubt you want style warnings enabled with -Wall :).
Comment 4 photon 2009-07-14 08:37:04 UTC
(In reply to comment #3)
> and with  integer promotion happening with simple stuff like a + b,
> some folks will have a hard time to understand that happens which
> is why it is not enabled with -Wall.

The warning is issued only for rare conversions involving a potential data loss which should always be explicit.

> 
> Plus there are style warnings not included in -Wall.  I really doubt you want
> style warnings enabled with -Wall :).
> 

-Wall should be a real -Wall. Now it does not include important warnings like those generated by -Wconversion (potential implicit data loss).

GCC should use numerical warning levels. The current -Wall should be renamed to, e.g., -W3 and the new -Wall option should enable all warnings as the name says.

Comment 5 Jakub Jelinek 2009-07-14 09:24:55 UTC
-Wconversion hits extremely often, it is definitely not a warning that can or should be enabled in -Wall, nor in -W.
Comment 6 Manuel López-Ibáñez 2009-07-14 14:04:18 UTC
There is a FAQ for the new Wconversion:

http://gcc.gnu.org/wiki/NewWconversion#faq

It should answer users' concerns.

As for Wall, we have users requesting less warnings from Wall and users requesting more. We try to find a balance. But you are free to suggest that existing warnings be moved to Wall or Wextra. However, first try compiling a few big projects (like the kernel, QT libraries and KDE) and check that the suggested warning only finds real bugs. Thanks.
Comment 7 photon 2009-07-14 15:13:52 UTC
(In reply to comment #5)
> -Wconversion hits extremely often, it is definitely not a warning that can or
> should be enabled in -Wall, nor in -W.
> 

The fact that "it hits often" should not be an argument against including it in the warning level -Wall. Implicit conversion to a smaller type occurs infrequently and should always be explicit to avoid hard to find bugs.
Comment 8 photon 2009-07-14 15:31:27 UTC
(In reply to comment #6)
> 
> As for Wall, we have users requesting less warnings from Wall and users
> requesting more. We try to find a balance. But you are free to suggest that
> existing warnings be moved to Wall or Wextra. 

There should be no discussion about adding or removing anything from the -Wall level. It should include all warnings available. The current -Wall should be renamed to something like -W3 to prevent misleading users. Each warning should be assigned a warning level depending on the severity. The user should normally only need to specify the desired warning level. -Wall should be used only occasionally to make sure no warning is missed.


> However, first try compiling a
> few big projects (like the kernel, QT libraries and KDE) and check that the
> suggested warning only finds real bugs. Thanks.
> 

Are you suggesting that -Wconversion does not find real bugs?

Comment 9 Andrew Pinski 2009-07-14 16:47:04 UTC
(In reply to comment #8)
>  The current -Wall should be
> renamed to something like -W3 to prevent misleading users. 

It only misleading users who don't read the documentation.  The all part is described in the documentation as:
"This enables all the warnings about constructions that some users consider
questionable, and that are easy to avoid (or modify to prevent the warning),
even in conjunction with macros. "

So the all is accurate in the sense it is all useful warnings; but it is not all warnings.  

Most folks don't want -Wformat-nonliteral and -Wformat-security in their warning set.  Or even -Wmissing-include-dirs or even -Winvalid-pch which will warn if you have a PCH done for -O0 and one done for -O2 and you are compiling at -O2 (and yes this happens all the time).

So your definition of -Wall is not very useful at all and will be even more misleading to users or why the warnings are happening.
Comment 10 photon 2009-07-14 18:11:12 UTC
(In reply to comment #9)
> 
> So your definition of -Wall is not very useful at all and will be even more
> misleading to users or why the warnings are happening.
> 

MSC's /Wall enables all warnings and I don't find it misleading at all.

http://msdn.microsoft.com/en-us/library/thxezb7y(VS.71).aspx