[PATCH] [clang-tidy] Don't leak the TodoCommentHandler object

Kim Gräsman kim.grasman at gmail.com
Thu Sep 18 12:04:06 PDT 2014


On Thu, Sep 18, 2014 at 8:53 PM, David Blaikie <dblaikie at gmail.com> 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



More information about the cfe-commits mailing list