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

Reply via email to