diff --git a/clang-tools-extra/clang-tidy/bugprone/DanglingHandleCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/DanglingHandleCheck.cpp --- a/clang-tools-extra/clang-tidy/bugprone/DanglingHandleCheck.cpp +++ b/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); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp +++ b/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";