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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
