[cling] Create new CompoundStmt instead of replacing children

interpreter/cling/lib/Interpreter/AutoSynthesizer.cpp

@@ -25,9 +25,9 @@ namespace cling {
   public:
     AutoFixer(Sema* S) : m_Sema(S), m_FoundDRE(0) {}
 
-    void Fix(CompoundStmt* CS) {
+    CompoundStmt* Fix(CompoundStmt* CS) {
       if (!CS->size())
-        return;
+        return nullptr;
       typedef llvm::SmallVector<Stmt*, 32> Statements;
       Statements Stmts;
       Stmts.append(CS->body_begin(), CS->body_end());
@@ -44,18 +44,30 @@ namespace cling {
         }
       }
       if (CS->size() != Stmts.size())
-        CS->replaceStmts(m_Sema->getASTContext(), Stmts);
+        return CompoundStmt::Create(m_Sema->getASTContext(), Stmts,
+                                    CS->getLBracLoc(), CS->getRBracLoc());
+      return nullptr;
     }
 
-    void Fix(CXXTryStmt* TS) {
-      Fix(TS->getTryBlock());
-      for(unsigned int h = 0; h < TS->getNumHandlers(); ++h) {
-        Stmt *s = TS->getHandler(h)->getHandlerBlock();
-        if (CompoundStmt* CS = dyn_cast_or_null<CompoundStmt>(s))
-          Fix(CS);
-        else if (CXXTryStmt *HandlerTS = dyn_cast_or_null<CXXTryStmt>(s))
-          Fix(HandlerTS);
+    CXXTryStmt* Fix(CXXTryStmt* TS) {
+      CompoundStmt *TryBlock = TS->getTryBlock();
+      if (CompoundStmt *NewTryBlock = Fix(TryBlock))
+        TryBlock = NewTryBlock;
+
+      llvm::SmallVector<Stmt*, 4> Handlers(TS->getNumHandlers());
+      for (unsigned int h = 0; h < TS->getNumHandlers(); ++h) {
+        Stmt *HandlerBlock = TS->getHandler(h)->getHandlerBlock();
+        if (CompoundStmt *HandlerCS = dyn_cast_or_null<CompoundStmt>(HandlerBlock)) {
+          if (CompoundStmt *NewHandlerCS = Fix(HandlerCS))
+            HandlerBlock = NewHandlerCS;
+        } else if (CXXTryStmt *HandlerTS = dyn_cast_or_null<CXXTryStmt>(HandlerBlock)) {
+          if (CXXTryStmt *NewHandlerTS = Fix(HandlerTS))
+            HandlerBlock = NewHandlerTS;
+        }
       }
+
+      return CXXTryStmt::Create(m_Sema->getASTContext(), TS->getTryLoc(),
+                                TryBlock, Handlers);
     }
 
     bool VisitDeclRefExpr(DeclRefExpr* DRE) {
@@ -92,9 +104,12 @@ namespace cling {
       // those.
       Stmt *Body = FD->getBody();
       if (CompoundStmt* CS = dyn_cast_or_null<CompoundStmt>(Body))
-        m_AutoFixer->Fix(CS);
+        Body = m_AutoFixer->Fix(CS);
       else if (CXXTryStmt *TS = dyn_cast_or_null<CXXTryStmt>(Body))
-        m_AutoFixer->Fix(TS);
+        Body = m_AutoFixer->Fix(TS);
+
+      if (Body != nullptr)
+        FD->setBody(Body);
     }
     return Result(D, true);
   }

interpreter/cling/lib/Interpreter/DeclExtractor.cpp

@@ -211,7 +211,10 @@ namespace cling {
       }
     }
 
-    CS->replaceStmts(*m_Context, Stmts);
+    // Create a new body.
+    auto newCS = CompoundStmt::Create(*m_Context, Stmts, CS->getLBracLoc(),
+                                      CS->getRBracLoc());
+    FD->setBody(newCS);
 
     if (hasNoErrors && !TouchedDecls.empty()) {
       // Put the wrapper after its declarations. (Nice when AST dumping)

interpreter/cling/lib/Interpreter/DynamicLookup.cpp

@@ -289,12 +289,12 @@ namespace cling {
                "Cannot have more than one stmt at that point");
 
         if (NewNode.isForReplacement()) {
-          if (Expr* E = NewNode.getAs<Expr>())
+          if (Expr* E = NewNode.getAs<Expr>()) {
             // Assume void if still not escaped
             *I = SubstituteUnknownSymbol(m_Context->VoidTy, E);
-        }
-        else {
-          *I = NewNode.getAsSingleNode();
+          } else {
+            *I = NewNode.getAsSingleNode();
+          }
         }
       }
     }
@@ -328,14 +328,21 @@ namespace cling {
     // where we know what to do. For Stmt, though, we need to substitute here,
     // knowing the "target" type.
     ASTNodeInfo thenInfo = Visit(Node->getThen());
-    if (thenInfo.isForReplacement())
-      Node->setThen(SubstituteUnknownSymbol(m_Context->VoidTy,
-                                            thenInfo.getAs<Expr>()));
+    if (thenInfo.isForReplacement()) {
+      Stmt* thenReplacement = thenInfo.getAsSingleNode();
+      if (Expr* thenExpr = dyn_cast<Expr>(thenReplacement))
+        thenReplacement = SubstituteUnknownSymbol(m_Context->VoidTy, thenExpr);
+      Node->setThen(thenReplacement);
+    }
     if (Stmt* ElseExpr = Node->getElse()) {
       ASTNodeInfo elseInfo = Visit(ElseExpr);
-      if (elseInfo.isForReplacement())
-        Node->setElse(SubstituteUnknownSymbol(m_Context->VoidTy,
-                                              elseInfo.getAs<Expr>()));
+      if (elseInfo.isForReplacement()) {
+        Stmt* elseReplacement = elseInfo.getAsSingleNode();
+        if (Expr* elseExpr = dyn_cast<Expr>(elseReplacement))
+          elseReplacement =
+            SubstituteUnknownSymbol(m_Context->VoidTy, elseExpr);
+        Node->setElse(elseReplacement);
+      }
     }
 
     return ASTNodeInfo(Node, false);
@@ -356,10 +363,6 @@ namespace cling {
           ASTNodes& NewStmts(NewNode.getNodes());
           for(unsigned i = 0; i < NewStmts.size(); ++i)
             NewChildren.push_back(NewStmts[i]);
-
-          Node->replaceStmts(*m_Context, NewChildren);
-          // Resolve all 1:n replacements
-          Visit(Node);
         }
         else {
           if (NewNode.isForReplacement()) {
@@ -386,10 +389,12 @@ namespace cling {
       }
     }
 
-    Node->replaceStmts(*m_Context, NewChildren);
+    auto* NewCS = CompoundStmt::Create(*m_Context, NewChildren,
+                                       Node->getLBracLoc(),
+                                       Node->getRBracLoc());
 
     --m_NestedCompoundStmts;
-    return ASTNodeInfo(Node, 0);
+    return ASTNodeInfo(NewCS, true);
   }
 
   ASTNodeInfo EvaluateTSynthesizer::VisitDeclStmt(DeclStmt* Node) {

interpreter/cling/lib/Interpreter/ValuePrinterSynthesizer.cpp

@@ -105,7 +105,7 @@ namespace cling {
           *(CS->body_begin()+indexOfLastExpr) = Result;
       }
       // Clear the artificial NullStmt-s
-      if (!ClearNullStmts(CS)) {
+      if (!ClearNullStmts(FD)) {
         // FIXME: Why it is here? Shouldn't it be in DeclExtractor?
         // if no body remove the wrapper
         DeclContext* DC = FD->getDeclContext();
@@ -166,14 +166,24 @@ namespace cling {
   }
 
 
-  unsigned ValuePrinterSynthesizer::ClearNullStmts(CompoundStmt* CS) {
+  unsigned ValuePrinterSynthesizer::ClearNullStmts(FunctionDecl* FD) {
+    CompoundStmt* CS = cast<CompoundStmt>(FD->getBody());
+    assert(CS && "Missing body?");
+
     llvm::SmallVector<Stmt*, 8> FBody;
     for (auto&& child: CS->children())
       if (!isa<NullStmt>(child))
         FBody.push_back(child);
 
-    if (CS->size() != FBody.size())
-      CS->replaceStmts(*m_Context, FBody);
+    // If body would be empty, return early - the function will be removed.
+    if (FBody.empty())
+      return 0;
+
+    if (CS->size() != FBody.size()) {
+      auto BodyCS = CompoundStmt::Create(*m_Context, FBody, CS->getLBracLoc(),
+                                         CS->getRBracLoc());
+      FD->setBody(BodyCS);
+    }
 
     return FBody.size();
   }

interpreter/cling/lib/Interpreter/ValuePrinterSynthesizer.h

@@ -16,7 +16,6 @@
 
 namespace clang {
   class ASTContext;
-  class CompoundStmt;
   class Decl;
   class FunctionDecl;
   class Expr;
@@ -63,7 +62,7 @@ namespace cling {
     /// critical error.
     bool tryAttachVP(clang::FunctionDecl* FD);
     clang::Expr* SynthesizeVP(clang::Expr* E);
-    unsigned ClearNullStmts(clang::CompoundStmt* CS);
+    unsigned ClearNullStmts(clang::FunctionDecl* FD);
 
     // Find and cache cling::runtime on first request.
     void FindAndCacheRuntimeLookupResult(clang::SourceLocation SourceLoc);

interpreter/cling/lib/Utils/AST.cpp

@@ -169,8 +169,11 @@ namespace utils {
               indexOfLastExpr++;
               newBody.insert(newBody.begin() + indexOfLastExpr, DRE);
 
-              // Attach the new body (note: it does dealloc/alloc of all nodes)
-              CS->replaceStmts(S->getASTContext(), newBody);
+              // Attach a new body.
+              auto newCS = CompoundStmt::Create(S->getASTContext(), newBody,
+                                                CS->getLBracLoc(),
+                                                CS->getRBracLoc());
+              FD->setBody(newCS);
               if (FoundAt)
                 *FoundAt = indexOfLastExpr;
               return DRE;

interpreter/llvm/src/tools/clang/include/clang/AST/Stmt.h

@@ -131,8 +131,7 @@ class alignas(void *) Stmt {
 
     unsigned : NumStmtBits;
 
-    unsigned WasReplaced : 1;
-    unsigned NumStmts : 32 - (NumStmtBits + 1);
+    unsigned NumStmts : 32 - NumStmtBits;
 
     /// The location of the opening "{".
     SourceLocation LBraceLoc;
@@ -1329,7 +1328,6 @@ class CompoundStmt final : public Stmt,
   explicit CompoundStmt(SourceLocation Loc)
       : Stmt(CompoundStmtClass), RBraceLoc(Loc) {
     CompoundStmtBits.NumStmts = 0;
-    CompoundStmtBits.WasReplaced = 0;
     CompoundStmtBits.LBraceLoc = Loc;
   }
 
@@ -1343,10 +1341,7 @@ class CompoundStmt final : public Stmt,
   using body_range = llvm::iterator_range<body_iterator>;
 
   body_range body() { return body_range(body_begin(), body_end()); }
-  body_iterator body_begin() {
-    Stmt** begin = getTrailingObjects<Stmt *>();
-    return !CompoundStmtBits.WasReplaced ? begin : (body_iterator)begin[0];
-  }
+  body_iterator body_begin() { return getTrailingObjects<Stmt *>(); }
   body_iterator body_end() { return body_begin() + size(); }
   Stmt *body_front() { return !body_empty() ? body_begin()[0] : nullptr; }
 
@@ -1362,7 +1357,7 @@ class CompoundStmt final : public Stmt,
   }
 
   const_body_iterator body_begin() const {
-    return const_cast<CompoundStmt*>(this)->body_begin();
+    return getTrailingObjects<Stmt *>();
   }
 
   const_body_iterator body_end() const { return body_begin() + size(); }
@@ -1396,8 +1391,6 @@ class CompoundStmt final : public Stmt,
     return const_reverse_body_iterator(body_begin());
   }
 
-  void replaceStmts(const ASTContext &C, llvm::ArrayRef<Stmt*> Stmts);
-
   // Get the Stmt that StmtExpr would consider to be the result of this
   // compound statement. This is used by StmtExpr to properly emulate the GCC
   // compound expression extension, which ignores trailing NullStmts when