[cfe-dev] Modifying an AST and Sema's dependency on an AST consumer

[Ted Kremenek]

On Dec 4, 2008, at 8:42 AM, steve naroff <snaroff at apple.com> wrote:

>
> On Dec 4, 2008, at 11:27 AM, Ted Kremenek wrote:
>
>> On Dec 4, 2008, at 7:26 AM, steve naroff <snaroff at apple.com> wrote:
>>
>>>
>>> On Dec 4, 2008, at 9:54 AM, Lukasz Janyst wrote:
>>>
>>>> On Wed, Dec 3, 2008 at 3:26 PM, Lukasz Janyst <ljanyst at cern.ch>  
>>>> wrote:
>>>>> tables in it. Yet still I seem to experience some strange memory
>>>>> issues when the modified TU gets processed by the CodeGen.
>>>>
>>>> I sorted it out. The problem was the object ownership. It seemed
>>>> natural to me that the identifier table should be owned by the
>>>> ASTContext, but it is owned by the Preprocessor object which I  
>>>> deleted
>>>> after I was done with the parsing.
>>>>
>>>
>>> The IdentifierTable should be owned by ASTContext (as you expected).
>>>
>>> This is a bug (that I thought we fixed a long time ago...apparently
>>> not).
>>>
>>> snaroff
>>
>> I don't think this is a bug.  See my other email.
>
> Hey Ted,
>
> I read your email but I'm still not convinced there isn't an issue  
> here (though it certainly isn't critical to deal with now).
>
> No doubt that running the preprocessor standalone will require an  
> IdentifierTable.
>
> When a preprocessor is being run in a larger context (where AST's  
> are being generated), it makes more sense (to me) if the "larger  
> context" owns the IdentifierTable. In this instance, ASTContext.
>
> From my perspective, an instance of the Preprocessor is transient,  
> while ASTContext is more long-lived.

That's a fair argument, as a Preprocessor is more of a "worker" object  
that is used to do a single pass over the source in a translation  
unit.  One possible solution is to have the IdentifierTable object be  
passed-by-reference to the constructor of Preprocessor with the  
assumption that   the ownership of the IdentifiedTable lies elsewhere.

I'm not certain, however, what the performance implications would be  
of such a change.  After mucking around in the Preprocessor for the  
last  couple of weeks I'm convinced that even small changes can have  
an unanticipated adverse effect on performance.