Index: clang-tidy/misc/UseAfterMoveCheck.cpp =================================================================== --- clang-tidy/misc/UseAfterMoveCheck.cpp +++ clang-tidy/misc/UseAfterMoveCheck.cpp @@ -384,6 +384,13 @@ // the direct ancestor of the std::move() that isn't one of the node // types ignored by ignoringParenImpCasts(). stmt(forEach(expr(ignoringParenImpCasts(CallMoveMatcher))), + // Don't allow an InitListExpr to be the moving call. An InitListExpr + // has both a syntactic and a semantic form, and the parent-child + // relationships are different between the two. This could cause an + // InitListExpr to be analyzed as the moving call in addition to the + // Expr that we actually want, resulting in two diagnostics with + // different code locations for the same move. + unless(initListExpr()), unless(expr(ignoringParenImpCasts(equalsBoundNode("call-move"))))) .bind("moving-call"), this); @@ -398,7 +405,7 @@ const auto *MovingCall = Result.Nodes.getNodeAs("moving-call"); const auto *Arg = Result.Nodes.getNodeAs("arg"); - if (!MovingCall) + if (!MovingCall || !MovingCall->getExprLoc().isValid()) MovingCall = CallMove; Stmt *FunctionBody = nullptr; Index: clang-tidy/safety/CMakeLists.txt =================================================================== --- clang-tidy/safety/CMakeLists.txt +++ clang-tidy/safety/CMakeLists.txt @@ -2,6 +2,7 @@ add_clang_library(clangTidySafetyModule NoAssemblerCheck.cpp + NoThrowInDestructorCheck.cpp SafetyTidyModule.cpp LINK_LIBS Index: clang-tidy/safety/NoThrowInDestructorCheck.h =================================================================== --- /dev/null +++ clang-tidy/safety/NoThrowInDestructorCheck.h @@ -0,0 +1,35 @@ +//===--- no-throw-in-destructorCheck.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_SAFETY_NO_THROW_IN_DESTRUCTOR_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_SAFETY_NO_THROW_IN_DESTRUCTOR_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace safety { + +/// This checks warns for throw statements in destructors. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/safety-NoThrowDestructor.html +class NoThrowInDestructorCheck : public ClangTidyCheck { +public: + NoThrowInDestructorCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace safety +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_SAFETY_NO_THROW_IN_DESTRUCTOR_H Index: clang-tidy/safety/NoThrowInDestructorCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/safety/NoThrowInDestructorCheck.cpp @@ -0,0 +1,34 @@ +//===--- no-throw-in-destructorCheck.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 "NoThrowInDestructorCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace safety { + +void NoThrowInDestructorCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + cxxThrowExpr(hasAncestor(cxxDestructorDecl())) + .bind("throw"), + this); +} + +void NoThrowInDestructorCheck::check(const MatchFinder::MatchResult &Result) { + const auto *MatchedThrow = Result.Nodes.getNodeAs("throw"); + diag(MatchedThrow->getLocStart(), "throw expression in destructor detected"); +} + +} // namespace safety +} // namespace tidy +} // namespace clang Index: clang-tidy/safety/SafetyTidyModule.cpp =================================================================== --- clang-tidy/safety/SafetyTidyModule.cpp +++ clang-tidy/safety/SafetyTidyModule.cpp @@ -11,6 +11,7 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "NoAssemblerCheck.h" +#include "NoThrowInDestructorCheck.h" namespace clang { namespace tidy { @@ -21,6 +22,8 @@ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck( "safety-no-assembler"); + CheckFactories.registerCheck( + "safety-no-throw-in-destructor"); } }; Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -155,4 +155,5 @@ readability-simplify-boolean-expr readability-static-definition-in-anonymous-namespace readability-uniqueptr-delete-release + safety-NoThrowDestructor safety-no-assembler Index: docs/clang-tidy/checks/safety-NoThrowDestructor.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/safety-NoThrowDestructor.rst @@ -0,0 +1,6 @@ +.. title:: clang-tidy - safety-NoThrowDestructor + +safety-NoThrowDestructor +======================== + +FIXME: Describe what patterns does the check detect and why. Give examples. Index: test/clang-tidy/misc-use-after-move.cpp =================================================================== --- test/clang-tidy/misc-use-after-move.cpp +++ test/clang-tidy/misc-use-after-move.cpp @@ -282,7 +282,7 @@ S s{std::move(a)}; a.foo(); // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved - // CHECK-MESSAGES: [[@LINE-3]]:6: note: move occurred here + // CHECK-MESSAGES: [[@LINE-3]]:7: note: move occurred here } void lambdas() { @@ -397,6 +397,21 @@ } template void movedTypeIsDependentType(); +// We handle the case correctly where the move consists of an implicit call +// to a conversion operator. +void implicitConversionOperator() { + struct Convertible { + operator A() && { return A(); } + }; + void takeA(A a); + + Convertible convertible; + takeA(std::move(convertible)); + convertible; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'convertible' used after it was moved + // CHECK-MESSAGES: [[@LINE-3]]:9: note: move occurred here +} + // Using decltype on an expression is not a use. void decltypeIsNotUse() { A a; Index: test/clang-tidy/safety-no-throw-in-destructor.cpp =================================================================== --- /dev/null +++ test/clang-tidy/safety-no-throw-in-destructor.cpp @@ -0,0 +1,56 @@ +// RUN: %check_clang_tidy %s safety-no-throw-in-destructor %t + +namespace std { +struct runtime_error {}; +} + +struct GoodInlineDestructor { + ~GoodInlineDestructor() { + int i = 1; // Ok + } +}; + +struct GoodDestructor { + ~GoodDestructor(); +}; + +GoodDestructor::~GoodDestructor() { + int i = 1; + float j = 0.5f; + // Ok +} + +struct BadDestructor { + ~BadDestructor(); +}; + +BadDestructor::~BadDestructor() { + throw std::runtime_error(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: throw expression in destructor detected +} + +struct BadInlineDestructor { + ~BadInlineDestructor() { throw std::runtime_error(); } + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: throw expression in destructor detected +}; + +struct ComplicatedInline { + ~ComplicatedInline() { + int i = 0; + float j = 0.5f; + char string[] = "Hallo welt"; + // Ok + } +}; + +struct ComplicatedBad { + ~ComplicatedBad() { + int i = 15; + if (i == 15) + throw std::runtime_error(); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throw expression in destructor detected + else + throw std::runtime_error(); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throw expression in destructor detected + } +};