Quuxplusone added a comment. Seems low-value at first glance, but I guess I can't argue with those Twitter and codesearch results.
Do you have codesearch results for `^2`? https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=%5E+2&search=Search Seems like a lot of false positives (and a lot of code comments turning up in codesearch??), but would be perhaps even more valuable to the kinds of users who would write `10^x` or `x^2`. What is going on in this case, and would a warning here be a false positive or a true positive? https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=10+%5E+7&search=Search ================ Comment at: lib/Sema/SemaExpr.cpp:10913 + if (ExprStr.find("0b") != llvm::StringRef::npos) + return; + if (S.getLangOpts().CPlusPlus) { ---------------- Why do you special-case `0b` prefix (or suffix — `0x0b` is also special-cased by this snippet), but you don't special-case octal or hexadecimal constants? I'm sure `x ^ 0x1234` will be a more common spelling than `x ^ 0b1001000110100`. ================ Comment at: lib/Sema/SemaExpr.cpp:10924 + + if (LeftSideValue == 2 || LeftSideValue == 10) { + llvm::APInt XorValue = LeftSideValue ^ RightSideValue; ---------------- Do you have metrics indicating that this line is an improvement over `if (true) {`? ================ Comment at: lib/Sema/SemaExpr.cpp:10959 + S.Diag(Loc, diag::note_xor_used_as_pow_silence) + << ("(" + LHSStr + ") ^ " + RHSStr); + } ---------------- I don't understand why parenthesizing one argument should silence the warning. Wouldn't it be more natural to suggest converting both arguments to hexadecimal or binary? That is, convert `10 ^ x` to `0xA ^ x`, or `2 ^ 16` to `0x2 ^ 0x10`. ================ Comment at: test/Sema/warn-xor-as-pow.c:38 + res = 0b10 ^ 16; + res = 2 ^ 0b100; + res = XOR(2, 16); ---------------- Please add test cases for `2 ^ 0x4` and `2 ^ 04` and `0x2 ^ 10` and `02 ^ 10`. ================ Comment at: test/Sema/warn-xor-as-pow.c:39 + res = 2 ^ 0b100; + res = XOR(2, 16); + unsigned char two = 2; ---------------- I don't understand why this line doesn't warn. Is it because the macro's name has the case-insensitive string `xor` in it? Is it because there is a macro involved at all? In either case, please add another test case for `res = FOOBAR(2, 16)`. Also `res = EPSILON` where `#define EPSILON 10^-300`. That seems to come up in the codesearch results a lot. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
