[cfe-dev] Ownership of Stmts and Exprs, and destroying them

[Chris Lattner]

On Nov 25, 2008, at 2:20 AM, Sebastian Redl wrote:

> OK, revised version of the patch. AstGuard is now  
> ASTGuard<Destroyer>, with typedef ExprGuard and StmtGuard.
>
> AstArrayGuard now holds and owns (in fact, derives from) the  
> SmallVector and was renamed to ASTVector, with template aliases  
> (derived classes) ExprVector<unsigned N> and StmtVector<unsigned N>.
> This has greatly simplified its use, as long as people remember that  
> you have to explicitly give up ownership with take() instead of  
> using &ar[0] to pass the content to the Sema.

Looks great to me!

Minor stuff:

+//===--- AstGuard.h - C Language Parser -------------------------*- C+ 
+ -*-===//

Please update comment



+  // RAII guard for freeing StmtTys and ExprTys on early exit in the  
parser.
+  template <void (Action::*Destroyer)(void*)>

Please make this a doxygen comment and mention the two instantiations  
that will be used.  Likewise for ASTVector.



+    void destroy() {
+      if (Node)
+        (Actions.*Destroyer)(Node);
+    }

Thanks for adding the check.  This avoids the virtual dispatch in the  
common case when ownership is taken before it is destroyed.


+  template <unsigned N>
+  class StmtVector : public ASTVector<&Action::DeleteStmt, N>

It is probably fine to just make these typedefs with a fixed N, say  
"12".


Otherwise, the patch looks great.  Thanks for tackling this Sebastian!

-Chris