On Mon, Jun 22, 2015 at 1:59 PM, David Blaikie <[email protected]> wrote: > > > On Mon, Jun 22, 2015 at 10:48 AM, Aaron Ballman <[email protected]> > wrote: >> >> Updated patch attached. >> >> On Mon, Jun 22, 2015 at 12:23 PM, David Blaikie <[email protected]> >> wrote: >> > I'm assuming emitReport always takes ownership? (it doesn't look like it >> > has >> > any API surface area to express to the caller (in the current raw >> > pointer >> > API) that it didn't take ownership) >> > >> > In that case, best to pass by value rather than (lvalue or rvalue) >> > reference >> > so caller's explicitly pass off ownership (std::move(R)) to emitReport. >> >> Fixed. >> >> > >> > You'll have to use llvm::make_unique, rather than std::make_unique - >> > since >> > we're still compiling as C++11, not C++14. >> >> Fixed. >> >> > I probably wouldn't bother renaming "R" to "UniqueR" in emitReport - >> > don't >> > think it adds/changes much. Ah, I see - there already was a UniqueR and >> > R. >> > Probably just use R, though? (now that it's the only thing, so there's >> > no >> > need to distinguish it as the unique one) >> >> Fixed. >> >> > Looks like a reformat of an unchanged/unrelated line: >> > >> > - BugReportEquivClass* EQ = EQClasses.FindNodeOrInsertPos(ID, >> > InsertPos); >> > + BugReportEquivClass *EQ = EQClasses.FindNodeOrInsertPos(ID, >> > InsertPos); >> >> Removed (will commit separately). > > > Will leave it to Anna to sign off on, but looks like an improvement to me. > > (my usual generic question on changes like this (should probably be treated > separately from this patch, if you've interest): could we use this by value > instead of by pointer? I see BugReport has virtual functions but not sure it > has any derived classes (the Doxygen docs don't seem to indicate any?))
There are some derived classes. See RetainCountChecker.cpp (CFRefLeakReport has an override). ~Aaron _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
