Author: alexfh Date: Tue Oct 9 08:58:18 2018 New Revision: 344058 URL: http://llvm.org/viewvc/llvm-project?rev=344058&view=rev Log: [clang-tidy] Fix handling of parens around new expressions in make_<smartptr> checks.
Summary: Extra parentheses around a new expression result in incorrect code after applying fixes. Reviewers: hokein Reviewed By: hokein Subscribers: xazax.hun, cfe-commits Differential Revision: https://reviews.llvm.org/D52989 Modified: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h clang-tools-extra/trunk/test/clang-tidy/modernize-make-shared.cpp clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp Modified: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp?rev=344058&r1=344057&r2=344058&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp Tue Oct 9 08:58:18 2018 @@ -21,6 +21,9 @@ namespace modernize { namespace { constexpr char StdMemoryHeader[] = "memory"; +constexpr char ConstructorCall[] = "constructorCall"; +constexpr char ResetCall[] = "resetCall"; +constexpr char NewExpression[] = "newExpression"; std::string GetNewExprName(const CXXNewExpr *NewExpr, const SourceManager &SM, @@ -30,7 +33,7 @@ std::string GetNewExprName(const CXXNewE NewExpr->getAllocatedTypeSourceInfo()->getTypeLoc().getSourceRange()), SM, Lang); if (NewExpr->isArray()) { - return WrittenName.str() + "[]"; + return (WrittenName + "[]").str(); } return WrittenName.str(); } @@ -38,9 +41,6 @@ std::string GetNewExprName(const CXXNewE } // namespace const char MakeSmartPtrCheck::PointerType[] = "pointerType"; -const char MakeSmartPtrCheck::ConstructorCall[] = "constructorCall"; -const char MakeSmartPtrCheck::ResetCall[] = "resetCall"; -const char MakeSmartPtrCheck::NewExpression[] = "newExpression"; MakeSmartPtrCheck::MakeSmartPtrCheck(StringRef Name, ClangTidyContext* Context, @@ -68,8 +68,8 @@ bool MakeSmartPtrCheck::isLanguageVersio void MakeSmartPtrCheck::registerPPCallbacks(CompilerInstance &Compiler) { if (isLanguageVersionSupported(getLangOpts())) { - Inserter.reset(new utils::IncludeInserter( - Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle)); + Inserter = llvm::make_unique<utils::IncludeInserter>( + Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle); Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks()); } } @@ -122,12 +122,12 @@ void MakeSmartPtrCheck::check(const Matc return; if (Construct) - checkConstruct(SM, Construct, Type, New); + checkConstruct(SM, Result.Context, Construct, Type, New); else if (Reset) - checkReset(SM, Reset, New); + checkReset(SM, Result.Context, Reset, New); } -void MakeSmartPtrCheck::checkConstruct(SourceManager &SM, +void MakeSmartPtrCheck::checkConstruct(SourceManager &SM, ASTContext *Ctx, const CXXConstructExpr *Construct, const QualType *Type, const CXXNewExpr *New) { @@ -154,7 +154,7 @@ void MakeSmartPtrCheck::checkConstruct(S return; } - if (!replaceNew(Diag, New, SM)) { + if (!replaceNew(Diag, New, SM, Ctx)) { return; } @@ -193,7 +193,7 @@ void MakeSmartPtrCheck::checkConstruct(S insertHeader(Diag, SM.getFileID(ConstructCallStart)); } -void MakeSmartPtrCheck::checkReset(SourceManager &SM, +void MakeSmartPtrCheck::checkReset(SourceManager &SM, ASTContext *Ctx, const CXXMemberCallExpr *Reset, const CXXNewExpr *New) { const auto *Expr = cast<MemberExpr>(Reset->getCallee()); @@ -224,7 +224,7 @@ void MakeSmartPtrCheck::checkReset(Sourc return; } - if (!replaceNew(Diag, New, SM)) { + if (!replaceNew(Diag, New, SM, Ctx)) { return; } @@ -241,10 +241,24 @@ void MakeSmartPtrCheck::checkReset(Sourc } bool MakeSmartPtrCheck::replaceNew(DiagnosticBuilder &Diag, - const CXXNewExpr *New, - SourceManager& SM) { - SourceLocation NewStart = New->getSourceRange().getBegin(); - SourceLocation NewEnd = New->getSourceRange().getEnd(); + 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; + } + } + } + return E; + }; + + SourceRange NewRange = SkipParensParents(New)->getSourceRange(); + SourceLocation NewStart = NewRange.getBegin(); + SourceLocation NewEnd = NewRange.getEnd(); // Skip when the source location of the new expression is invalid. if (NewStart.isInvalid() || NewEnd.isInvalid()) Modified: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h?rev=344058&r1=344057&r2=344058&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h (original) +++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h Tue Oct 9 08:58:18 2018 @@ -44,9 +44,6 @@ protected: virtual bool isLanguageVersionSupported(const LangOptions &LangOpts) const; static const char PointerType[]; - static const char ConstructorCall[]; - static const char ResetCall[]; - static const char NewExpression[]; private: std::unique_ptr<utils::IncludeInserter> Inserter; @@ -55,14 +52,15 @@ private: 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 checkConstruct(SourceManager &SM, ASTContext *Ctx, + const CXXConstructExpr *Construct, const QualType *Type, + const CXXNewExpr *New); + void checkReset(SourceManager &SM, ASTContext *Ctx, + const CXXMemberCallExpr *Member, const CXXNewExpr *New); /// Returns true when the fixes for replacing CXXNewExpr are generated. bool replaceNew(DiagnosticBuilder &Diag, const CXXNewExpr *New, - SourceManager &SM); + SourceManager &SM, ASTContext *Ctx); void insertHeader(DiagnosticBuilder &Diag, FileID FD); }; Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-make-shared.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-make-shared.cpp?rev=344058&r1=344057&r2=344058&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/modernize-make-shared.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/modernize-make-shared.cpp Tue Oct 9 08:58:18 2018 @@ -70,6 +70,18 @@ void basic() { // 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)); + // 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>(); + + P4.reset((((new int())))); + // 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))); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_shared instead [modernize-make-shared] + // CHECK-FIXES: P4 = std::make_shared<int>(); + { // No std. using namespace std; Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp?rev=344058&r1=344057&r2=344058&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp Tue Oct 9 08:58:18 2018 @@ -110,6 +110,19 @@ void basic() { // 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)); + // 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)); + // 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)))); + // 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))))); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique] + // CHECK-FIXES: P5 = std::make_unique<int>(); + { // No std. using namespace std; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
