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

[Alexei Svitkine]

Oops some tabs made it into the previous one.

Here's the same thing with correct whitespace.

-Alexei

On Tue, Jan 27, 2009 at 6:32 PM, Alexei Svitkine
<alexei.svitkine at gmail.com> wrote:
> Thanks for the comments!
>
> Here's a revised patch.
>
> -Alexei
>
> On Tue, Jan 27, 2009 at 6:22 PM, Ted Kremenek <kremenek at apple.com> wrote:
>> 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
>>
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ParseAST5.diff
Type: application/octet-stream
Size: 4528 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20090127/d18ccc56/attachment.obj>