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

[lebedev.ri]

Summary

As discussed in IRC with @rsmith, it is slightly not good to keep that in the CastExpr itself:
Given the explicit cast, which is represented in AST as an ExplicitCastExpr + ImplicitCastExpr's,
only the ImplicitCastExpr's will be marked as PartOfExplicitCast, but not the ExplicitCastExpr itself.
Thus, it is only ever true for ImplicitCastExpr's, so we don't need to write/read/dump it for ExplicitCastExpr's.

We don't need to worry that we write the PartOfExplicitCast in PCH after CastExpr::path_iterator,
since the ExprImplicitCastAbbrev is only used when the NumBaseSpecs == 0, i.e. there is no 'path'.

[erichkeane]

2 small items, otherwise looks good.

include/clang/AST/Expr.h

2824

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.

lib/Sema/SemaCast.cpp

97

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).

[lebedev.ri]

In D49838#1176622, @erichkeane wrote:

2 small items, otherwise looks good.

Thank you for taking a look!

include/clang/AST/Expr.h

2824

Aha, ok.
That will result in less CastExprBits.PartOfExplicitCast = false; lines scattered across different constructors, too.

2832

Oh, i forget to init it here.

lib/Sema/SemaCast.cpp

97

I was trying to avoid having one extra variable, which may confuse things.
Let's see maybe it's not that ugly..

[lebedev.ri]

Address @erichkeane review notes.

lib/Sema/SemaCast.cpp

97

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...)

[rsmith]

include/clang/AST/Expr.h

2937

Please also rename this to`isPartOfExplicitCast` as requested on IRC.

include/clang/AST/Stmt.h

206

representable for -> set for

lib/Sema/SemaCast.cpp

97

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);

[lebedev.ri]

Address @rsmith review notes.

include/clang/AST/Expr.h

2937

Right, sorry.

lib/Sema/SemaCast.cpp

97

Oh wow, one can define variables not just in the "init-statement", how did i not notice this earlier?!
That changes the picture indeed.

include/clang/AST/Expr.h

CastExpr(StmtClass SC, QualType ty, ExprValueKind VK, const CastKind kind,
               (op && op->isInstantiationDependent())),
              // An implicit cast expression doesn't (lexically) contain an
              // unexpanded pack, even if its target type does.
              ((SC != ImplicitCastExprClass &&
                ty->containsUnexpandedParameterPack()) ||
               (op && op->containsUnexpandedParameterPack()))),
         Op(op) {
     CastExprBits.Kind = kind;
-    CastExprBits.PartOfExplicitCast = false;
     setBasePathSize(BasePathSize);
     assert(CastConsistency());
   }
 
   /// Construct an empty cast.
   CastExpr(StmtClass SC, EmptyShell Empty, unsigned BasePathSize)
     : Expr(SC, Empty) {
     setBasePathSize(BasePathSize);
   }
 
 public:
   CastKind getCastKind() const { return (CastKind) CastExprBits.Kind; }
   void setCastKind(CastKind K) { CastExprBits.Kind = K; }
 
-  bool getIsPartOfExplicitCast() const {
-    return CastExprBits.PartOfExplicitCast;
-  }
-  void setIsPartOfExplicitCast(bool PartOfExplicitCast) {
-    CastExprBits.PartOfExplicitCast = PartOfExplicitCast;
-  }
-
   static const char *getCastKindName(CastKind CK);
   const char *getCastKindName() const { return getCastKindName(getCastKind()); }
 
   Expr *getSubExpr() { return cast<Expr>(Op); }
   const Expr *getSubExpr() const { return cast<Expr>(Op); }
   void setSubExpr(Expr *E) { Op = E; }
 
   /// Retrieve the cast subexpression as it was written in the source

 /// }
 /// @endcode
 class ImplicitCastExpr final
     : public CastExpr,
       private llvm::TrailingObjects<ImplicitCastExpr, CXXBaseSpecifier *> {
 private:
   ImplicitCastExpr(QualType ty, CastKind kind, Expr *op,
                    unsigned BasePathLength, ExprValueKind VK)
       : CastExpr(ImplicitCastExprClass, ty, VK, kind, op, BasePathLength) {
+    CastExprBits.PartOfExplicitCast = false;
   }
 
   /// Construct an empty implicit cast.
   explicit ImplicitCastExpr(EmptyShell Shell, unsigned PathSize)
-    : CastExpr(ImplicitCastExprClass, Shell, PathSize) { }
+      : CastExpr(ImplicitCastExprClass, Shell, PathSize) {
+    CastExprBits.PartOfExplicitCast = false;
+  }
 
 public:
   enum OnStack_t { OnStack };
   ImplicitCastExpr(OnStack_t _, QualType ty, CastKind kind, Expr *op,
                    ExprValueKind VK)
       : CastExpr(ImplicitCastExprClass, ty, VK, kind, op, 0) {
+    CastExprBits.PartOfExplicitCast = false;
+  }
+
+  bool getIsPartOfExplicitCast() const {
+    return CastExprBits.PartOfExplicitCast;
+  }
+  void setIsPartOfExplicitCast(bool PartOfExplicitCast) {
+    CastExprBits.PartOfExplicitCast = PartOfExplicitCast;
   }
 
   static ImplicitCastExpr *Create(const ASTContext &Context, QualType T,
                                   CastKind Kind, Expr *Operand,
                                   const CXXCastPath *BasePath,
                                   ExprValueKind Cat);
 
   static ImplicitCastExpr *CreateEmpty(const ASTContext &Context,
                                        unsigned PathSize);
 
   SourceLocation getLocStart() const LLVM_READONLY {

include/clang/AST/Stmt.h

class DeclRefExprBitfields {
     unsigned HasTemplateKWAndArgsInfo : 1;
     unsigned HasFoundDecl : 1;
     unsigned HadMultipleCandidates : 1;
     unsigned RefersToEnclosingVariableOrCapture : 1;
   };
 
   class CastExprBitfields {
     friend class CastExpr;
+    friend class ImplicitCastExpr;
 
     unsigned : NumExprBits;
 
     unsigned Kind : 6;
     unsigned PartOfExplicitCast : 1;
     unsigned BasePathSize : 32 - 6 - 1 - NumExprBits;
   };
 
   class CallExprBitfields {
     friend class CallExpr;
 
     unsigned : NumExprBits;
 

lib/AST/ASTDumper.cpp

public:
     void VisitCapturedStmt(const CapturedStmt *Node);
 
     // OpenMP
     void VisitOMPExecutableDirective(const OMPExecutableDirective *Node);
 
     // Exprs
     void VisitExpr(const Expr *Node);
     void VisitCastExpr(const CastExpr *Node);
+    void VisitImplicitCastExpr(const ImplicitCastExpr *Node);
     void VisitDeclRefExpr(const DeclRefExpr *Node);
     void VisitPredefinedExpr(const PredefinedExpr *Node);
     void VisitCharacterLiteral(const CharacterLiteral *Node);
     void VisitIntegerLiteral(const IntegerLiteral *Node);
     void VisitFixedPointLiteral(const FixedPointLiteral *Node);
     void VisitFloatingLiteral(const FloatingLiteral *Node);
     void VisitStringLiteral(const StringLiteral *Str);
     void VisitInitListExpr(const InitListExpr *ILE);
void ASTDumper::VisitCastExpr(const CastExpr *Node) {
   VisitExpr(Node);
   OS << " <";
   {
     ColorScope Color(*this, CastColor);
     OS << Node->getCastKindName();
   }
   dumpBasePath(OS, Node);
   OS << ">";
+}
 
+void ASTDumper::VisitImplicitCastExpr(const ImplicitCastExpr *Node) {
+  VisitCastExpr(Node);
   if (Node->getIsPartOfExplicitCast())
     OS << " part_of_explicit_cast";
 }
 
 void ASTDumper::VisitDeclRefExpr(const DeclRefExpr *Node) {
   VisitExpr(Node);
 
   OS << " ";

lib/Sema/SemaCast.cpp

struct CastOperation {
     void CheckCXXCStyleCast(bool FunctionalCast, bool ListInitialization);
     void CheckCStyleCast();
 
     void updatePartOfExplicitCastFlags(CastExpr *CE) {
       // Walk down from the CE to the OrigSrcExpr, and mark all immediate
       // ImplicitCastExpr's as being part of ExplicitCastExpr. The original CE
       // (which is a ExplicitCastExpr), and the OrigSrcExpr are not touched.
       while ((CE = dyn_cast<ImplicitCastExpr>(CE->getSubExpr())))
-        CE->setIsPartOfExplicitCast(true);
+        dyn_cast<ImplicitCastExpr>(CE)->setIsPartOfExplicitCast(true);
     }
 
     /// Complete an apparently-successful cast operation that yields
     /// the given expression.
     ExprResult complete(CastExpr *castExpr) {
       // If this is an unbridged cast, wrap the result in an implicit
       // cast that yields the unbridged-cast placeholder type.
       if (IsARCUnbridgedCast) {

lib/Serialization/ASTReaderStmt.cpp

 }
 
 void ASTStmtReader::VisitCastExpr(CastExpr *E) {
   VisitExpr(E);
   unsigned NumBaseSpecs = Record.readInt();
   assert(NumBaseSpecs == E->path_size());
   E->setSubExpr(Record.readSubExpr());
   E->setCastKind((CastKind)Record.readInt());
-  E->setIsPartOfExplicitCast(Record.readInt());
   CastExpr::path_iterator BaseI = E->path_begin();
   while (NumBaseSpecs--) {
     auto *BaseSpec = new (Record.getContext()) CXXBaseSpecifier;
     *BaseSpec = Record.readCXXBaseSpecifier();
     *BaseI++ = BaseSpec;
   }
 }
 
ASTStmtReader::VisitBinaryConditionalOperator(BinaryConditionalOperator *E) {
   E->SubExprs[BinaryConditionalOperator::LHS] = Record.readSubExpr();
   E->SubExprs[BinaryConditionalOperator::RHS] = Record.readSubExpr();
   E->QuestionLoc = ReadSourceLocation();
   E->ColonLoc = ReadSourceLocation();
 }
 
 void ASTStmtReader::VisitImplicitCastExpr(ImplicitCastExpr *E) {
   VisitCastExpr(E);
+  E->setIsPartOfExplicitCast(Record.readInt());
 }
 
 void ASTStmtReader::VisitExplicitCastExpr(ExplicitCastExpr *E) {
   VisitCastExpr(E);
   E->setTypeInfoAsWritten(GetTypeSourceInfo());
 }
 
 void ASTStmtReader::VisitCStyleCastExpr(CStyleCastExpr *E) {

lib/Serialization/ASTWriterStmt.cpp

void ASTStmtWriter::VisitObjCBridgedCastExpr(ObjCBridgedCastExpr *E) {
   Code = serialization::EXPR_OBJC_BRIDGED_CAST;
 }
 
 void ASTStmtWriter::VisitCastExpr(CastExpr *E) {
   VisitExpr(E);
   Record.push_back(E->path_size());
   Record.AddStmt(E->getSubExpr());
   Record.push_back(E->getCastKind()); // FIXME: stable encoding
-  Record.push_back(E->getIsPartOfExplicitCast());
 
   for (CastExpr::path_iterator
          PI = E->path_begin(), PE = E->path_end(); PI != PE; ++PI)
     Record.AddCXXBaseSpecifier(**PI);
 }
 
 void ASTStmtWriter::VisitBinaryOperator(BinaryOperator *E) {
   VisitExpr(E);
ASTStmtWriter::VisitBinaryConditionalOperator(BinaryConditionalOperator *E) {
   Record.AddStmt(E->getFalseExpr());
   Record.AddSourceLocation(E->getQuestionLoc());
   Record.AddSourceLocation(E->getColonLoc());
   Code = serialization::EXPR_BINARY_CONDITIONAL_OPERATOR;
 }
 
 void ASTStmtWriter::VisitImplicitCastExpr(ImplicitCastExpr *E) {
   VisitCastExpr(E);
+  Record.push_back(E->getIsPartOfExplicitCast());
 
   if (E->path_size() == 0)
     AbbrevToUse = Writer.getExprImplicitCastAbbrev();
 
   Code = serialization::EXPR_IMPLICIT_CAST;
 }
 
 void ASTStmtWriter::VisitExplicitCastExpr(ExplicitCastExpr *E) {