ckennelly updated this revision to Diff 301648. ckennelly added a comment. Apply feedback from ymandel
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90392/new/ https://reviews.llvm.org/D90392 Files: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp +++ clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp @@ -96,34 +96,28 @@ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique] // CHECK-FIXES: P1 = std::make_unique<int>(); - // Without parenthesis. + // Without parenthesis, default initialization. std::unique_ptr<int> P2 = std::unique_ptr<int>(new int); - // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique] - // CHECK-FIXES: std::unique_ptr<int> P2 = std::make_unique<int>(); P2.reset(new int); - // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique] - // CHECK-FIXES: P2 = std::make_unique<int>(); P2 = std::unique_ptr<int>(new int); - // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique] - // CHECK-FIXES: P2 = std::make_unique<int>(); // With auto. auto P3 = std::unique_ptr<int>(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead // CHECK-FIXES: auto P3 = std::make_unique<int>(); - std::unique_ptr<int> P4 = std::unique_ptr<int>((new int)); + std::unique_ptr<int> P4 = std::unique_ptr<int>((new int())); // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique] // CHECK-FIXES: std::unique_ptr<int> P4 = std::make_unique<int>(); - P4.reset((new int)); + P4.reset((new int())); // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique] // CHECK-FIXES: P4 = std::make_unique<int>(); - std::unique_ptr<int> P5 = std::unique_ptr<int>((((new int)))); + std::unique_ptr<int> P5 = std::unique_ptr<int>((((new int())))); // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique] // CHECK-FIXES: std::unique_ptr<int> P5 = std::make_unique<int>(); - P5.reset(((((new int))))); + P5.reset(((((new int()))))); // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique] // CHECK-FIXES: P5 = std::make_unique<int>(); @@ -137,6 +131,8 @@ Q = unique_ptr<int>(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead // CHECK-FIXES: Q = std::make_unique<int>(); + + Q = unique_ptr<int>(new int); } std::unique_ptr<int> R(new int()); @@ -446,12 +442,12 @@ // CHECK-FIXES: FFs = std::make_unique<Foo[]>(Num2); std::unique_ptr<int[]> FI; - FI.reset(new int[5]()); // default initialization. + FI.reset(new int[5]()); // value initialization. // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: // CHECK-FIXES: FI = std::make_unique<int[]>(5); // The check doesn't give warnings and fixes for cases where the original new - // expression doesn't do any initialization. + // expression does default initialization. FI.reset(new int[5]); FI.reset(new int[Num]); FI.reset(new int[Num2]); @@ -459,13 +455,13 @@ void aliases() { typedef std::unique_ptr<int> IntPtr; - IntPtr Typedef = IntPtr(new int); + IntPtr Typedef = IntPtr(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use std::make_unique instead // CHECK-FIXES: IntPtr Typedef = std::make_unique<int>(); // We use 'bool' instead of '_Bool'. typedef std::unique_ptr<bool> BoolPtr; - BoolPtr BoolType = BoolPtr(new bool); + BoolPtr BoolType = BoolPtr(new bool()); // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: use std::make_unique instead // CHECK-FIXES: BoolPtr BoolType = std::make_unique<bool>(); @@ -476,12 +472,12 @@ // CHECK-FIXES: BasePtr StructType = std::make_unique<Base>(); #define PTR unique_ptr<int> - std::unique_ptr<int> Macro = std::PTR(new int); + std::unique_ptr<int> Macro = std::PTR(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead // CHECK-FIXES: std::unique_ptr<int> Macro = std::make_unique<int>(); #undef PTR - std::unique_ptr<int> Using = unique_ptr_<int>(new int); + std::unique_ptr<int> Using = unique_ptr_<int>(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead // CHECK-FIXES: std::unique_ptr<int> Using = std::make_unique<int>(); } @@ -505,7 +501,7 @@ Nest.reset(new std::unique_ptr<int>(new int)); // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead // CHECK-FIXES: Nest = std::make_unique<std::unique_ptr<int>>(new int); - Nest->reset(new int); + Nest->reset(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead // CHECK-FIXES: *Nest = std::make_unique<int>(); } Index: clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp +++ clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp @@ -51,16 +51,14 @@ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_shared instead [modernize-make-shared] // CHECK-FIXES: P1 = std::make_shared<int>(); - // Without parenthesis. + // Without parenthesis, default initialization. std::shared_ptr<int> P2 = std::shared_ptr<int>(new int); - // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_shared instead [modernize-make-shared] - // CHECK-FIXES: std::shared_ptr<int> P2 = std::make_shared<int>(); - P2.reset(new int); + P2.reset(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_shared instead [modernize-make-shared] // CHECK-FIXES: P2 = std::make_shared<int>(); - P2 = std::shared_ptr<int>(new int); + P2 = std::shared_ptr<int>(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_shared instead [modernize-make-shared] // CHECK-FIXES: P2 = std::make_shared<int>(); @@ -69,7 +67,7 @@ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_shared instead // CHECK-FIXES: auto P3 = std::make_shared<int>(); - std::shared_ptr<int> P4 = std::shared_ptr<int>((new int)); + std::shared_ptr<int> P4 = std::shared_ptr<int>((new int())); // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_shared instead [modernize-make-shared] // CHECK-FIXES: std::shared_ptr<int> P4 = std::make_shared<int>(); @@ -77,7 +75,7 @@ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_shared instead [modernize-make-shared] // CHECK-FIXES: P4 = std::make_shared<int>(); - P4 = std::shared_ptr<int>(((new int))); + P4 = std::shared_ptr<int>(((new int()))); // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_shared instead [modernize-make-shared] // CHECK-FIXES: P4 = std::make_shared<int>(); @@ -231,13 +229,13 @@ void aliases() { typedef std::shared_ptr<int> IntPtr; - IntPtr Typedef = IntPtr(new int); + IntPtr Typedef = IntPtr(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use std::make_shared instead // CHECK-FIXES: IntPtr Typedef = std::make_shared<int>(); // We use 'bool' instead of '_Bool'. typedef std::shared_ptr<bool> BoolPtr; - BoolPtr BoolType = BoolPtr(new bool); + BoolPtr BoolType = BoolPtr(new bool()); // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: use std::make_shared instead // CHECK-FIXES: BoolPtr BoolType = std::make_shared<bool>(); @@ -248,12 +246,12 @@ // CHECK-FIXES: BasePtr StructType = std::make_shared<Base>(); #define PTR shared_ptr<int> - std::shared_ptr<int> Macro = std::PTR(new int); + std::shared_ptr<int> Macro = std::PTR(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_shared instead // CHECK-FIXES: std::shared_ptr<int> Macro = std::make_shared<int>(); #undef PTR - std::shared_ptr<int> Using = shared_ptr_<int>(new int); + std::shared_ptr<int> Using = shared_ptr_<int>(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_shared instead // CHECK-FIXES: std::shared_ptr<int> Using = std::make_shared<int>(); } @@ -277,7 +275,7 @@ Nest.reset(new std::shared_ptr<int>(new int)); // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_shared instead // CHECK-FIXES: Nest = std::make_shared<std::shared_ptr<int>>(new int); - Nest->reset(new int); + Nest->reset(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_shared instead // CHECK-FIXES: *Nest = std::make_shared<int>(); } Index: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp +++ clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "../utils/TypeTraits.h" #include "MakeSharedCheck.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Lex/Lexer.h" @@ -120,14 +121,18 @@ if (New->getType()->getPointeeType()->getContainedAutoType()) return; - // Be conservative for cases where we construct an array without any - // initialization. + // Be conservative for cases where we construct and default initialize. + // // For example, + // P.reset(new int) // check fix: P = std::make_unique<int>() // P.reset(new int[5]) // check fix: P = std::make_unique<int []>(5) // - // The fix of the check has side effect, it introduces default initialization + // The fix of the check has side effect, it introduces value initialization // which maybe unexpected and cause performance regression. - if (New->isArray() && !New->hasInitializer()) + const bool Initializes = New->hasInitializer() || + !utils::type_traits::isTriviallyDefaultConstructible( + New->getAllocatedType(), *Result.Context); + if (!Initializes) return; if (Construct) checkConstruct(SM, Result.Context, Construct, Type, New);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits