r217474 - Unique_ptrify PPCallbacks ownership.
Craig Topper
craig.topper at gmail.com
Wed Sep 10 22:19:22 PDT 2014
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>
More information about the cfe-commits
mailing list