[cfe-commits] r60900 - in /cfe/trunk: include/clang/Parse/Ownership.h

[steve naroff]

On Dec 14, 2008, at 10:59 AM, Sebastian Redl wrote:

> steve naroff wrote:
>> I just read Howard's post closely. While his explanation of the  
>> "mechanics" are interesting, it doesn't tell me why this idiom is  
>> compelling for clang's Parser/Action subsystems.
>>
> Having owning pointers in the Action interface prevents leaks in  
> Sema from thoughtlessness.
>>>
>>>>
>>>> I'm curious...does compiler-supported ownership tracking actually  
>>>> fix any existing clang bugs? (e.g. crashers, leaks, etc.). If  
>>>> not, I assume this work is targeted at future-proofing the front- 
>>>> end as we embark on more complex C++ features?
>>> There are still 300 bytes worth of leaks in the Parser/ 
>>> statements.c testcase. I'm pretty sure they come from discarded  
>>> Declarator structures.
>>> Also, there's still a lot of places where, in the case of an  
>>> error, existing Stmt and Expr nodes are dropped and leaked in the  
>>> Sema. There used to be a lot such places in the Parser too, since  
>>> manual deletion in the case of error was just too much hassle.  
>>> Now, with the smart pointers, such places are reduced to where the  
>>> nodes are hidden within larger structures (Declarator is the worst  
>>> offender, I think). I haven't quantified the improvement, but it  
>>> was pretty easy to prove from the code logic that some places  
>>> simply leaked.
>>
>> Since Declarator's are stack allocated, they can't be leaked. I  
>> guess you are talking about Declarator's leaking their contents?
> Yes. Default expressions of function arguments and size expressions  
> in VLA arrays, for example. There are other stack-allocated  
> structures that can leak expressions they hold.
>> Please be more specific...300 bytes worth of leaks is hardly  
>> compelling.
> Not for the compiler as a standalone program. But consider this: as  
> it was before, the parser/sema combination leaked on most code  
> errors, be they semantic or syntactic. For example, if a statement  
> was missing a semicolon, it was leaked.

>
> Now consider an IDE that integrates the Clang libraries for online  
> syntax checking, auto-completion information, and similar things.  
> Given that it works on code being edited, there will be lots of  
> errors, and every single compilation with errors will leak.
>

Are you saying that the only way to plug the leaks is to use smart  
pointers *everywhere*?

> The moment the IDE implementor realizes this, he will think very  
> hard about not using Clang.
>


As you are well aware, clang is an Open Source project that is under  
active development. Memory leaks would certainly need to be addressed  
before IDE integration is practical. In general, clang's architecture  
has been driven by it's clients. If you are integrating clang within  
an IDE, then I am confident your work will indeed solve problems faced  
by IDE developers.

>
>> In general, the Parser doesn't do much dynamic memory allocation  
>> (yet you've had to make *many* changes to the Parser/Action API's).
> This is not really true. While the parser doesn't do much dynamic  
> memory allocation by itself, it handles very many heap-allocated  
> objects from Sema.

It *is* true. Sema does the allocation, not the Parser. Since Sema  
does the allocation, I don't know why the Parser interface needs to  
change.

>
>>
>> While I sympathize with the idea of type-encoded ownership, I'm  
>> trying to understand how it helps the Parser/Action interface  
>> *specifically*. I think having to add an ActionBase class to make  
>> all this stuff work is bizarre.
> The ActionBase is actually only necessary because I want the full  
> definitions of the smart pointers available in the Action  
> definition. (Though the alternative was even more ugly.)
> Having type-encoded ownership in the Parser/Action interface has  
> several benefits:
> - A newcomer to the codebase, when he sees 'Owning' in the type of  
> arguments and return values, will immediately know that these  
> functions take ownership. (Unless he's never seen the idea of type- 
> encoded ownership before. But even then it should be easy to  
> understand.)
> - We need those smart pointers in the implementation. Having them in  
> the interface too helps consistency and avoids errors.
>

My background is more on the Objective-C side, where classes are used  
much differently than in C++.

Since I don't have much combat experience with C++ smart pointers (and  
type-encoded ownership), I'm really not in a great position to judge  
it. Part of my "push back" is probably fear of the unknown.  
Nevertheless, given the degree of API changes and recent port problem,  
I hope the pain is worth the gain (in terms of improving productivity  
and reducing leaks). You seem like you are doing some great work...I  
just wanted to make folks aware of the "less is more" spirit that we  
try to follow.

snaroff

> Sebastian