Index: clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -12,6 +12,7 @@ #include "../ClangTidyModuleRegistry.h" #include "ArgumentCommentCheck.h" #include "CopyConstructorInitCheck.h" +#include "DanglingHandleCheck.h" #include "IntegerDivisionCheck.h" #include "MisplacedOperatorInStrlenInAllocCheck.h" #include "StringConstructorCheck.h" @@ -29,6 +30,8 @@ "bugprone-argument-comment"); CheckFactories.registerCheck( "bugprone-copy-constructor-init"); + CheckFactories.registerCheck( + "bugprone-dangling-handle"); CheckFactories.registerCheck( "bugprone-integer-division"); CheckFactories.registerCheck( Index: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt @@ -4,6 +4,7 @@ ArgumentCommentCheck.cpp BugproneTidyModule.cpp CopyConstructorInitCheck.cpp + DanglingHandleCheck.cpp IntegerDivisionCheck.cpp MisplacedOperatorInStrlenInAllocCheck.cpp StringConstructorCheck.cpp Index: clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.h +++ clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.h @@ -0,0 +1,43 @@ +//===--- DanglingHandleCheck.h - clang-tidy----------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_DANGLING_HANDLE_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_DANGLING_HANDLE_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Detect dangling references in value handlers like +/// std::experimental::string_view. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-dangling-handle.html +class DanglingHandleCheck : public ClangTidyCheck { +public: + DanglingHandleCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + void registerMatchersForVariables(ast_matchers::MatchFinder *Finder); + void registerMatchersForReturn(ast_matchers::MatchFinder *Finder); + + const std::vector HandleClasses; + const ast_matchers::internal::Matcher IsAHandle; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_DANGLING_HANDLE_H Index: clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.cpp @@ -0,0 +1,184 @@ +//===--- DanglingHandleCheck.cpp - clang-tidy------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "DanglingHandleCheck.h" +#include "../utils/Matchers.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; +using namespace clang::tidy::matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +namespace { + +ast_matchers::internal::BindableMatcher +handleFrom(const ast_matchers::internal::Matcher &IsAHandle, + const ast_matchers::internal::Matcher &Arg) { + return cxxConstructExpr(hasDeclaration(cxxMethodDecl(ofClass(IsAHandle))), + hasArgument(0, Arg)); +} + +ast_matchers::internal::Matcher handleFromTemporaryValue( + const ast_matchers::internal::Matcher &IsAHandle) { + // 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())); + + return handleFrom(IsAHandle, anyOf(cxxBindTemporaryExpr(), TemporaryTernary)); +} + +ast_matchers::internal::Matcher isASequence() { + return hasAnyName("::std::deque", "::std::forward_list", "::std::list", + "::std::vector"); +} + +ast_matchers::internal::Matcher isASet() { + return hasAnyName("::std::set", "::std::multiset", "::std::unordered_set", + "::std::unordered_multiset"); +} + +ast_matchers::internal::Matcher isAMap() { + return hasAnyName("::std::map", "::std::multimap", "::std::unordered_map", + "::std::unordered_multimap"); +} + +ast_matchers::internal::BindableMatcher makeContainerMatcher( + const ast_matchers::internal::Matcher &IsAHandle) { + // This matcher could be expanded to detect: + // - Constructors: eg. vector(3, string("A")); + // - emplace*(): This requires a different logic to determine that + // the conversion will happen inside the container. + // - map's insert: This requires detecting that the pair conversion triggers + // the bug. A little more complicated than what we have now. + return callExpr( + hasAnyArgument( + ignoringParenImpCasts(handleFromTemporaryValue(IsAHandle))), + anyOf( + // For sequences: assign, push_back, resize. + cxxMemberCallExpr( + callee(functionDecl(hasAnyName("assign", "push_back", "resize"))), + on(expr(hasType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(recordDecl(isASequence())))))))), + // For sequences and sets: insert. + cxxMemberCallExpr(callee(functionDecl(hasName("insert"))), + on(expr(hasType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(recordDecl( + anyOf(isASequence(), isASet()))))))))), + // For maps: operator[]. + cxxOperatorCallExpr(callee(cxxMethodDecl(ofClass(isAMap()))), + hasOverloadedOperatorName("[]")))); +} + +} // anonymous namespace + +DanglingHandleCheck::DanglingHandleCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + HandleClasses(utils::options::parseStringList(Options.get( + "HandleClasses", + "std::basic_string_view;std::experimental::basic_string_view"))), + IsAHandle(cxxRecordDecl(hasAnyName(std::vector( + HandleClasses.begin(), HandleClasses.end()))) + .bind("handle")) {} + +void DanglingHandleCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "HandleClasses", + utils::options::serializeStringList(HandleClasses)); +} + +void DanglingHandleCheck::registerMatchersForVariables(MatchFinder *Finder) { + const auto ConvertedHandle = handleFromTemporaryValue(IsAHandle); + + // Find 'Handle foo(ReturnsAValue());' + Finder->addMatcher( + varDecl(hasType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(cxxRecordDecl(IsAHandle))))), + hasInitializer( + exprWithCleanups(has(ignoringParenImpCasts(ConvertedHandle))) + .bind("bad_stmt"))), + this); + + // Find 'Handle foo = ReturnsAValue();' + Finder->addMatcher( + 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( + cxxOperatorCallExpr(callee(cxxMethodDecl(ofClass(IsAHandle))), + hasOverloadedOperatorName("="), + hasArgument(1, ConvertedHandle)) + .bind("bad_stmt"), + this); + + // Container insertions that will dangle. + Finder->addMatcher(makeContainerMatcher(IsAHandle).bind("bad_stmt"), this); +} + +void DanglingHandleCheck::registerMatchersForReturn(MatchFinder *Finder) { + // Return a local. + Finder->addMatcher( + 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"), + this); + + // Return a temporary. + Finder->addMatcher( + returnStmt( + has(ignoringParenImpCasts(exprWithCleanups(has(ignoringParenImpCasts( + handleFrom(IsAHandle, handleFromTemporaryValue(IsAHandle)))))))) + .bind("bad_stmt"), + this); +} + +void DanglingHandleCheck::registerMatchers(MatchFinder *Finder) { + registerMatchersForVariables(Finder); + registerMatchersForReturn(Finder); +} + +void DanglingHandleCheck::check(const MatchFinder::MatchResult &Result) { + auto *Handle = Result.Nodes.getNodeAs("handle"); + diag(Result.Nodes.getNodeAs("bad_stmt")->getLocStart(), + "%0 outlives its value") + << Handle->getQualifiedNameAsString(); +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang Index: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt @@ -7,7 +7,6 @@ MisplacedConstCheck.cpp UnconventionalAssignOperatorCheck.cpp BoolPointerImplicitConversionCheck.cpp - DanglingHandleCheck.cpp DefinitionsInHeadersCheck.cpp FoldInitTypeCheck.cpp ForwardDeclarationNamespaceCheck.cpp Index: clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.h +++ clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.h @@ -1,43 +0,0 @@ -//===--- DanglingHandleCheck.h - clang-tidy----------------------*- C++ -*-===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// - -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DANGLING_HANDLE_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DANGLING_HANDLE_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace misc { - -/// Detect dangling references in value handlers like -/// std::experimental::string_view. -/// -/// For the user-facing documentation see: -/// http://clang.llvm.org/extra/clang-tidy/checks/misc-dangling-handle.html -class DanglingHandleCheck : public ClangTidyCheck { -public: - DanglingHandleCheck(StringRef Name, ClangTidyContext *Context); - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; - void storeOptions(ClangTidyOptions::OptionMap &Opts) override; - -private: - void registerMatchersForVariables(ast_matchers::MatchFinder *Finder); - void registerMatchersForReturn(ast_matchers::MatchFinder *Finder); - - const std::vector HandleClasses; - const ast_matchers::internal::Matcher IsAHandle; -}; - -} // namespace misc -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DANGLING_HANDLE_H Index: clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp @@ -1,184 +0,0 @@ -//===--- DanglingHandleCheck.cpp - clang-tidy------------------------------===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// - -#include "DanglingHandleCheck.h" -#include "../utils/Matchers.h" -#include "../utils/OptionsUtils.h" -#include "clang/AST/ASTContext.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" - -using namespace clang::ast_matchers; -using namespace clang::tidy::matchers; - -namespace clang { -namespace tidy { -namespace misc { - -namespace { - -ast_matchers::internal::BindableMatcher -handleFrom(const ast_matchers::internal::Matcher &IsAHandle, - const ast_matchers::internal::Matcher &Arg) { - return cxxConstructExpr(hasDeclaration(cxxMethodDecl(ofClass(IsAHandle))), - hasArgument(0, Arg)); -} - -ast_matchers::internal::Matcher handleFromTemporaryValue( - const ast_matchers::internal::Matcher &IsAHandle) { - // 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())); - - return handleFrom(IsAHandle, anyOf(cxxBindTemporaryExpr(), TemporaryTernary)); -} - -ast_matchers::internal::Matcher isASequence() { - return hasAnyName("::std::deque", "::std::forward_list", "::std::list", - "::std::vector"); -} - -ast_matchers::internal::Matcher isASet() { - return hasAnyName("::std::set", "::std::multiset", "::std::unordered_set", - "::std::unordered_multiset"); -} - -ast_matchers::internal::Matcher isAMap() { - return hasAnyName("::std::map", "::std::multimap", "::std::unordered_map", - "::std::unordered_multimap"); -} - -ast_matchers::internal::BindableMatcher makeContainerMatcher( - const ast_matchers::internal::Matcher &IsAHandle) { - // This matcher could be expanded to detect: - // - Constructors: eg. vector(3, string("A")); - // - emplace*(): This requires a different logic to determine that - // the conversion will happen inside the container. - // - map's insert: This requires detecting that the pair conversion triggers - // the bug. A little more complicated than what we have now. - return callExpr( - hasAnyArgument( - ignoringParenImpCasts(handleFromTemporaryValue(IsAHandle))), - anyOf( - // For sequences: assign, push_back, resize. - cxxMemberCallExpr( - callee(functionDecl(hasAnyName("assign", "push_back", "resize"))), - on(expr(hasType(hasUnqualifiedDesugaredType( - recordType(hasDeclaration(recordDecl(isASequence())))))))), - // For sequences and sets: insert. - cxxMemberCallExpr(callee(functionDecl(hasName("insert"))), - on(expr(hasType(hasUnqualifiedDesugaredType( - recordType(hasDeclaration(recordDecl( - anyOf(isASequence(), isASet()))))))))), - // For maps: operator[]. - cxxOperatorCallExpr(callee(cxxMethodDecl(ofClass(isAMap()))), - hasOverloadedOperatorName("[]")))); -} - -} // anonymous namespace - -DanglingHandleCheck::DanglingHandleCheck(StringRef Name, - ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), - HandleClasses(utils::options::parseStringList(Options.get( - "HandleClasses", - "std::basic_string_view;std::experimental::basic_string_view"))), - IsAHandle(cxxRecordDecl(hasAnyName(std::vector( - HandleClasses.begin(), HandleClasses.end()))) - .bind("handle")) {} - -void DanglingHandleCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "HandleClasses", - utils::options::serializeStringList(HandleClasses)); -} - -void DanglingHandleCheck::registerMatchersForVariables(MatchFinder *Finder) { - const auto ConvertedHandle = handleFromTemporaryValue(IsAHandle); - - // Find 'Handle foo(ReturnsAValue());' - Finder->addMatcher( - varDecl(hasType(hasUnqualifiedDesugaredType( - recordType(hasDeclaration(cxxRecordDecl(IsAHandle))))), - hasInitializer( - exprWithCleanups(has(ignoringParenImpCasts(ConvertedHandle))) - .bind("bad_stmt"))), - this); - - // Find 'Handle foo = ReturnsAValue();' - Finder->addMatcher( - 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( - cxxOperatorCallExpr(callee(cxxMethodDecl(ofClass(IsAHandle))), - hasOverloadedOperatorName("="), - hasArgument(1, ConvertedHandle)) - .bind("bad_stmt"), - this); - - // Container insertions that will dangle. - Finder->addMatcher(makeContainerMatcher(IsAHandle).bind("bad_stmt"), this); -} - -void DanglingHandleCheck::registerMatchersForReturn(MatchFinder *Finder) { - // Return a local. - Finder->addMatcher( - 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"), - this); - - // Return a temporary. - Finder->addMatcher( - returnStmt( - has(ignoringParenImpCasts(exprWithCleanups(has(ignoringParenImpCasts( - handleFrom(IsAHandle, handleFromTemporaryValue(IsAHandle)))))))) - .bind("bad_stmt"), - this); -} - -void DanglingHandleCheck::registerMatchers(MatchFinder *Finder) { - registerMatchersForVariables(Finder); - registerMatchersForReturn(Finder); -} - -void DanglingHandleCheck::check(const MatchFinder::MatchResult &Result) { - auto *Handle = Result.Nodes.getNodeAs("handle"); - diag(Result.Nodes.getNodeAs("bad_stmt")->getLocStart(), - "%0 outlives its value") - << Handle->getQualifiedNameAsString(); -} - -} // namespace misc -} // namespace tidy -} // namespace clang Index: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp @@ -12,7 +12,6 @@ #include "../ClangTidyModuleRegistry.h" #include "AssertSideEffectCheck.h" #include "BoolPointerImplicitConversionCheck.h" -#include "DanglingHandleCheck.h" #include "DefinitionsInHeadersCheck.h" #include "FoldInitTypeCheck.h" #include "ForwardDeclarationNamespaceCheck.h" @@ -73,7 +72,6 @@ "misc-unconventional-assign-operator"); CheckFactories.registerCheck( "misc-bool-pointer-implicit-conversion"); - CheckFactories.registerCheck("misc-dangling-handle"); CheckFactories.registerCheck( "misc-definitions-in-headers"); CheckFactories.registerCheck("misc-fold-init-type"); Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/trunk/docs/ReleaseNotes.rst +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst @@ -57,6 +57,9 @@ Improvements to clang-tidy -------------------------- +- The 'misc-dangling-handle' check was renamed to `bugprone-dangling-handle + `_ + - The 'misc-argument-comment' check was renamed to `bugprone-argument-comment `_ Index: clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-dangling-handle.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-dangling-handle.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-dangling-handle.rst @@ -0,0 +1,38 @@ +.. title:: clang-tidy - bugprone-dangling-handle + +bugprone-dangling-handle +======================== + +Detect dangling references in value handles like +``std::experimental::string_view``. +These dangling references can be a result of constructing handles from temporary +values, where the temporary is destroyed soon after the handle is created. + +Examples: + +.. code-block:: c++ + + string_view View = string(); // View will dangle. + string A; + View = A + "A"; // still dangle. + + vector V; + V.push_back(string()); // V[0] is dangling. + V.resize(3, string()); // V[1] and V[2] will also dangle. + + string_view f() { + // All these return values will dangle. + return string(); + string S; + return S; + char Array[10]{}; + return Array; + } + +Options +------- + +.. option:: HandleClasses + + A semicolon-separated list of class names that should be treated as handles. + By default only ``std::experimental::basic_string_view`` is considered. Index: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst @@ -19,6 +19,7 @@ boost-use-to-string bugprone-argument-comment bugprone-copy-constructor-init + bugprone-dangling-handle bugprone-integer-division bugprone-misplaced-operator-in-strlen-in-alloc bugprone-string-constructor @@ -108,7 +109,6 @@ llvm-twine-local misc-assert-side-effect misc-bool-pointer-implicit-conversion - misc-dangling-handle misc-definitions-in-headers misc-fold-init-type misc-forward-declaration-namespace Index: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-dangling-handle.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-dangling-handle.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-dangling-handle.rst @@ -1,38 +0,0 @@ -.. title:: clang-tidy - misc-dangling-handle - -misc-dangling-handle -==================== - -Detect dangling references in value handles like -``std::experimental::string_view``. -These dangling references can be a result of constructing handles from temporary -values, where the temporary is destroyed soon after the handle is created. - -Examples: - -.. code-block:: c++ - - string_view View = string(); // View will dangle. - string A; - View = A + "A"; // still dangle. - - vector V; - V.push_back(string()); // V[0] is dangling. - V.resize(3, string()); // V[1] and V[2] will also dangle. - - string_view f() { - // All these return values will dangle. - return string(); - string S; - return S; - char Array[10]{}; - return Array; - } - -Options -------- - -.. option:: HandleClasses - - A semicolon-separated list of class names that should be treated as handles. - By default only ``std::experimental::basic_string_view`` is considered. Index: clang-tools-extra/trunk/test/clang-tidy/bugprone-dangling-handle.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/bugprone-dangling-handle.cpp +++ clang-tools-extra/trunk/test/clang-tidy/bugprone-dangling-handle.cpp @@ -0,0 +1,191 @@ +// RUN: %check_clang_tidy %s bugprone-dangling-handle %t -- \ +// RUN: -config="{CheckOptions: \ +// RUN: [{key: bugprone-dangling-handle.HandleClasses, \ +// RUN: value: 'std::basic_string_view; ::llvm::StringRef;'}]}" \ +// RUN: -- -std=c++11 + +namespace std { + +template +class vector { + public: + using const_iterator = const T*; + using iterator = T*; + using size_type = int; + + void assign(size_type count, const T& value); + iterator insert(const_iterator pos, const T& value); + iterator insert(const_iterator pos, T&& value); + iterator insert(const_iterator pos, size_type count, const T& value); + void push_back(const T&); + void push_back(T&&); + void resize(size_type count, const T& value); +}; + +template +class pair {}; + +template +class set { + public: + using const_iterator = const T*; + using iterator = T*; + + std::pair insert(const T& value); + std::pair insert(T&& value); + iterator insert(const_iterator hint, const T& value); + iterator insert(const_iterator hint, T&& value); +}; + +template +class map { + public: + using value_type = pair; + value_type& operator[](const Key& key); + value_type& operator[](Key&& key); +}; + +class basic_string { + public: + basic_string(); + basic_string(const char*); + ~basic_string(); +}; + +typedef basic_string string; + +class basic_string_view { + public: + basic_string_view(const char*); + basic_string_view(const basic_string&); +}; + +typedef basic_string_view string_view; + +} // namespace std + +namespace llvm { + +class StringRef { + public: + StringRef(); + StringRef(const char*); + StringRef(const std::string&); +}; + +} // namespace llvm + +std::string ReturnsAString(); + +void Positives() { + std::string_view view1 = std::string(); + // CHECK-MESSAGES: [[@LINE-1]]:20: 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 + + 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 + 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 +} + +void OtherTypes() { + llvm::StringRef ref = std::string(); + // CHECK-MESSAGES: [[@LINE-1]]:19: warning: llvm::StringRef outlives its value +} + +const char static_array[] = "A"; +std::string_view ReturnStatements(int i, std::string value_arg, + const std::string &ref_arg) { + const char array[] = "A"; + const char* ptr = "A"; + std::string s; + static std::string ss; + switch (i) { + // Bad cases + case 0: + return array; // refers to local + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: std::basic_string_view outliv + case 1: + return s; // refers to local + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: std::basic_string_view outliv + case 2: + return std::string(); // refers to temporary + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: std::basic_string_view outliv + case 3: + return value_arg; // refers to by-value arg + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: std::basic_string_view outliv + + // Ok cases + case 100: + return ss; // refers to static + case 101: + return static_array; // refers to static + case 102: + return ptr; // pointer is ok + case 103: + return ref_arg; // refers to by-ref arg + } + + struct S { + std::string_view view() { return value; } + std::string value; + }; + + (void)[&]()->std::string_view { + // This should not warn. The string is bound by reference. + return s; + }; + (void)[=]() -> std::string_view { + // This should not warn. The reference is valid as long as the lambda. + return s; + }; + (void)[=]() -> std::string_view { + // FIXME: This one should warn. We are returning a reference to a local + // lambda variable. + std::string local; + return local; + }; + return ""; +} + +void Containers() { + std::vector v; + v.assign(3, std::string()); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives + v.insert(nullptr, std::string()); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives + v.insert(nullptr, 3, std::string()); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives + v.push_back(std::string()); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives + v.resize(3, std::string()); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives + + std::set s; + s.insert(std::string()); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives + s.insert(nullptr, std::string()); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives + + std::map m; + m[std::string()]; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives +} + +void TakesAStringView(std::string_view); + +void Negatives(std::string_view default_arg = ReturnsAString()) { + std::string str; + std::string_view view = str; + + TakesAStringView(std::string()); +} Index: clang-tools-extra/trunk/test/clang-tidy/misc-dangling-handle.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/misc-dangling-handle.cpp +++ clang-tools-extra/trunk/test/clang-tidy/misc-dangling-handle.cpp @@ -1,191 +0,0 @@ -// RUN: %check_clang_tidy %s misc-dangling-handle %t -- \ -// RUN: -config="{CheckOptions: \ -// RUN: [{key: misc-dangling-handle.HandleClasses, \ -// RUN: value: 'std::basic_string_view; ::llvm::StringRef;'}]}" \ -// RUN: -- -std=c++11 - -namespace std { - -template -class vector { - public: - using const_iterator = const T*; - using iterator = T*; - using size_type = int; - - void assign(size_type count, const T& value); - iterator insert(const_iterator pos, const T& value); - iterator insert(const_iterator pos, T&& value); - iterator insert(const_iterator pos, size_type count, const T& value); - void push_back(const T&); - void push_back(T&&); - void resize(size_type count, const T& value); -}; - -template -class pair {}; - -template -class set { - public: - using const_iterator = const T*; - using iterator = T*; - - std::pair insert(const T& value); - std::pair insert(T&& value); - iterator insert(const_iterator hint, const T& value); - iterator insert(const_iterator hint, T&& value); -}; - -template -class map { - public: - using value_type = pair; - value_type& operator[](const Key& key); - value_type& operator[](Key&& key); -}; - -class basic_string { - public: - basic_string(); - basic_string(const char*); - ~basic_string(); -}; - -typedef basic_string string; - -class basic_string_view { - public: - basic_string_view(const char*); - basic_string_view(const basic_string&); -}; - -typedef basic_string_view string_view; - -} // namespace std - -namespace llvm { - -class StringRef { - public: - StringRef(); - StringRef(const char*); - StringRef(const std::string&); -}; - -} // namespace llvm - -std::string ReturnsAString(); - -void Positives() { - std::string_view view1 = std::string(); - // CHECK-MESSAGES: [[@LINE-1]]:20: warning: std::basic_string_view outlives its value [misc-dangling-handle] - - std::string_view view_2 = ReturnsAString(); - // CHECK-MESSAGES: [[@LINE-1]]:20: warning: std::basic_string_view outlives - - 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 - 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 -} - -void OtherTypes() { - llvm::StringRef ref = std::string(); - // CHECK-MESSAGES: [[@LINE-1]]:19: warning: llvm::StringRef outlives its value -} - -const char static_array[] = "A"; -std::string_view ReturnStatements(int i, std::string value_arg, - const std::string &ref_arg) { - const char array[] = "A"; - const char* ptr = "A"; - std::string s; - static std::string ss; - switch (i) { - // Bad cases - case 0: - return array; // refers to local - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: std::basic_string_view outliv - case 1: - return s; // refers to local - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: std::basic_string_view outliv - case 2: - return std::string(); // refers to temporary - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: std::basic_string_view outliv - case 3: - return value_arg; // refers to by-value arg - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: std::basic_string_view outliv - - // Ok cases - case 100: - return ss; // refers to static - case 101: - return static_array; // refers to static - case 102: - return ptr; // pointer is ok - case 103: - return ref_arg; // refers to by-ref arg - } - - struct S { - std::string_view view() { return value; } - std::string value; - }; - - (void)[&]()->std::string_view { - // This should not warn. The string is bound by reference. - return s; - }; - (void)[=]() -> std::string_view { - // This should not warn. The reference is valid as long as the lambda. - return s; - }; - (void)[=]() -> std::string_view { - // FIXME: This one should warn. We are returning a reference to a local - // lambda variable. - std::string local; - return local; - }; - return ""; -} - -void Containers() { - std::vector v; - v.assign(3, std::string()); - // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives - v.insert(nullptr, std::string()); - // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives - v.insert(nullptr, 3, std::string()); - // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives - v.push_back(std::string()); - // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives - v.resize(3, std::string()); - // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives - - std::set s; - s.insert(std::string()); - // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives - s.insert(nullptr, std::string()); - // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives - - std::map m; - m[std::string()]; - // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives -} - -void TakesAStringView(std::string_view); - -void Negatives(std::string_view default_arg = ReturnsAString()) { - std::string str; - std::string_view view = str; - - TakesAStringView(std::string()); -}