r217474 - Unique_ptrify PPCallbacks ownership.

Kim Gräsman kim.grasman at gmail.com
Wed Sep 10 12:13:46 PDT 2014


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



More information about the cfe-commits mailing list