[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.


================
Comment at: include/clang/Basic/DiagnosticGroups.td:508
 def Varargs : DiagGroup<"varargs">;
+def XorUsedAsPow : DiagGroup<"xor-used-as-pow">;
 
----------------
Does this match the naming that GCC ended up with?


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3309
+def warn_xor_used_as_pow_shift_count_overflow : Warning<
+  "result of '%0' is %1; did you mean '%2', but shift count >= width of type">,
+  InGroup<XorUsedAsPow>;
----------------
This is still a bad diagnostic, I'd rather see it fixed.


================
Comment at: lib/Sema/SemaExpr.cpp:10929
+    // Do not diagnose if xor keyword is used.
+    if (ExprStr.find("xor") != llvm::StringRef::npos)
+      return;
----------------
Doesn't this match any expression that contains `xor`? Put another way, I don't see `"xor"` used anywhere else under clang, what's usually done?


================
Comment at: lib/Sema/SemaExpr.cpp:10951
+  // Do not diagnose binary literals.
+  if (ExprStr.find("0b") != llvm::StringRef::npos)
+    return;
----------------
This ignores `0B`.


================
Comment at: lib/Sema/SemaExpr.cpp:10954
+  // Do not diagnose hexadecimal literals.
+  if (ExprStr.find("0x") != llvm::StringRef::npos)
+    return;
----------------
This ignores `0X`.


================
Comment at: lib/Sema/SemaExpr.cpp:10961
+    return;
+
+  if (LeftSideValue == 2 && RightSideIntValue >= 0) {
----------------
The above seems to mishandle user-defined literals. Unless the UDL contains `0x` or `0b` in them. I'd like to see this tested.


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

https://reviews.llvm.org/D63423