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/



_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to