<div dir="ltr">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)<br><br>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.<br><br>You'll have to use llvm::make_unique, rather than std::make_unique - since we're still compiling as C++11, not C++14.<div><br></div><div>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)<br><br>Looks like a reformat of an unchanged/unrelated line:<br><br><div>-  BugReportEquivClass* EQ = EQClasses.FindNodeOrInsertPos(ID, InsertPos);</div><div>+  BugReportEquivClass *EQ = EQClasses.FindNodeOrInsertPos(ID, InsertPos);<br><br>And where does emitReport actually take ownership? </div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jun 22, 2015 at 8:07 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Currently, when the analyzer wants to emit a report, it takes a naked<br>
pointer, and the emitReport function eventually takes over ownership<br>
of that pointer. I think this is a dangerous API because it's not<br>
particularly clear that this ownership transfer will happen, or that<br>
the pointer must be unique.<br>
<br>
This patch makes the ownership semantics more clear by encoding it as<br>
part of the API. There should be no functional changes, and I do not<br>
think it caught any bugs, but I do think this is an improvement.<br>
<br>
Thoughts or opinions?<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span><br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div>