[clang-tidy] Ignore macros in make-unique check.

[hokein]

Summary

The check doesn't fully support smart-ptr usages inside macros, which
may cause incorrect fixes, or even crashes, ignore them for now.

[alexfh]

LG with one comment.

clang-tidy/modernize/MakeSmartPtrCheck.cpp

116

Seems to be a bit too broad. Maybe just disable the fix in macros? If warnings without fixes will be a problem in some codebases, we could then introduce the IgnoreMacros option as in readability-redundant-declaration, modernize-use-using, modernize-use-bool-literals, and modernize-use-default-member-init.

[hokein]

Add IgnoreMacros option.

[hokein]

Add a missing test file.

[hokein]

Please take another look.

clang-tidy/modernize/MakeSmartPtrCheck.cpp

116

The idea sounds good to me. Done.

If warnings without fixes will be a problem in some codebases, we could then introduce the IgnoreMacros option

Didn't know there is IgnoreMacros option, but we are missing the option document for these checks. Will add them afterwards.

[alexfh]

LG with a couple of nits.

clang-tidy/modernize/MakeSmartPtrCheck.cpp

54

Let's go with getLocalOrGlobal to be able to set the option for all checks at once. Other checks having this option do this already.

test/clang-tidy/modernize-make-unique-macros.cpp

14

Please add CHECK-FIXES to verify that the code remains intact (both macro definition and expansion).

[hokein]

Address review comments.

clang-tidy/modernize/MakeSmartPtrCheck.h

protected:
   static const char ResetCall[];
   static const char NewExpression[];
 
 private:
   std::unique_ptr<utils::IncludeInserter> Inserter;
   const utils::IncludeSorter::IncludeStyle IncludeStyle;
   const std::string MakeSmartPtrFunctionHeader;
   const std::string MakeSmartPtrFunctionName;
+  const bool IgnoreMacros;
 
   void checkConstruct(SourceManager &SM, const CXXConstructExpr *Construct,
                       const QualType *Type, const CXXNewExpr *New);
   void checkReset(SourceManager &SM, const CXXMemberCallExpr *Member,
                   const CXXNewExpr *New);
 
   void replaceNew(DiagnosticBuilder &Diag, const CXXNewExpr *New,
                   SourceManager &SM);
   void insertHeader(DiagnosticBuilder &Diag, FileID FD);
 };
 
 } // namespace modernize
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MAKE_SMART_PTR_H

clang-tidy/modernize/MakeSmartPtrCheck.cpp

 MakeSmartPtrCheck::MakeSmartPtrCheck(StringRef Name, ClangTidyContext *Context,
                                      StringRef MakeSmartPtrFunctionName)
     : ClangTidyCheck(Name, Context),
       IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
           Options.getLocalOrGlobal("IncludeStyle", "llvm"))),
       MakeSmartPtrFunctionHeader(
           Options.get("MakeSmartPtrFunctionHeader", StdMemoryHeader)),
       MakeSmartPtrFunctionName(
-          Options.get("MakeSmartPtrFunction", MakeSmartPtrFunctionName)) {}
+          Options.get("MakeSmartPtrFunction", MakeSmartPtrFunctionName)),
+      IgnoreMacros(Options.get("IgnoreMacros", true)) {}
 
 void MakeSmartPtrCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IncludeStyle", IncludeStyle);
   Options.store(Opts, "MakeSmartPtrFunctionHeader", MakeSmartPtrFunctionHeader);
   Options.store(Opts, "MakeSmartPtrFunction", MakeSmartPtrFunctionName);
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
 void MakeSmartPtrCheck::registerPPCallbacks(CompilerInstance &Compiler) {
   if (getLangOpts().CPlusPlus11) {
     Inserter.reset(new utils::IncludeInserter(
         Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle));
     Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks());
   }
