[clang-tidy] No fixes for auto new expression in smart check

[hokein]

Summary

The fix for auto new expression is illegal.

[aaron.ballman]

clang-tidy/modernize/MakeSmartPtrCheck.cpp

258

Drop the quotation marks.

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

288

It seems like we could also generate the correct replacement for the user here.

PE1 = std::make_unique<decltype(E())>();

However, I feel like this isn't worth it in general and the correct behavior is to not diagnose in this situation in the first place -- I don't think anyone will find the make_unique<> version to be an improvement over the new auto() version. What do you think?

[hokein]

No diagnose this case as well.

[hokein]

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

288

This seems reasonable to me. Also the new auto() is very rare even in our monolithic repository.

[aaron.ballman]

LGTM with a minor typo to correct.

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

285

warninags -> warnings

[hokein]

Fix a typo.

[hokein]

Thanks for the quick review.

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

285

oops...

clang-tidy/modernize/MakeSmartPtrCheck.cpp

void MakeSmartPtrCheck::check(const MatchFinder::MatchResult &Result) {
   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;
+  // Skip when this is a new-expression with `auto`, e.g. new auto(1)
+  if (New->getType()->getPointeeType()->getContainedAutoType())
+    return;
 
   // Be conservative for cases where we construct an array without any
   // initalization.
   // For example,
   //    P.reset(new int[5]) // check fix: P = make_unique<int []>(5)
   //
   // The fix of the check has side effect, it introduces default initialization
   // which maybe unexpected and cause performance regression.
if (Expr->isArrow())
     Diag << FixItHint::CreateInsertion(ExprStart, "*");
 
   insertHeader(Diag, SM.getFileID(OperatorLoc));
 }
 
 bool MakeSmartPtrCheck::replaceNew(DiagnosticBuilder &Diag,
                                    const CXXNewExpr *New, SourceManager &SM,
                                    ASTContext *Ctx) {
   auto SkipParensParents = [&](const Expr *E) {
     for (const Expr *OldE = nullptr; E != OldE;) {
       OldE = E;
       for (const auto &Node : Ctx->getParents(*E)) {
         if (const Expr *Parent = Node.get<ParenExpr>()) {
           E = Parent;
           break;
         }
       }

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

void initialization(int T, Base b) {
   // Initialization with default constructor.
   std::unique_ptr<E> PE1 = std::unique_ptr<E>(new E{});
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
   // CHECK-FIXES: std::unique_ptr<E> PE1 = std::make_unique<E>();
   PE1.reset(new E{});
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
   // CHECK-FIXES: PE1 = std::make_unique<E>();
 
+  // No warnings for `auto` new expression.
+  PE1.reset(new auto(E()));
+
   //============================================================================
   //  NOTE: For initlializer-list constructors, the check only gives warnings,
   //  and no fixes are generated.
   //============================================================================
 
   // Initialization with the initializer-list constructor.
   std::unique_ptr<E> PE2 = std::unique_ptr<E>(new E{1, 2});
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
   // CHECK-FIXES: std::unique_ptr<E> PE2 = std::unique_ptr<E>(new E{1, 2});