rtrieu accepted this revision. rtrieu added a comment. Fix the comments, then it should be ready to commit.
================ Comment at: lib/Analysis/CFG.cpp:49 @@ +48,3 @@ +tryNormalizeBinaryOperator(const BinaryOperator *B) { + auto TryTransformToIntOrEnumConstant = [](const Expr *E) -> const Expr * { + E = E->IgnoreParens(); ---------------- Why is this a lambda instead of a helper function? ================ Comment at: lib/Analysis/CFG.cpp:88 @@ +87,3 @@ +/// or DeclRefExprs (that have decls of type EnumConstantDecl) +static bool areExprTypesCompatible(const Expr *A, const Expr *B) { + // User intent isn't clear if they're mixing int literals with enum ---------------- Use E1 and E2 instead of A and B to match the naming of decls below. ================ Comment at: lib/Analysis/CFG.cpp:98 @@ +97,3 @@ + + // Currently we're only given EnumConstantDecls or IntegerLiterals + auto *C1 = cast<EnumConstantDecl>(cast<DeclRefExpr>(A)->getDecl()); ---------------- Change the comment to note that IntegerLiterals are handled above and only EnumConstantDecls are expected beyond this point. ================ Comment at: lib/Analysis/CFG.cpp:99-104 @@ +98,8 @@ + // Currently we're only given EnumConstantDecls or IntegerLiterals + auto *C1 = cast<EnumConstantDecl>(cast<DeclRefExpr>(A)->getDecl()); + auto *C2 = cast<EnumConstantDecl>(cast<DeclRefExpr>(B)->getDecl()); + + const TagDecl *E1 = TagDecl::castFromDeclContext(C1->getDeclContext()); + const TagDecl *E2 = TagDecl::castFromDeclContext(C2->getDeclContext()); + return E1 == E2; +} ---------------- There's a few extra casts in here, plus some blind conversions between types. Add your assumptions for the types in asserts. Also, DeclContext should use cast<> to get to Decl types. I recommend the following: ``` assert(isa<DeclRefExpr>(E1) && isa<DeclRefExpr>(E2)); auto *Decl1 = cast<DeclRefExpr>(E1)->getDecl(); auto *Decl2 = cast<DeclRefExpr>(E2)->getDecl(); assert(isa<EnumConstantDecl>(Decl1) && isa<EnumConstantDecl>(Decl2)); const DeclContext *DC1 = Decl1->getDeclContext(); const DeclContext *DC2 = Decl2->getDeclContext(); assert(isa<EnumDecl>(DC1) && isa<EnumDecl>(DC2)); return DC1 == DC2; ``` http://reviews.llvm.org/D13157 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits