[PATCH] clarify ownership of BugReport objects

[Aaron Ballman]

Thank you! Commit in r240400.

~Aaron

On Tue, Jun 23, 2015 at 1:09 AM, Anna Zaks <ganna at apple.com> wrote:
> LGTM and LGTJordan!
>
> Thank you,
> Anna.
>
>> On Jun 22, 2015, at 11:03 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>
>> On Mon, Jun 22, 2015 at 1:59 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>
>>>
>>> On Mon, Jun 22, 2015 at 10:48 AM, Aaron Ballman <aaron at aaronballman.com>
>>> wrote:
>>>>
>>>> Updated patch attached.
>>>>
>>>> On Mon, Jun 22, 2015 at 12:23 PM, David Blaikie <dblaikie at gmail.com>
>>>> 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).
> Correct.
>>
>> ~Aaron
>