[cfe-dev] Bug 3888: cleanup attribute

[Jordan Rose]

[+cfe-dev list]

On Aug 31, 2014, at 13:47 , Thomas H.P. Andersen <phomes at gmail.com> wrote:

> On Fri, Aug 8, 2014 at 6:22 PM, Jordan Rose <jordan_rose at apple.com> wrote:
>> You're very welcome to e-mail me directly! You should also feel free to CC the cfe-dev mailing list, so that other people can both see the results of our conversation and possibly answer questions faster than me. (I'm only spending a bit of time reviewing patches each day, I'm afraid.)
>> 
>> Take as long as you need, ask questions along the way, and thanks for looking into this!
>> Jordan
> 
> Hi Jordan,
> 
> I had a few moments to look at this again in the weekend. I guess I am
> making progress but I feel that whenever I figure something out I also
> find 3 new things I have to understand. I hope that you can help to
> make a few things clearer for me.
> 
> I have been working on a few different approaches but I do not feel
> sure about what the correct way forward is. I have created the
> CFGGNUCleanup subclass of CFGElement and the misc methods to add it to
> the CFG. But now I am unsure of whether to extend the logic in
> addAutomaticObjDtors to also deal with VarDecls with cleanup
> attribute? The method name would have to change but the alternative is
> to add another method that would be called in all the same places as
> addAutomaticObjDtors (the Visit* methods where vars go out of scope)
> or even a new method to call both?.

I think extending the method is the right way to go. I wouldn't even worry about changing the name; __attribute__((cleanup)) is very clearly a destructor-like thing already. (If you can think of a better name, you're welcome to suggest it, though.)


> I am not sure I even understand
> how addAutomaticObjDtors is supposed to work. If I understand
> correctly it will iterate over all VarDecl's within the scope we are
> going to leave. It seems to add the automaticObjDtor to all of them.
> Is there some kind of filtering I just don't understand or have I
> misunderstood it completly?

It looks like only variables with non-trivial destructors are added to the local scope to begin with; see CFGBuilder::addLocalScopeForVarDecl (or just line 1337). We'd want to add variables with __attribute__((cleanup)) as well, and then be careful to handle them correctly in addAutomaticObjDtors. A brute-force way to handle this would be to replace LocalScope's list of VarDecls with a PointerIntPair<VarDecl, 1, bool>, where the boolean value indicates whether this is being added for its attribute-provided cleanups or its destructor.

What does Clang do when compiling a variable with both __attribute__((cleanup)) and a non-trivial destructor? Because we should make sure we do the same thing in the CFG. (Also, do we accept multiple cleanup attributes on the same variable?)


> 
> I also had a look at ExprEngine to process the new CFGNUCleanup there.
> Do I understand correctly that this would be the place where I should
> add a VisitGNUCleanup method that will take the CFGGNUCleanup and
> there create the statement to call the function function given in the
> cleanup attribute with the variable as argument?

I would take a look at how ProcessImplicitDtor gets invoked, and mirror that.

Feel free to also send partial patches to the cfe-commits list, or post them at http://reviews.llvm.org with me as a reviewer and cfe-commits as a subscriber.

Thanks for looking at this!
Jordan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140904/3b9f7060/attachment.html>