
<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>[+cfe-dev list]</div><br><div><div>On Aug 31, 2014, at 13:47 , Thomas H.P. Andersen <<a href="mailto:phomes@gmail.com">phomes@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">On Fri, Aug 8, 2014 at 6:22 PM, Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:<br><blockquote type="cite">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.)<br><br>Take as long as you need, ask questions along the way, and thanks for looking into this!<br>Jordan<br></blockquote><br>Hi Jordan,<br><br>I had a few moments to look at this again in the weekend. I guess I am<br>making progress but I feel that whenever I figure something out I also<br>find 3 new things I have to understand. I hope that you can help to<br>make a few things clearer for me.<br><br>I have been working on a few different approaches but I do not feel<br>sure about what the correct way forward is. I have created the<br>CFGGNUCleanup subclass of CFGElement and the misc methods to add it to<br>the CFG. But now I am unsure of whether to extend the logic in<br>addAutomaticObjDtors to also deal with VarDecls with cleanup<br>attribute? The method name would have to change but the alternative is<br>to add another method that would be called in all the same places as<br>addAutomaticObjDtors (the Visit* methods where vars go out of scope)<br>or even a new method to call both?.</blockquote><div><br></div><div>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.)</div><div><br></div><br><blockquote type="cite"> I am not sure I even understand<br>how addAutomaticObjDtors is supposed to work. If I understand<br>correctly it will iterate over all VarDecl's within the scope we are<br>going to leave. It seems to add the automaticObjDtor to all of them.<br>Is there some kind of filtering I just don't understand or have I<br>misunderstood it completly?<br></blockquote><div><br></div>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.</div><div><br></div><div>What does Clang do when compiling a variable with <i>both</i> __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?)<br><div><br></div><div><br></div><blockquote type="cite"><br>I also had a look at ExprEngine to process the new CFGNUCleanup there.<br>Do I understand correctly that this would be the place where I should<br>add a VisitGNUCleanup method that will take the CFGGNUCleanup and<br>there create the statement to call the function function given in the<br>cleanup attribute with the variable as argument?<br></blockquote></div><br><div>I would take a look at how ProcessImplicitDtor gets invoked, and mirror that.</div><div><br></div><div>Feel free to also send partial patches to the cfe-commits list, or post them at <a href="http://reviews.llvm.org">http://reviews.llvm.org</a> with me as a reviewer and cfe-commits as a subscriber.</div><div><br></div><div>Thanks for looking at this!</div><div>Jordan</div></body></html>