thakis added a comment.

> I don't understand how you draw that conclusion, but the reason behind why we 
> went with that as a way to silence the diagnostic is because using the prefix 
> acts as a signal that the developer wants to do bit manipulation more than 
> just a decimal literal does. It's not ideal because it's largely a hidden way 
> to silence diagnostics, but it's also not the only place where we do that 
> sort of thing (like surrounding something in parens to silence a diagnostic, 
> or casting something to void, etc).

I think adding parens and casts are fairly well-understood to suppress 
warnings. Other things, like changing the spelling of literals, aren't. C++ is 
already a fairly baroque language, and every new thing that makes your code 
behave subtly different with a different compiler makes it more so. So I think 
we should be careful to not attach semantic meaning in surprising ways. 
Warnings have to carry their weight.

>> Maybe a better fixit is to suggest `xor` instead of `^` which also 
>> suppresses the warning and which is imho a bit less weird. (`xor` is so rare 
>> that it's still a bit weird though.)
> 
> I don't think suggesting a fixit for `xor` is a good solution. For starters, 
> you need a header included in order to use that for C. As a textual 
> suggestion "; use the 'xor' alternative token to perform an exclusive OR" 
> wouldn't bother me too much though.

I don't like xor all that much either :)

I do like Richard's suggestion.

Quuxplusone: Saying "the chromium devs don't want to change their code" is 
missing the mark. I'm happy to change that file, but that doesn't really help. 
There are hundreds of people committing hundreds of changes to the code base 
every day. They have varying levels of C++ proficiency (like everywhere). It's 
not just about getting our code to compile today, but also about the long-term 
effects of the warnings we have enabled. We have to be careful about which 
warnings we enable, and this one is maybe not quite there yet on a 
usefulness/overhead tradeoff evaluation. I'd argue that Chromium is fairly 
typical here and that clang's default warning set should follow similar 
considerations.

I looked through D63423 <https://reviews.llvm.org/D63423> and didn't find an 
evaluation of false / true positive rates when lhs and rhs are just literals vs 
when they're identifiers / macros. Glancing through 
https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=10+%5E&search=Search , I 
couldn't find a single true positive where lhs or rhs weren't a bare literal 
(but I didn't look super carefully). Did I just miss that in D63423 
<https://reviews.llvm.org/D63423>? What was the motivation for firing on more 
than bare literals?


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

https://reviews.llvm.org/D66397



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

Reply via email to