On Thu, Sep 18, 2014 at 8:53 PM, David Blaikie <[email protected]> wrote: > ================ > Comment at: clang-tidy/google/TodoCommentCheck.cpp:58 > @@ +57,3 @@ > +TodoCommentCheck::TodoCommentCheck(StringRef Name, ClangTidyContext *Context) > + : ClangTidyCheck(Name, Context), Handler(new TodoCommentHandler(*this)) > {} > + > ---------------- > FWIW I tend to use llvm::make_unique even in init lists like this - it > reassures me when I go back to read it that the member is actually a > unique_ptr, not a raw pointer I might need to pay attention to. > > ================ > Comment at: clang-tidy/google/TodoCommentCheck.cpp:60 > @@ -59,3 +62,3 @@ > void TodoCommentCheck::registerPPCallbacks(CompilerInstance &Compiler) { > - Compiler.getPreprocessor().addCommentHandler(new > TodoCommentHandler(*this)); > } > ---------------- > might be nice to have addCommentHandler take by reference... maybe, one day, > etc.
+1. Would it be possible to make addPPCallbacks non-owning too, so that these two callback registrations work the same? I'd be happy to cook up a patch here, because the ownership semantics for these preprocessor callbacks is confusing, but I'm guessing there's some reason addPPCallbacks needs to be owning (e.g. PPChainedCallbacks). - Kim _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
