r217474 - Unique_ptrify PPCallbacks ownership.

[Craig Topper]

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. But given that I find that kind of ugly I think I'll revert the
interface part of the change.


On Wed, Sep 10, 2014 at 12:13 PM, Kim Gräsman <kim.grasman at gmail.com> wrote:

> Hi Craig,
>
> On Wed, Sep 10, 2014 at 6:53 AM, Craig Topper <craig.topper at gmail.com>
> wrote:
> > Author: ctopper
> > Date: Tue Sep  9 23:53:53 2014
> > New Revision: 217474
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=217474&view=rev
> > Log:
> > Unique_ptrify PPCallbacks ownership.
> >
> > Unique_ptr creation stil needs to be moved earlier at some of the call
> sites.
>
> I'm not happy with this change at all. It precludes a PPCallbacks also
> serving other duties. E.g. in IWYU we have:
>
>     IwyuPreprocessorInfo* const preprocessor_consumer
>         = new IwyuPreprocessorInfo();
>     compiler.getPreprocessor().addPPCallbacks(preprocessor_consumer);  //
> here
>     compiler.getPreprocessor().addCommentHandler(preprocessor_consumer);
>  // here
>
>     VisitorState* const visitor_state
>         = new VisitorState(&compiler, *preprocessor_consumer);  // and here
>     return std::unique_ptr<IwyuAstConsumer>(new
> IwyuAstConsumer(visitor_state));
>
> Our PPCallbacks impl also happens to be a comment handler, plus we
> want to share the data it's collected with our AST consumer.
>
> The unique_ptr completely kills this scenario, and alternative
> designed seem much more complex.
>
> Would you consider reverting?
>
> Thanks,
> - Kim
>



-- 
~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140910/ed06cdfc/attachment.html>