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

[hans]

Summary

Preprocessor:addCommentHandler() does not take ownership, so we'd end up leaking the TodoCommentHandler.

This patch makes it owned by the Check object.

[bkramer]

Yikes, another API with non-obvious ownership semantics. Thanks for catching this.

Patch looks good.

[dblaikie]

clang-tidy/google/TodoCommentCheck.cpp

58 ↗

(On Diff #13839)

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.

60 ↗

(On Diff #13839)

might be nice to have addCommentHandler take by reference... maybe, one day, etc.

[hans]

Closed by commit rL218068 (authored by @hans).

clang-tools-extra/trunk/clang-tidy/google/TodoCommentCheck.h

 namespace tidy {
 namespace readability {
 
 /// \brief Finds TODO comments without a username or bug number.
 ///
 /// Corresponding cpplint.py check: readability/todo
 class TodoCommentCheck : public ClangTidyCheck {
 public:
-  TodoCommentCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  TodoCommentCheck(StringRef Name, ClangTidyContext *Context);
+  ~TodoCommentCheck();
   void registerPPCallbacks(CompilerInstance &Compiler) override;
+
+private:
+  class TodoCommentHandler;
+  std::unique_ptr<TodoCommentHandler> Handler;
 };
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_TODOCOMMENTCHECK_H

clang-tools-extra/trunk/clang-tidy/google/TodoCommentCheck.cpp

 #include "TodoCommentCheck.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/Preprocessor.h"
 
 namespace clang {
 namespace tidy {
 namespace readability {
 
-namespace {
-class TodoCommentHandler : public CommentHandler {
+class TodoCommentCheck::TodoCommentHandler : public CommentHandler {
 public:
   explicit TodoCommentHandler(TodoCommentCheck &Check)
       : Check(Check), TodoMatch("^// *TODO(\\(.*\\))?:?( )?(.*)$") {}
 
   bool HandleComment(Preprocessor &PP, SourceRange Range) override {
     StringRef Text =
         Lexer::getSourceText(CharSourceRange::getCharRange(Range),
                              PP.getSourceManager(), PP.getLangOpts());
Check.diag(Range.getBegin(), "missing username/bug in TODO")
                                         NewText);
     return false;
   }
 
 private:
   TodoCommentCheck &Check;
   llvm::Regex TodoMatch;
 };
-} // namespace
+
+TodoCommentCheck::TodoCommentCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      Handler(llvm::make_unique<TodoCommentHandler>(*this)) {}
+
+TodoCommentCheck::~TodoCommentCheck() {}
 
 void TodoCommentCheck::registerPPCallbacks(CompilerInstance &Compiler) {
-  Compiler.getPreprocessor().addCommentHandler(new TodoCommentHandler(*this));
+  Compiler.getPreprocessor().addCommentHandler(Handler.get());
 }
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang