[PATCH] D13157: Teach -Wtautological-overlap-compare about enums

[George Burgess IV via cfe-commits]

george.burgess.iv added inline comments.

================
Comment at: lib/Analysis/CFG.cpp:54
@@ +53,3 @@
+    auto *DR = dyn_cast<DeclRefExpr>(E->IgnoreParenImpCasts());
+    if (DR == nullptr)
+      return nullptr;
----------------
aaron.ballman wrote:
> Please don't compare a pointer against nullptr with an equality operator. This can be simplified into:
> 
> ```
> if (const auto *DR = dyn_cast<>)
>   return isa<> ? "
> return nullptr;
> ```
Thanks for catching that!

================
Comment at: lib/Analysis/CFG.cpp:97
@@ +96,3 @@
+  // Currently we're only given EnumConstantDecls or IntegerLiterals
+  auto *C1 = cast<EnumConstantDecl>(cast<DeclRefExpr>(A)->getDecl());
+  auto *C2 = cast<EnumConstantDecl>(cast<DeclRefExpr>(B)->getDecl());
----------------
aaron.ballman wrote:
> Are you sure that A and B will only ever be DeclRefExprs? You dyn_cast elsewhere.
I'm 100% sure if my code matches my intent exactly. ;)

This helper exists solely to make `checkIncorrectLogicOperator` a bit cleaner -- by the time we call it there, we can assume `tryNormalizeBinaryOperator` succeeded on both BinOps, and `tryNormalizeBinaryOperator` only returns `DeclRefExpr`s (that contain `EnumConstantDecl`s) or `IntegerLiteral`s.

I see that this is unclear though, so I added a line to the function comment to make this constraint more explicit. If you think it would be better, I can also just manually inline this code in `checkIncorrectLogicOperator`.


http://reviews.llvm.org/D13157