[cfe-dev] Parser Stmt/Expr Owning Pointer

[Chris Lattner]

On Dec 7, 2008, at 8:29 AM, Sebastian Redl wrote:

> Hi,
>
> In my quest to have parser and sema use smart pointers to ensure  
> that all nodes are always freed,

Yay!

> I have now replaced ASTGuard with ASTOwner. ASTGuard was a class  
> that simply sat by and watched expressions, deleting them if  
> necessary, but it was an add-on, which led to very ugly code:
>
> ExprResult Res = Parse...();
> ExprGuard Guard(Actions);
>
> The new ASTOwner is a proper move-emulated smart pointer, and also  
> holds the validity information of ActionResult. Thus, the code becomes
>
> ExprOwner Res(Actions, Parse...());
>
> Passing Actions to every pointer is an unfortunate necessity.

Ok.

> The move emulation of this pointer is specialized to support  
> backwards compatibility. The ASTMove class, core of the move  
> emulation, cannot only be implicitly converted to a new ASTOwner  
> (which will take ownership), but also to an ActionResult or a void  
> pointer, which also have ownership. This allows the use of move() in  
> many contexts, such as returning an ASTOwner where an ActionResult  
> is expected, or passing a void* to the Action by calling move().  
> This is a significant advantage in making the use of the smart  
> pointer intuitive: move() is used whenever you give up ownership, no  
> matter what receives it.

In terms of API, can we have a better name than .move() ?  From the  
API perspective, seeing:
+      return Idx.move();
is really strange.

How about .take(), as in "take ownership"?

Is there any good way to make the type checker catch cases where  
ownership is not transfered?  Using "return Idx;" accidentally would  
be a very subtle mistake would would end up with a dangling pointer  
(if I understand).  Maybe if the *default* was to take ownership it  
would lead to accidental null-dereference bugs or instead of dangling  
pointer bugs?

> The change is simple but very far-reaching, because I've excised  
> every single ExprResult/StmtResult that isn't part of a function  
> signature. Thus, I'll await comments before committing it. It also  
> influences the way everyone should write code in the parser.

I really like the idea, but lets talk about the meta issues above.  I  
haven't had a chance to review the patch in detail yet.

> I'd also like to see if the use of the smart pointer, which is a  
> word larger than ActionResult and two words larger than a raw  
> pointer, negatively affects performance. Do you people have a hint,  
> or perhaps even a complete framework, for tracking parser performance?

Are you on a mac?  If so, "time clang INPUTS/Cocoa.m" with a release- 
asserts build is the benchmark of choice right now :)

> In the next step, I'll make the Parser's internal signatures use the  
> smart pointer, which should reduce the number of times Actions is  
> passed greatly, but increases the number of times move() is called  
> greatly, too. After that, we'll want to use smart pointers in the  
> Action interface, and finally use smart pointers inside Sema.

Very cool.  Thanks you for working on this Sebastian!

-Chris