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

[Sebastian Redl]

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. The moment the IDE 
implementor realizes this, he will think very hard about not using Clang.
> 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.
>
> 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.

Sebastian