thakis added a comment.
In D66397#1642012 <https://reviews.llvm.org/D66397#1642012>, @Quuxplusone wrote:
> @thakis wrote:
>
> > What was the motivation for firing on more than bare literals?
>
> Well, fundamentally, there is no difference in code quality between any of
> these snippets:
>
> #define BIG 300
> double bigpower1() { return 10 ^ BIG; }
>
> static constexpr int BIG = 300; double bigpower2() { return 10 ^ BIG; }
>
> double bigpower3(int BIG) { return 10 ^ BIG; }
>
> double bigpower4(int BIG) { return 10 ^ (BIG+1); }
>
> minval -= 10 ^ -precision; //
> https://codesearch.isocpp.org/actcd19/main/q/qgis/qgis_2.18.28+dfsg-2/src/gui/editorwidgets/qgsrangewidgetwrapper.cpp
> intermediate = (str[offset] - '0') / (10 ^ lpc); //
> https://codesearch.isocpp.org/actcd19/main/p/pacemaker/pacemaker_1.1.18-2/lib/common/iso8601.c
> real_loop += (((unsigned int) *argv[4]+k) - 48) * 10^(strlen(argv[4]) -
> (k+1)); //
> https://codesearch.isocpp.org/actcd19/main/liba/libaria/libaria_2.8.0+repack-1.2/tests/testCOM.cpp
Thanks for the real-world examples, these are convincing.
> If you seriously mean "xor", then you shouldn't be writing code that looks
> like you meant "exponentiate." I don't think it matters whether the LHS or
> RHS come from literals, macros, constexpr variables, const variables,
> function calls, additions, subtractions, or what-have-you. It seems
> reasonable, when you write `10 ^ x` or `2 ^ x` in C++, for the compiler to
> tell you about it so you can go fix it. (Admittedly it looks like a slippery
> slope to `int square(int n) { return n ^ 2; }` — but we can be data-driven
> here. There are no true-positive instances of `^ 2` in our corpus, whereas
> there are many true positives of `10 ^` and `2 ^`.)
Right, I agree with the "data-driven" bit. Hence my point that if in practice
all true positives are from literals, then we should only warn on literals. The
examples you posted show that that's not the case.
I now agree that it makes sense to warn when the operands are macros or
variables.
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
