On 23-Jun-2015 02:27 PM, Daniel Marjamäki wrote:
I recommend a more specific name than LossOfSignChecker unless you want it to
have more warnings later. Right now, a name such as
LossOfSignInAssignmentChecker.cpp would be better imho.
I don't have immediate plans to add more warnings on this, but there
definitely are many more cases where we can lose the sign and
potentially cause runtime failures.
That said, I can rename it as you suggest since I do not have immediate
plans to address more cases.
Sure there can be issues with loss of sign. If we can warn about mistakes that
is awesome!
However personally I think it seems noisy to warn about:
unsigned char uc1 = -1;
As I see it, it is quite clear that the developer wants to assign -1 to uc1 and
that a cast is expected. On a normal machine the uc1 would get the value 255
and this is likely expected.
Sure, but wouldn't it be better to expect an explicit cast if the
developer really meant what he wrote?
Have you tried this checker on some real projects to see if the results are
good?
No, not really.
However, I hit this issue when moving a large software from x86 to
AArch64, where unfortunately, the 'sign' of a plain char changes,
causing us to lose days of effort in narrowing down the issue.
For example, the following representative program that works perfectly
on x86 would fail silently on AArch64:
1 #include <stdio.h>
2
3 const char data[] = { -1 };
4
5 int main() {
6 int value = data[0];
7 if (value != -1) {
8 printf("failed! value = %d\n", value);
9 return 1;
10 }
11 return 0;
12 }
Do you think narrowing down the checker to the above singular case would
be more appropriate? I would still think that while the 'unsigned char'
case you point out may be a grey area, an "integer" assignment with the
wrong sign would certainly not be a good programming practice.
Thanks,
Soumitra
http://reviews.llvm.org/D10634
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
