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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: include/clang/AST/Expr.h:2928
 
+  bool getIsPartOfExplicitCast() const {
+    return CastExprBits.PartOfExplicitCast;
----------------
Please also rename this to`isPartOfExplicitCast` as requested on IRC.


================
Comment at: include/clang/AST/Stmt.h:206
     unsigned Kind : 6;
-    unsigned PartOfExplicitCast : 1;
+    unsigned PartOfExplicitCast : 1; // Only representable for ImplicitCastExpr.
     unsigned BasePathSize : 32 - 6 - 1 - NumExprBits;
----------------
representable for -> set for


================
Comment at: lib/Sema/SemaCast.cpp:97
       while ((CE = dyn_cast<ImplicitCastExpr>(CE->getSubExpr())))
-        CE->setIsPartOfExplicitCast(true);
+        dyn_cast<ImplicitCastExpr>(CE)->setIsPartOfExplicitCast(true);
     }
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > 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..
> Indeed, `cast<>` should be used.
> But trying to avoid this cast at all results in uglier code, so let's not.
> (I need to call `getSubExpr()` on this new `ImplicitCastExpr`, so i'd need to maintain two variables, etc...)
Maybe something like:
```
for (auto *Next = CE->getSubExpr(); auto *ICE = dyn_cast<ImplicitCastExpr>(Next); Next = ICE->getSubExpr())
  ICE->setIsPartOfExplicitCast(true);
```
or just
```
for (; auto *ICE = dyn_cast<ImplicitCastExpr>(CE->getSubExpr()); CE = ICE)
  ICE->setIsPartOfExplicitCast(true);
```


Repository:
  rC Clang

https://reviews.llvm.org/D49838