[PATCH] D49838: [AST] Sink 'part of explicit cast' down into ImplicitCastExpr

[Roman Lebedev via Phabricator via cfe-commits]

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