sorenj added a comment.

Okay, but as you can see the majority of my test cases are intentionally false 
negatives `- -Wsign-conversion` triggers so often than many people don't use 
it. And, `unsigned x = 2;` does not trigger a sign conversion warning despite 
there being a conversion form 2 to 2u. This check is targeting a very specific 
but frequent case of functions that do not guard against containers that might 
be empty.

Regarding the false positives - I think you are focusing on the semantics of 
the fix-it which is literally a no-op. The code before and after may be just as 
wrong, but the salience of the implied conversion is a bit higher. Maybe that's 
not a good idea for a change that may be applied automatically, even if safe.

In short I'm not sure if you are objecting the principle here, or the 
implementation. Is this something that can be refined further? I understand the 
objection in the first case, but what about the second case makes you think 
this is not a good diagnostic and a false positive? It's the specific case I 
was targeting: the unprotected subtraction of a non-zero value from a 
potentially smaller value.

This check was used across our very large code base and was evaluated by 
examining the code by looking at the context where these warning fired. The 
vast majority were judged to be true positives. Unfortunately it's not possible 
to share those results. I will try to run this check on some fraction of the 
llvm code base and follow up.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71607/new/

https://reviews.llvm.org/D71607



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to