rjmccall added inline comments.
================ Comment at: clang/lib/CodeGen/CGExprAgg.cpp:688 + + op = castE; } ---------------- ekatz wrote: > rjmccall wrote: > > ekatz wrote: > > > rjmccall wrote: > > > > I liked the structure of the old code better, in case we want to look > > > > through other kinds of expressions. Please just add `op = > > > > castE->getSubExpr()` before the `continue`. > > > I see your point. I'll change that. > > > Though I must say that the old structure is a little strange with the > > > `return nullptr` in the end of the loop... > > Oh, you know, there's also an `IgnoreParenNoopCasts` that we could just use > > instead of this loop if we're willing to ignore other possible expressions > > we might want to look through. > I am not familiar enough with clang's code to make the decision to remove > this function. > Personally I wanted to fix the bug, and I guess this function is here for a > reason..? I don't think there's anything here really worth keeping. Please keep the function around but replace its body with something like: ``` op = op->IgnoreParenNoopCasts(); if (auto castE = dyn_cast<CastExpr>(op)) { if (castE->getCastKind() == kind) return castE->getSubExpr(); } return nullptr; ``` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78098/new/ https://reviews.llvm.org/D78098 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits