Looks good. Thanks for working on this! - case OwnershipAttr::Returns: - State = MallocMemReturnsAttr(C, CE, *i); - break; .. + case OwnershipAttr::Returns: + State = MallocMemReturnsAttr(C, CE, *i); + break; .. Is the indentation the only diff here? If yes, please keep the previous version; most of the case statements are not indented in the llvm/clang codebases. We usually try to keep the patches focused on the problem solved. If cleanup of existing code is necessary, it should go into a separate commit.
- HelpText<"Check for memory leaks, double free, and use-after-free problems. Assumes that all user-defined functions which might free a pointer are annotated.">, + HelpText<"Check for memory leaks, double free, and use-after-free problems. Traces memory managed by malloc()/free(). Assumes that all user-defined functions which might free a pointer are annotated.">, This is getting too long. Jordan had previously pointed out that you might need to add 'new' and 'delete' to doesNotFreeMemOrKnown. I do not see that changed. Has it been done or is it not necessary? Thanks, Anna. On Mar 5, 2013, at 6:25 PM, Jordan Rose <[email protected]> wrote: > > On Mar 5, 2013, at 5:12 , Anton Yartsev <[email protected]> wrote: > >> Attached is the new patch with unix.MismatchedDeallocator and >> cplusplus.NewDelete splitted from unix.Malloc plus comments addressed and >> several other improvements. > > Awesome. At this point I think this is basically ready to go in. Anna? > > (We need to update SATestBuild.py and lib/Frontend/CompilerInvocation.cpp to > enable the "cplusplus" package again.) > > >> + delete p; // expected-warning{{Memory allocated by operator new[] should >> be deallocated by operator delete[], not operator delete}} > > > Bikeshedding again on the warning text: do we really need the word "operator" > in there? Maybe just putting quotes around the operator name would be good > enough: > +1 It's best to keep the diagnostics as concise as possible. > "Memory allocated by 'new[]' should be deallocated by 'delete[]', not > 'delete'". > > That's this part of printAllocDeallocName (which may have been suggested by > me): > >> + if (const CXXNewExpr *NE = dyn_cast<CXXNewExpr>(E)) { >> + os << *NE->getOperatorNew(); >> + return true; >> + } >> + >> + if (const CXXDeleteExpr *DE = dyn_cast<CXXDeleteExpr>(E)) { >> + os << *DE->getOperatorDelete(); >> + return true; >> + } > > It looks like there's a function getOperatorSpelling that will get the name > without the word "operator". (It makes sense to leave in the "operator" when > it appears in an explicit call expr, just not a normal expression or the > "expected" case. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
