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 <[email protected]> wrote: > Hi Craig, > > On Wed, Sep 10, 2014 at 6:53 AM, Craig Topper <[email protected]> > 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
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
