ekatz marked an inline comment as done.
ekatz added inline comments.
================
Comment at: clang/lib/CodeGen/CGExprAgg.cpp:688
+
+ op = castE;
}
----------------
ekatz wrote:
> rjmccall wrote:
> > 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;
> > ```
> Sorry, I, kinda, misunderstood you suggestion before.
> This is a really good implementation, but as you said, this function should,
> and I quote from its description, "look through various unimportant
> expressions". So I guess it is better to leave it this way (with the loop),
> as it is already in the base code. What do you say?
I'll counter myself. (COVID-19 + Kids = TOTAL Tiredness)
This still gives us the opportunity to add "various unimportant expressions".
I'll change to the suggested implementation.
Thanks! :)
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
