[cfe-dev] [Patch] Make ParseAST() take ASTContext and TU params

[Ted Kremenek]

Hi Alexei,

I apologize, I left the "ei" off your name.

On Jan 27, 2009, at 3:06 PM, Ted Kremenek wrote:

> Hi Alex,
>
> Sorry for not looking at this sooner.  The patch looks good, but I
> have two additional nits.
>
> First, could you possibly add a comment to the prototype and
> definition of ParseAST indicating what is the behavior of
> TranslationUnit and ASTContext are null?  The postcondition for this
> situation is that once ParseAST completes then any subsequent accesses
> to the ASTs it provided is no longer valid (because they are
> reclaimed).  This should be explicitly stated, since the ASTConsumer
> (which lives beyond the call to ParseAST) could potentially have
> dangling references to the ASTs.
>
> Second, does it make sense to pass in both a TranslationUnit and an
> ASTContext pointer?  Since TranslationUnit holds a reference to
> ASTContext, I think we only need a single argument.  If the
> TranslationUnit* is null then the ASTContext* is null as well.
>
> Ted
>
> On Jan 27, 2009, at 11:49 AM, Alexei Svitkine wrote:
>
>> If no one objects to these changes, can someone commit this to the
>> repository?
>>
>> -Alexei
>>
>> On Tue, Jan 27, 2009 at 10:31 AM, Alexei Svitkine
>> <alexei.svitkine at gmail.com> wrote:
>>> On Mon, Jan 26, 2009 at 2:29 PM, Alexei Svitkine
>>> <alexei.svitkine at gmail.com> wrote:
>>>> Here's an updated patch that also removes the comment about the
>>>> FreeMemory param which no longer exists.
>>>>
>>>> -Alexei
>>>>
>>>> On Sat, Jan 24, 2009 at 2:51 PM, Alexei Svitkine
>>>> <alexei.svitkine at gmail.com> wrote:
>>>>> You are right. Here's a revised patch.
>>>>>
>>>>> -Alexei
>>>>>
>>>>> On Sat, Jan 24, 2009 at 11:19 AM, Sebastian Redl
>>>>> <sebastian.redl at getdesigned.at> wrote:
>>>>>> Alexei Svitkine wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Attached is a patch to make ParseAST() take ASTContext and
>>>>>>> TranslationUnit as parameters, as suggested by Steve Naroff.
>>>>>>>
>>>>>>
>>>>>> The double negative in the code confuses me somewhat, but isn't
>>>>>> this
>>>>>> condition the wrong way round?
>>>>>>
>>>>>>> +    if (!DisableFree) {
>>>>>>> +      Context = new ASTContext(PP.getLangOptions(),
>>>>>>> PP.getSourceManager(),
>>>>>>> +                               PP.getTargetInfo(),
>>>>>>> +                               PP.getIdentifierTable(),
>>>>>>> PP.getSelectorTable());
>>>>>>> +      TU = new TranslationUnit(*Context);
>>>>>>> +    }
>>>>>>> +    ParseAST(PP, Consumer.get(), Context, TU, Stats);
>>>>>>
>>>>>> Previously, if DisableFree was true, FreeMemory was false, and
>>>>>> ParseAST
>>>>>> didn't free the objects.
>>>>>> Now, if DisableFree is true, the driver doesn't allocate
>>>>>> objects, so
>>>>>> ParseAST does and will free them.
>>>>>>
>>>>>> Sebastian
>>>>>>
>>>>>
>>>>
>>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev