loskutov updated this revision to Diff 552164.
loskutov added a comment.

git clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158371/new/

https://reviews.llvm.org/D158371

Files:
  clang-tools-extra/clang-tidy/bugprone/DanglingHandleCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp
@@ -1,8 +1,12 @@
-// RUN: %check_clang_tidy -std=c++11,c++14 %s bugprone-dangling-handle %t -- \
+// RUN: %check_clang_tidy -std=c++11,c++14 -check-suffix=,CXX14 %s bugprone-dangling-handle %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN:             {bugprone-dangling-handle.HandleClasses: \
+// RUN:               'std::basic_string_view; ::llvm::StringRef;'}}"
+
+// RUN: %check_clang_tidy -std=c++17-or-later -check-suffix=,CXX17 %s bugprone-dangling-handle %t -- \
 // RUN:   -config="{CheckOptions: \
 // RUN:             {bugprone-dangling-handle.HandleClasses: \
 // RUN:               'std::basic_string_view; ::llvm::StringRef;'}}"
-// FIXME: Fix the checker to work in C++17 or later mode.
 
 namespace std {
 
@@ -84,27 +88,32 @@
 
 void Positives() {
   std::string_view view1 = std::string();
-  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: std::basic_string_view outlives its value [bugprone-dangling-handle]
+  // CHECK-MESSAGES-CXX14: [[@LINE-1]]:20: warning: std::basic_string_view outlives its value [bugprone-dangling-handle]
+  // CHECK-MESSAGES-CXX17: [[@LINE-2]]:28: warning: std::basic_string_view outlives its value [bugprone-dangling-handle]
 
   std::string_view view_2 = ReturnsAString();
-  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: std::basic_string_view outlives
+  // CHECK-MESSAGES-CXX14: [[@LINE-1]]:20: warning: std::basic_string_view outlives its value [bugprone-dangling-handle]
+  // CHECK-MESSAGES-CXX17: [[@LINE-2]]:29: warning: std::basic_string_view outlives its value [bugprone-dangling-handle]
 
   view1 = std::string();
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives
 
   const std::string& str_ref = "";
   std::string_view view3 = true ? "A" : str_ref;
-  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: std::basic_string_view outlives
+  // CHECK-MESSAGES-CXX14: [[@LINE-1]]:20: warning: std::basic_string_view outlives
+  // CHECK-MESSAGES-CXX17: [[@LINE-2]]:28: warning: std::basic_string_view outlives
   view3 = true ? "A" : str_ref;
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives
 
   std::string_view view4(ReturnsAString());
-  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: std::basic_string_view outlives
+  // CHECK-MESSAGES-CXX14: [[@LINE-1]]:20: warning: std::basic_string_view outlives
+  // CHECK-MESSAGES-CXX17: [[@LINE-2]]:26: warning: std::basic_string_view outlives
 }
 
 void OtherTypes() {
   llvm::StringRef ref = std::string();
-  // CHECK-MESSAGES: [[@LINE-1]]:19: warning: llvm::StringRef outlives its value
+  // CHECK-MESSAGES-CXX14: [[@LINE-1]]:19: warning: llvm::StringRef outlives its value
+  // CHECK-MESSAGES-CXX17: [[@LINE-2]]:25: warning: llvm::StringRef outlives its value
 }
 
 const char static_array[] = "A";
Index: clang-tools-extra/clang-tidy/bugprone/DanglingHandleCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/DanglingHandleCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/DanglingHandleCheck.cpp
@@ -36,9 +36,9 @@
   // If a ternary operator returns a temporary value, then both branches hold a
   // temporary value. If one of them is not a temporary then it must be copied
   // into one to satisfy the type of the operator.
-  const auto TemporaryTernary =
-      conditionalOperator(hasTrueExpression(cxxBindTemporaryExpr()),
-                          hasFalseExpression(cxxBindTemporaryExpr()));
+  const auto TemporaryTernary = conditionalOperator(
+      hasTrueExpression(ignoringParenImpCasts(cxxBindTemporaryExpr())),
+      hasFalseExpression(ignoringParenImpCasts(cxxBindTemporaryExpr())));
 
   return handleFrom(IsAHandle, anyOf(cxxBindTemporaryExpr(), TemporaryTernary));
 }
@@ -103,26 +103,17 @@
 void DanglingHandleCheck::registerMatchersForVariables(MatchFinder *Finder) {
   const auto ConvertedHandle = handleFromTemporaryValue(IsAHandle);
 
-  // Find 'Handle foo(ReturnsAValue());'
+  // Find 'Handle foo(ReturnsAValue());', 'Handle foo = ReturnsAValue();'
   Finder->addMatcher(
       varDecl(hasType(hasUnqualifiedDesugaredType(
                   recordType(hasDeclaration(cxxRecordDecl(IsAHandle))))),
+              unless(parmVarDecl()),
               hasInitializer(
-                  exprWithCleanups(has(ignoringParenImpCasts(ConvertedHandle)))
+                  exprWithCleanups(ignoringElidableConstructorCall(has(
+                                       ignoringParenImpCasts(ConvertedHandle))))
                       .bind("bad_stmt"))),
       this);
 
-  // Find 'Handle foo = ReturnsAValue();'
-  Finder->addMatcher(
-      traverse(TK_AsIs,
-               varDecl(hasType(hasUnqualifiedDesugaredType(recordType(
-                           hasDeclaration(cxxRecordDecl(IsAHandle))))),
-                       unless(parmVarDecl()),
-                       hasInitializer(exprWithCleanups(
-                                          has(ignoringParenImpCasts(handleFrom(
-                                              IsAHandle, ConvertedHandle))))
-                                          .bind("bad_stmt")))),
-      this);
   // Find 'foo = ReturnsAValue();  // foo is Handle'
   Finder->addMatcher(
       traverse(TK_AsIs,
@@ -141,36 +132,35 @@
 void DanglingHandleCheck::registerMatchersForReturn(MatchFinder *Finder) {
   // Return a local.
   Finder->addMatcher(
-      traverse(
-          TK_AsIs,
-          returnStmt(
-              // The AST contains two constructor calls:
-              //   1. Value to Handle conversion.
-              //   2. Handle copy construction.
-              // We have to match both.
-              has(ignoringImplicit(handleFrom(
-                  IsAHandle,
-                  handleFrom(IsAHandle,
-                             declRefExpr(to(varDecl(
-                                 // Is function scope ...
-                                 hasAutomaticStorageDuration(),
-                                 // ... and it is a local array or Value.
-                                 anyOf(hasType(arrayType()),
-                                       hasType(hasUnqualifiedDesugaredType(
-                                           recordType(hasDeclaration(recordDecl(
-                                               unless(IsAHandle)))))))))))))),
-              // Temporary fix for false positives inside lambdas.
-              unless(hasAncestor(lambdaExpr())))
-              .bind("bad_stmt")),
+      traverse(TK_AsIs,
+               returnStmt(
+                   // The AST contains two constructor calls:
+                   //   1. Value to Handle conversion.
+                   //   2. Handle copy construction (elided in C++17+).
+                   // We have to match both.
+                   has(ignoringImplicit(ignoringElidableConstructorCall(
+                       ignoringImplicit(handleFrom(
+                           IsAHandle,
+                           declRefExpr(to(varDecl(
+                               // Is function scope ...
+                               hasAutomaticStorageDuration(),
+                               // ... and it is a local array or Value.
+                               anyOf(hasType(arrayType()),
+                                     hasType(hasUnqualifiedDesugaredType(
+                                         recordType(hasDeclaration(recordDecl(
+                                             unless(IsAHandle))))))))))))))),
+                   // Temporary fix for false positives inside lambdas.
+                   unless(hasAncestor(lambdaExpr())))
+                   .bind("bad_stmt")),
       this);
 
   // Return a temporary.
   Finder->addMatcher(
-      traverse(
-          TK_AsIs,
-          returnStmt(has(exprWithCleanups(has(ignoringParenImpCasts(handleFrom(
-                         IsAHandle, handleFromTemporaryValue(IsAHandle)))))))
-              .bind("bad_stmt")),
+      traverse(TK_AsIs,
+               returnStmt(has(exprWithCleanups(ignoringElidableConstructorCall(
+                              has(ignoringParenImpCasts(
+                                  handleFromTemporaryValue(IsAHandle)))))))
+                   .bind("bad_stmt")),
       this);
 }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D158371: [clang-tid... Ignat Loskutov via Phabricator via cfe-commits

Reply via email to