[clang-tools-extra] r344058 - [clang-tidy] Fix handling of parens around new expressions in make_<smartptr> checks.

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 9 08:58:18 PDT 2018


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;




More information about the cfe-commits mailing list