const auto *Construct =
       Result.Nodes.getNodeAs<CXXConstructExpr>(ConstructorCall);
   const auto *Reset = Result.Nodes.getNodeAs<CXXMemberCallExpr>(ResetCall);
   const auto *Type = Result.Nodes.getNodeAs<QualType>(PointerType);
   const auto *New = Result.Nodes.getNodeAs<CXXNewExpr>(NewExpression);
 
   if (New->getNumPlacementArgs() != 0)
     return;
 
   if (Construct)
     checkConstruct(SM, Construct, Type, New);
   else if (Reset)
     checkReset(SM, Reset, New);
 }
 
 void MakeSmartPtrCheck::checkConstruct(SourceManager &SM,
                                        const CXXConstructExpr *Construct,
                                        const QualType *Type,
                                        const CXXNewExpr *New) {
   SourceLocation ConstructCallStart = Construct->getExprLoc();
+  bool InMacro = ConstructCallStart.isMacroID();
+
+  if (InMacro && IgnoreMacros) {
+    return;
+  }
 
   bool Invalid = false;
   StringRef ExprStr = Lexer::getSourceText(
       CharSourceRange::getCharRange(
           ConstructCallStart, Construct->getParenOrBraceRange().getBegin()),
       SM, getLangOpts(), &Invalid);
   if (Invalid)
     return;
 
   auto Diag = diag(ConstructCallStart, "use %0 instead")
               << MakeSmartPtrFunctionName;
 
+  // Disable the fix in macros.
+  if (InMacro) {
+    return;
+  }
+
   // Find the location of the template's left angle.
   size_t LAngle = ExprStr.find("<");
   SourceLocation ConstructCallEnd;
   if (LAngle == StringRef::npos) {
     // If the template argument is missing (because it is part of the alias)
     // we have to add it back.
     ConstructCallEnd = ConstructCallStart.getLocWithOffset(ExprStr.size());
     Diag << FixItHint::CreateInsertion(
void MakeSmartPtrCheck::checkReset(SourceManager &SM,
                                    const CXXNewExpr *New) {
   const auto *Expr = cast<MemberExpr>(Reset->getCallee());
   SourceLocation OperatorLoc = Expr->getOperatorLoc();
   SourceLocation ResetCallStart = Reset->getExprLoc();
   SourceLocation ExprStart = Expr->getLocStart();
   SourceLocation ExprEnd =
       Lexer::getLocForEndOfToken(Expr->getLocEnd(), 0, SM, getLangOpts());
 
+  bool InMacro = ExprStart.isMacroID();
+
+  if (InMacro && IgnoreMacros) {
+    return;
+  }
+
   auto Diag = diag(ResetCallStart, "use %0 instead")
               << MakeSmartPtrFunctionName;
 
+  // Disable the fix in macros.
+  if (InMacro) {
+    return;
+  }
+
   Diag << FixItHint::CreateReplacement(
       CharSourceRange::getCharRange(OperatorLoc, ExprEnd),
       (llvm::Twine(" = ") + MakeSmartPtrFunctionName + "<" +
        GetNewExprName(New, SM, getLangOpts()) + ">")
           .str());
 
   if (Expr->isArrow())
     Diag << FixItHint::CreateInsertion(ExprStart, "*");

docs/clang-tidy/checks/modernize-make-unique.rst

.. option:: MakeSmartPtrFunctionHeader
 
    A string specifying the corresponding header of make-unique-ptr function.
    Default is `memory`.
 
 .. option:: IncludeStyle
 
    A string specifying which include-style is used, `llvm` or `google`. Default
    is `llvm`.
+
+.. option:: IgnoreMacros
+
+   If set to non-zero, the check will not give warnings inside macros. Default
+   is `1`.

test/clang-tidy/modernize-make-unique-macros.cpp

+// RUN: %check_clang_tidy %s modernize-make-unique %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-make-unique.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -std=c++11  -I%S/Inputs/modernize-smart-ptr
+
+#include "unique_ptr.h"
+
+class Foo {};
+class Bar {};
+#define DEFINE(...) __VA_ARGS__
+template<typename T>
+void g2(std::unique_ptr<Foo> *t) {
+  DEFINE(
+      auto p = std::unique_ptr<Foo>(new Foo);
+      // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use std::make_unique instead
+      t->reset(new Foo);
+      // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_unique instead
+      );
+}
+void macro() {
+  std::unique_ptr<Foo> *t;
+  g2<Bar>(t);
+}
+#undef DEFINE

test/clang-tidy/modernize-make-unique.cpp

void reset() {
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use std::make_unique instead
   // CHECK-FIXES: P = std::make_unique<int>();
 
   auto Q = &P;
   Q->reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead
   // CHECK-FIXES: *Q = std::make_unique<int>();
 }
+
+#define DEFINE(...) __VA_ARGS__
+template<typename T>
+void g2(std::unique_ptr<Foo> *t) {
+  DEFINE(auto p = std::unique_ptr<Foo>(new Foo); t->reset(new Foo););
+}
+void macro() {
+  std::unique_ptr<Foo> *t;
+  g2<bar::Bar>(t);
+}
+#undef DEFINE