From 547f89d607044004969f2556e1d10770917b753d Mon Sep 17 00:00:00 2001 From: Haojian Wu Date: Thu, 18 Oct 2018 09:13:34 +0000 Subject: [PATCH] [clang-tidy] Ignore a case where the fix of make_unique check introduces side effect. Summary: Previously, ptr.reset(new char[5]) will be replaced with `p = make_unique(5)`, the fix has side effect -- doing default initialization, it may cause performace regression (we are bitten by this rececntly) The check should be conservative for these cases. Reviewers: alexfh Subscribers: xazax.hun, cfe-commits Differential Revision: https://reviews.llvm.org/D53377 llvm-svn: 344733 --- .../clang-tidy/modernize/MakeSmartPtrCheck.cpp | 9 +++++++++ clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp | 13 +++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp index df5e7735bf1..f14251335b6 100644 --- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp @@ -121,6 +121,15 @@ void MakeSmartPtrCheck::check(const MatchFinder::MatchResult &Result) { if (New->getNumPlacementArgs() != 0) 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(5) + // + // The fix of the check has side effect, it introduces default initialization + // which maybe unexpected and cause performance regression. + if (New->isArray() && !New->hasInitializer()) + return; if (Construct) checkConstruct(SM, Result.Context, Construct, Type, New); else if (Reset) diff --git a/clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp b/clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp index d2a73f3f311..aeeb6dbd809 100644 --- a/clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp +++ b/clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp @@ -403,18 +403,15 @@ void initialization(int T, Base b) { // CHECK-FIXES: FFs = std::make_unique(Num2); std::unique_ptr FI; - FI.reset(new int[5]); - // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: - // CHECK-FIXES: FI = std::make_unique(5); - FI.reset(new int[5]()); + FI.reset(new int[5]()); // default initialization. // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: // CHECK-FIXES: FI = std::make_unique(5); + + // The check doesn't give warnings and fixes for cases where the original new + // expresion doesn't do any initialization. + FI.reset(new int[5]); FI.reset(new int[Num]); - // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: - // CHECK-FIXES: FI = std::make_unique(Num); FI.reset(new int[Num2]); - // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: - // CHECK-FIXES: FI = std::make_unique(Num2); } void aliases() { -- 2.11.0