[PATCH] Loss of Sign Checker

[Soumitra Chatterjee]

In http://reviews.llvm.org/D10634#192481, @danielmarjamaki 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?

In http://reviews.llvm.org/D10634#192482, @danielmarjamaki 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.


http://reviews.llvm.org/D10634

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/