-Wconversion generates false warnings for the following clean code: { char c = 1; char c2 = 2; // warning: conversion to ‘char’ from ‘int’ may alter its value c >>= 1; c += (char) 1; c += c2; c = ~c2; }
Theses are not false warnings: c >>= 1; is really c = (int)c >> 1; c += (char) 1; c = (int)c + (int)(char)1; c += c2; c = (int)c + (int) c2; c = ~c2; c = ~(int)c2; Only the last one might be a false warning depending on if c2 is negative or unsigned or not. The rest are correct because of the way C/C++ define arithmetic operations and automatic promotions.
Andrew, what you say is true, but in this case the warning is not very useful. I'd prefer to warn only when the operator is larger than the target of the assignment. I would like to hear other opinions.
Joseph, could you comment on this?
Manu, I agree that these warnings are in some sense technically correct but they are not useful. They can't point to any actual bug. I guess would be that if every input to the expression has the size of the target of the expression, then the warning should be suppressed.
Then, let's keep this around as an enhancement request.
(In reply to comment #1) > Theses are not false warnings: > c >>= 1; > > is really c = (int)c >> 1; They are false warnings. The implicit conversion cannot alter the value.
(In reply to comment #5) > Then, let's keep this around as an enhancement request. > I think this is actually a bug as the specification of the warning is: Warn for implicit conversions that may alter a value. This is not the case here.
For: c += (char) 1; The value can change as you have a wrapping if c is CHAR_MAX. Likewise with: c += c2;
Sure, it can wrap, but -Wconversion is not for wrapping warnings.
Subject: Re: -Wconversion: do not warn for operands not larger than target type On Wed, 15 Jul 2009, ian at airs dot com wrote: > Sure, it can wrap, but -Wconversion is not for wrapping warnings. It's for warnings about implicit conversions changing a value; the arithmetic, in a wider type (deliberately or otherwise), does not wrap, but the value gets changed by the implicit conversion back to char. If the user had explicit casts to int in their arithmetic, there could be no doubt that the warning is appropriate.
(In reply to comment #8) > For: > > c += (char) 1; > > The value can change as you have a wrapping if c is CHAR_MAX. > > Likewise with: > c += c2; > The value cannot change even if an overflow occurs. { unsigned char c = 0xff; c += 1; // c is 0 here c = 0xff; c = (unsigned int)c + 1; // c is 0 here }
Except it does alter its value from 0x100 to 0x00 :).
Or rather from SCHAR_MAX + 1 to SCHAR_MIN :). Since it is 0x7F + 1 == (int)0x80. So we have a negative value now from a positive value.
(In reply to comment #13) > Or rather from SCHAR_MAX + 1 to SCHAR_MIN :). Since it is 0x7F + 1 == > (int)0x80. So we have a negative value now from a positive value. > This occurs regardless of the implicit conversion. The implicit conversion cannot alter the result. Therefore, the warning is false in this case too. { char c = 0x7f; ++c; // c is 0x80 here c = 0x7f; c = (int)c + 1; // c is 0x80 here }
Patch: http://gcc.gnu.org/ml/gcc-patches/2009-07/msg01179.html
*** Bug 41274 has been marked as a duplicate of this bug. ***
The patch was rejected but it may be accepted by using a new -Wno-* option to disable these warnings. Perhaps -Wno-conversion-after-promotion? Suggestions are welcome.
(In reply to comment #17) > The patch was rejected but it may be accepted by using a new -Wno-* option to > disable these warnings. Perhaps -Wno-conversion-after-promotion? > > Suggestions are welcome. > -Wconversion should be fixed to work as specified. No warning should be generated for the following code: char c = 2; // warning: conversion to ‘char’ from ‘int’ may alter its value c >>= 1;
*** Bug 50519 has been marked as a duplicate of this bug. ***
*** Bug 52703 has been marked as a duplicate of this bug. ***
Why was this patch rejected, and is there a way to improve it so that obviously safe cases (such as PR52703) are not warned about without having to specify a '-Wno-' option? Yes, according to the standard (C++03 5/9), calculations done on variables smaller than int are first promoted to int, then the calculation is done, then the value is converted back to the target size. However, C++03 1.8/3, the "as-if rule", states that it the program can't tell the difference, you can do whatever you want (see my answer to a similar question on Stack Overflow here: http://stackoverflow.com/questions/5563000/implicit-type-conversion-rules-in-c-operators/8935697#8935697). The C++ standard does not require a diagnostic for this, and the apparent behavior is identical. Therefore, there can be no appeals to the C++ standard on the behavior of the warning. Because this is a purely option warning for which gcc defines the rules, we should define it to be useful. If gcc can prove that all of the values are greater than 0 (for instance, if all of the values are unsigned prior to implicit promotion or are positive integral constant expressions), then there is no possibility of having a negative value. Thanks to signed integer overflow being undefined, there is no risk of creating a negative value that way, either. Therefore, we should not warn. Having to manually say "Turn off stuff that no one could ever possibly want to see" seems like a sure way to make this warning useless.
I agree with David. My code has many instances of things like a+=2 where a is a char and this makes -Wconversion useless to me. It's too bad, since it would really be helpful as I am in the process of changing some data types in the code. If someone really thinks there should be a warning for this behavior, how about adding a separate -Wchar-arithmetic warning which warns on all char arithmetic and see how much people use it.
(In reply to comment #22) > If someone really thinks there should be a warning for this behavior, how about > adding a separate > -Wchar-arithmetic > warning which warns on all char arithmetic and see how much people use it. I think adding a new option Wconversion-after-promotion that covers all cases where the conversion occurs to the same type that was originally promoted from would be the most appropriate. I have a feeling that it should not be hard to implement, but I am not sure how, and I have many other things I would like to do first. So, please David, Wes, photon, give it a try. The code is in c-family/c-common.c (conversion_warning).
Well, please fix this. My cases are instructions like unsigned short x = 100; unsigned short y = 200; // gives a warning x += y;
Today I encountered a similar case as follows. The conversion warning by gcc is not true as right-shifting an unsigned short decreases the value. BTW clang does not emit warnings for this code snippet. $: cat s1.c unsigned short fn(unsigned short a, int right) { return a >> right; } $: gcc-trunk -c -Wconversion s1.c s1.c: In function ‘fn’: s1.c:2:3: warning: conversion to ‘short unsigned int’ from ‘int’ may alter its value [-Wconversion] return a >> right; ^ $: clang-trunk -c -Wconversion s1.c $:
*** Bug 61029 has been marked as a duplicate of this bug. ***
I encountered a similar problem with -Wconversion. This option is useless if it would warn for cases like short int i; i += 5; I concede Andrew's point about this being technically a place to warn, but in practice, i would like for it to warn only when the target is a "smaller" than any of the inputs.
*** Bug 67764 has been marked as a duplicate of this bug. ***
This came up on the gcc mailing list again here: https://gcc.gnu.org/ml/gcc/2018-09/msg00076.html
New patch: https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00624.html
*** Bug 91312 has been marked as a duplicate of this bug. ***
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:c77074d05691053ee7347d9e44ab89b3adb23fb1 commit r10-6126-gc77074d05691053ee7347d9e44ab89b3adb23fb1 Author: Jason Merrill <jason@redhat.com> Date: Tue Jan 7 12:20:26 2020 -0500 PR c++/40752 - useless -Wconversion with short +=. This is a longstanding issue with lots of duplicates; people are not interested in a -Wconversion warning about mychar += 1. So now that warning depends on -Warith-conversion; otherwise we only warn if operands of the arithmetic have conversion issues. * c.opt (-Warith-conversion): New. * c-warn.c (conversion_warning): Recurse for operands of operators. Only warn about the whole expression with -Warith-conversion.
Fixed for GCC 10.
(In reply to Jason Merrill from comment #33) > Fixed for GCC 10. Hi, I've noticed that the new tests fail: c-c++-common/Wconversion-pr40752.c -Wc++-compat (test for warnings, line 34) c-c++-common/Wconversion-pr40752.c -Wc++-compat (test for warnings, line 38) c-c++-common/Wconversion-pr40752.c -Wc++-compat (test for warnings, line 40) c-c++-common/Wconversion-pr40752.c -Wc++-compat (test for warnings, line 42) c-c++-common/Wconversion-pr40752.c -Wc++-compat (test for excess errors) c-c++-common/Wconversion-pr40752a.c -Wc++-compat (test for warnings, line 17) c-c++-common/Wconversion-pr40752a.c -Wc++-compat (test for warnings, line 19) c-c++-common/Wconversion-pr40752a.c -Wc++-compat (test for warnings, line 40) c-c++-common/Wconversion-pr40752a.c -Wc++-compat (test for warnings, line 42) As a side note, the commit r10-6126-gc77074d05691053ee7347d9e44ab89b3adb23fb1 lacks ChangeLog entries for doc and testsuite, and the commit message does not mention the doc and testsuite changes either.
I see even more failures on powerpc64 both BE and LE: > FAIL: c-c++-common/Wconversion-pr40752.c -Wc++-compat (test for warnings, line 34) > FAIL: c-c++-common/Wconversion-pr40752.c -Wc++-compat (test for warnings, line 38) > FAIL: c-c++-common/Wconversion-pr40752.c -Wc++-compat (test for warnings, line 40) > FAIL: c-c++-common/Wconversion-pr40752.c -Wc++-compat (test for warnings, line 42) > FAIL: c-c++-common/Wconversion-pr40752.c -Wc++-compat (test for excess errors) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++14 (test for warnings, line 34) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++14 (test for warnings, line 36) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++14 (test for warnings, line 38) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++14 (test for warnings, line 40) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++14 (test for warnings, line 42) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++17 (test for warnings, line 34) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++17 (test for warnings, line 36) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++17 (test for warnings, line 38) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++17 (test for warnings, line 40) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++17 (test for warnings, line 42) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++2a (test for warnings, line 34) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++2a (test for warnings, line 36) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++2a (test for warnings, line 38) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++2a (test for warnings, line 40) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++2a (test for warnings, line 42) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++98 (test for warnings, line 34) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++98 (test for warnings, line 36) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++98 (test for warnings, line 38) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++98 (test for warnings, line 40) > FAIL: c-c++-common/Wconversion-pr40752.c -std=gnu++98 (test for warnings, line 42) > FAIL: c-c++-common/Wconversion-pr40752a.c -Wc++-compat (test for warnings, line 17) > FAIL: c-c++-common/Wconversion-pr40752a.c -Wc++-compat (test for warnings, line 19) > FAIL: c-c++-common/Wconversion-pr40752a.c -Wc++-compat (test for warnings, line 40) > FAIL: c-c++-common/Wconversion-pr40752a.c -Wc++-compat (test for warnings, line 42) > FAIL: c-c++-common/Wconversion-pr40752a.c -std=gnu++14 (test for warnings, line 12) > FAIL: c-c++-common/Wconversion-pr40752a.c -std=gnu++14 (test for warnings, line 17) > FAIL: c-c++-common/Wconversion-pr40752a.c -std=gnu++14 (test for warnings, line 19) > FAIL: c-c++-common/Wconversion-pr40752a.c -std=gnu++14 (test for warnings, line 36) > FAIL: c-c++-common/Wconversion-pr40752a.c -std=gnu++17 (test for warnings, line 12) > FAIL: c-c++-common/Wconversion-pr40752a.c -std=gnu++17 (test for warnings, line 17) > FAIL: c-c++-common/Wconversion-pr40752a.c -std=gnu++17 (test for warnings, line 19) > FAIL: c-c++-common/Wconversion-pr40752a.c -std=gnu++17 (test for warnings, line 36) > FAIL: c-c++-common/Wconversion-pr40752a.c -std=gnu++2a (test for warnings, line 12) > FAIL: c-c++-common/Wconversion-pr40752a.c -std=gnu++2a (test for warnings, line 17) > FAIL: c-c++-common/Wconversion-pr40752a.c -std=gnu++2a (test for warnings, line 19) > FAIL: c-c++-common/Wconversion-pr40752a.c -std=gnu++2a (test for warnings, line 36) > FAIL: c-c++-common/Wconversion-pr40752a.c -std=gnu++98 (test for warnings, line 12) > FAIL: c-c++-common/Wconversion-pr40752a.c -std=gnu++98 (test for warnings, line 17) > FAIL: c-c++-common/Wconversion-pr40752a.c -std=gnu++98 (test for warnings, line 19) > FAIL: c-c++-common/Wconversion-pr40752a.c -std=gnu++98 (test for warnings, line 36)
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:55b7df8bfb12938e7716445d4e2dc0d2ddf44bac commit r10-6157-g55b7df8bfb12938e7716445d4e2dc0d2ddf44bac Author: Jason Merrill <jason@redhat.com> Date: Wed Jan 22 14:21:06 2020 -0500 c-family: Fix problems with blender and PPC from PR 40752 patch. blender in SPEC is built with -funsigned-char, which is also the default on PPC, and exposed -Wsign-conversion issues that weren't seen by the x86_64 testsuite. In blender we were complaining about operands to an expression that we didn't't previously complain about as a whole. So only check operands after we check the whole expression. Also, to fix the PR 40752 testcases on -funsigned-char targets, don't consider -Wsign-conversion for the second operand of PLUS_EXPR, especially since fold changes "x - 5" to "x + (-5)". And don't use SCHAR_MAX with plain char. PR testsuite/93391 - PR 40752 test fails with unsigned plain char. PR c++/40752 * c-warn.c (conversion_warning): Check operands only after checking the whole expression. Don't check second operand of + for sign.
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:6d00f052ef209bacdd93f503b0c5fb428cc6c434 commit r10-6181-g6d00f052ef209bacdd93f503b0c5fb428cc6c434 Author: Jason Merrill <jason@redhat.com> Date: Thu Jan 23 10:37:18 2020 -0500 c-family: One more 40752 tweak for unsigned char. My last patch didn't fix all the failures on unsignd char targets. We were missing one warning because by suppressing -Wsign-conversion for the second operand of + we missed an overflow that we want to warn about, and we properly don't warn about unsigned / or %. PR testsuite/93391 - PR 40752 test fails with unsigned plain char. * c-warn.c (conversion_warning): Change -Wsign-conversion handling. * lib/target-supports.exp (check_effective_target_unsigned_char): New.