<p dir="ltr"><br>
On Sep 10, 2014 10:27 PM, "Craig Topper" <<a href="mailto:craig.topper@gmail.com">craig.topper@gmail.com</a>> wrote:<br>
><br>
> You'll notice half the places in clang that called addPPCallbacks ended up with a cast to std::unique_ptr in the call to addPPCallbacks for the same reason. </p>
<p dir="ltr">If that api takes ownership, I'd still vote for it to take by unique_ptr even if many callers will still want to refer to the object later. Otherwise its hard to tell who's responsible for cleanup.</p>
<p dir="ltr">> But given that I find that kind of ugly I think I'll revert the interface part of the change.<br>
><br>
><br>
> On Wed, Sep 10, 2014 at 12:13 PM, Kim Gräsman <<a href="mailto:kim.grasman@gmail.com">kim.grasman@gmail.com</a>> wrote:<br>
>><br>
>> Hi Craig,<br>
>><br>
>> On Wed, Sep 10, 2014 at 6:53 AM, Craig Topper <<a href="mailto:craig.topper@gmail.com">craig.topper@gmail.com</a>> wrote:<br>
>> > Author: ctopper<br>
>> > Date: Tue Sep  9 23:53:53 2014<br>
>> > New Revision: 217474<br>
>> ><br>
>> > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=217474&view=rev">http://llvm.org/viewvc/llvm-project?rev=217474&view=rev</a><br>
>> > Log:<br>
>> > Unique_ptrify PPCallbacks ownership.<br>
>> ><br>
>> > Unique_ptr creation stil needs to be moved earlier at some of the call sites.<br>
>><br>
>> I'm not happy with this change at all. It precludes a PPCallbacks also<br>
>> serving other duties. E.g. in IWYU we have:<br>
>><br>
>>     IwyuPreprocessorInfo* const preprocessor_consumer<br>
>>         = new IwyuPreprocessorInfo();<br>
>>     compiler.getPreprocessor().addPPCallbacks(preprocessor_consumer);  // here<br>
>>     compiler.getPreprocessor().addCommentHandler(preprocessor_consumer);<br>
>>  // here<br>
>><br>
>>     VisitorState* const visitor_state<br>
>>         = new VisitorState(&compiler, *preprocessor_consumer);  // and here<br>
>>     return std::unique_ptr<IwyuAstConsumer>(new IwyuAstConsumer(visitor_state));<br>
>><br>
>> Our PPCallbacks impl also happens to be a comment handler, plus we<br>
>> want to share the data it's collected with our AST consumer.<br>
>><br>
>> The unique_ptr completely kills this scenario, and alternative<br>
>> designed seem much more complex.<br>
>><br>
>> Would you consider reverting?<br>
>><br>
>> Thanks,<br>
>> - Kim<br>
><br>
><br>
><br>
><br>
> -- <br>
> ~Craig<br>
><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">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
><br>
</p>