wristow added inline comments.
================ Comment at: lib/CodeGen/CGClass.cpp:1135 MemberExpr *ME2 = dyn_cast<MemberExpr>(RHS); - if (dyn_cast<FieldDecl>(ME2->getMemberDecl()) != Field) + if (!ME2 || dyn_cast<FieldDecl>(ME2->getMemberDecl()) != Field) return nullptr; ---------------- rjmccall wrote: > I would prefer: > > if (MemberExpr *ME2 = dyn_cast<MemberExpr>(RHS)) { > if (ME2->getMemberDecl() == Field) > return Field; > } > return nullptr; I see that change removes the `dyn_cast<FieldDecl>`. Was that intended, or an oversight? In terms of changing the code-structure, in code on it's own, I do like the approach you described. But in this case, there is a sequence of `if (<condition1>) return nullptr; ... if (conditionN) return nullptr; return Field;`. Then after the block containing that set of guarded `nullptr` returns with a final `return Field;`, there is a similar block. And then there is a third block with a similar set. So changing the structure in that way breaks that pattern. With that in mind, do you still want that change done? https://reviews.llvm.org/D29208 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits