On Sep 10, 2014 10:27 PM, "Craig Topper" <[email protected]> wrote: > > 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.
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. > 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 >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
