Thanks for looking on the patch. W dniu 28 września 2010 03:31 użytkownik Ted Kremenek <[email protected]>napisał:
> > On Sep 27, 2010, at 3:55 PM, Marcin Świderski wrote: > > > 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. > > 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. > > 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. > > Is there any performance benchmark prepared for such cases? > > > > I've added some test cases. They can be later used to create regresion > tests. > > 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). > > 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. > A few specific comments inline: > > Index: include/clang/Analysis/CFG.h > =================================================================== > --- include/clang/Analysis/CFG.h (revision 114789) > +++ include/clang/Analysis/CFG.h (working copy) > @@ -1159,7 +1417,7 @@ > // No increment code. Create a special, empty, block that is used as > the > // target block for "looping back" to the start of the loop. > assert(Succ == EntryConditionBlock); > - Succ = createBlock(); > + Succ = Block ? Block : createBlock(); > > Why this line change? The "loop back" block is very important for analyzer > diagnostics. Is this because 'addAutomaticObjDtors' might create a block? > > Yes. > } > > // Finish up the increment (or empty) block if it hasn't been already. >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
