Thanks for looking on the patch.<br><br><div class="gmail_quote">W dniu 28 września 2010 03:31 użytkownik Ted Kremenek <span dir="ltr"><<a href="mailto:kremenek@apple.com">kremenek@apple.com</a>></span> napisał:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im"><br>
On Sep 27, 2010, at 3:55 PM, Marcin Świderski wrote:<br>
<br>
> Patch adds implicit destructors generation for objects with automatic storage duration. Patch is rather big, but it contains all cases (that I thought of), as it will be easier to review as a whole IMO. While commiting I can divide it to smaller chuncks.<br>

<br>
</div>Overall this looks great.  There are a few misspelled words in the comments resulting from typos (e.g., 'Elememnts'), so it's probably worth running a spell-checker.<br>
<br>
Besides specific comments (below), I'm concerned about the impact on CFG-construction time when scope building is *not* enabled.  This is currently the common case for uses of the CFG in Sema.  I think there shouldn't be any noticeable penalty since most of the logic is guarded with a flag, but it is worth verifying.<br>

<div class="im"><br></div></blockquote><div>Is there any performance benchmark prepared for such cases?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">
><br>
> I've added some test cases. They can be later used to create regresion tests.<br>
<br>
</div>For the test cases, how do you plan on testing it?  Also, since these are all small tests, please include them in one file (if possible).<br>
<br></blockquote><div>You did mention something about similar case in test folder, but I can't currently locate your message. Some suggestions would be useful, e.g. how to set up test for feature that is currently turned off.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
A few specific comments inline:<br>
<br>
Index: include/clang/Analysis/CFG.h<br>
===================================================================<br>
--- include/clang/Analysis/CFG.h        (revision 114789)<br>
+++ include/clang/Analysis/CFG.h        (working copy)<br>
@@ -1159,7 +1417,7 @@<br>
       // No increment code.  Create a special, empty, block that is used as the<br>
       // target block for "looping back" to the start of the loop.<br>
       assert(Succ == EntryConditionBlock);<br>
-      Succ = createBlock();<br>
+      Succ = Block ? Block : createBlock();<br>
<br>
Why this line change?  The "loop back" block is very important for analyzer diagnostics.  Is this because 'addAutomaticObjDtors' might create a block?<br>
<br></blockquote><div>Yes. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
     }<br>
<br>
     // Finish up the increment (or empty) block if it hasn't been already.<br></blockquote></div>