[PATCH] clarify ownership of BugReport objects
David Blaikie
dblaikie at gmail.com
Mon Jun 22 09:23:36 PDT 2015
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.
You'll have to use llvm::make_unique, rather than std::make_unique - since
we're still compiling as C++11, not C++14.
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)
Looks like a reformat of an unchanged/unrelated line:
- BugReportEquivClass* EQ = EQClasses.FindNodeOrInsertPos(ID, InsertPos);
+ BugReportEquivClass *EQ = EQClasses.FindNodeOrInsertPos(ID, InsertPos);
And where does emitReport actually take ownership?
On Mon, Jun 22, 2015 at 8:07 AM, Aaron Ballman <aaron at aaronballman.com>
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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150622/fb073c7c/attachment.html>
More information about the cfe-commits
mailing list