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). ~Aaron > > And where does emitReport actually take ownership? > > On Mon, Jun 22, 2015 at 8:07 AM, Aaron Ballman <[email protected]> > wrote: >> >> Currently, when the analyzer wants to emit a report, it takes a naked >> pointer, and the emitReport function eventually takes over ownership >> of that pointer. I think this is a dangerous API because it's not >> particularly clear that this ownership transfer will happen, or that >> the pointer must be unique. >> >> This patch makes the ownership semantics more clear by encoding it as >> part of the API. There should be no functional changes, and I do not >> think it caught any bugs, but I do think this is an improvement. >> >> Thoughts or opinions? >> >> ~Aaron >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >
unique.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
