Quuxplusone added inline comments.
================
Comment at: lib/Sema/SemaExpr.cpp:10931
+ // Do not diagnose hexadecimal literals
+ if (ExprStr.find("0x") != llvm::StringRef::npos)
+ return;
----------------
Can you use `starts_with` (or the LLVM equivalent) in both of these cases?
It'll be faster and also more correct.
Hex and binary are handled up here on line 10927, but octal is handled down on
line 10955; why? Can't they be combined into one place in the code?
================
Comment at: lib/Sema/SemaExpr.cpp:10982
+ S.Diag(Loc, diag::note_xor_used_as_pow_silence) << ("0x2 ^ " + RHSStr);
+ } else if (LeftSideValue == 10 && RightSideIntValue != 0) {
+ std::string SuggestedValue;
----------------
Suppressing the warning specifically on `10 ^ 0` (which means `10`) seems like
an anti-feature.
================
Comment at: lib/Sema/SemaExpr.cpp:10988
+ } else {
+ SuggestedValue = "1" + std::string(RightSideIntValue, '0');
+ }
----------------
I suggest at least one unit test case that involves `10 ^ 100`, and considering
the user-friendliness of the resulting error message. How about suggesting
`"1e" + std::to_string(RightSideIntValue)` as the fixit in both cases?
================
Comment at: test/SemaCXX/warn-xor-as-pow.cpp:52
+ res = FOOBAR(2, 16);
+ res = EPSILON;
+ res = 0b10 ^ 16;
----------------
I still expect to see a warning either on this line, or on the line where the
macro is defined. I don't see why we'd want to be silent in this case.
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
