lebedev.ri added a comment.
In https://reviews.llvm.org/D49838#1176622, @erichkeane wrote:
> 2 small items, otherwise looks good.
Thank you for taking a look!
================
Comment at: include/clang/AST/Expr.h:2824
CastExprBits.Kind = kind;
- CastExprBits.PartOfExplicitCast = false;
setBasePathSize(BasePathSize);
----------------
erichkeane wrote:
> So, I'd prefer that this line get left in. Removing this makes it the single
> unused item in CastExprBitfields, so leaving it uninitialized is likely a bad
> idea.
Aha, ok.
That will result in less ` CastExprBits.PartOfExplicitCast = false;` lines
scattered across different constructors, too.
================
Comment at: include/clang/AST/Expr.h:2832
: Expr(SC, Empty) {
setBasePathSize(BasePathSize);
}
----------------
Oh, i forget to init it here.
================
Comment at: lib/Sema/SemaCast.cpp:97
while ((CE = dyn_cast<ImplicitCastExpr>(CE->getSubExpr())))
- CE->setIsPartOfExplicitCast(true);
+ dyn_cast<ImplicitCastExpr>(CE)->setIsPartOfExplicitCast(true);
}
----------------
erichkeane wrote:
> I think I'd prefer just using a different variable in the 'while' loop to
> avoid the cast. something like while((auto ICE =....
>
> That said, either way this isn't a dyn_cast, this would be just a cast (since
> we KNOW the type).
I was trying to avoid having one extra variable, which may confuse things.
Let's see maybe it's not that ugly..
Repository:
rC Clang
https://reviews.llvm.org/D49838
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
