Index: clang-tidy/misc/BoolPointerImplicitConversion.h =================================================================== --- clang-tidy/misc/BoolPointerImplicitConversion.h +++ /dev/null @@ -1,38 +0,0 @@ -//===--- BoolPointerImplicitConversion.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_BOOL_POINTER_IMPLICIT_CONV_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BOOL_POINTER_IMPLICIT_CONV_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace misc { - -/// \brief Checks for conditions based on implicit conversion from a bool -/// pointer to bool e.g. -/// bool *p; -/// if (p) { -/// // Never used in a pointer-specific way. -/// } -class BoolPointerImplicitConversion : public ClangTidyCheck { -public: - BoolPointerImplicitConversion(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; -}; - -} // namespace misc -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BOOL_POINTER_IMPLICIT_CONV_H - Index: clang-tidy/misc/BoolPointerImplicitConversion.cpp =================================================================== --- clang-tidy/misc/BoolPointerImplicitConversion.cpp +++ /dev/null @@ -1,77 +0,0 @@ -//===--- BoolPointerImplicitConversion.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 "BoolPointerImplicitConversion.h" - -using namespace clang::ast_matchers; - -namespace clang { -namespace ast_matchers { - -AST_MATCHER(CastExpr, isPointerToBoolean) { - return Node.getCastKind() == CK_PointerToBoolean; -} -AST_MATCHER(QualType, isBoolean) { return Node->isBooleanType(); } - -} // namespace ast_matchers - -namespace tidy { -namespace misc { - -void BoolPointerImplicitConversion::registerMatchers(MatchFinder *Finder) { - // Look for ifs that have an implicit bool* to bool conversion in the - // condition. Filter negations. - Finder->addMatcher( - ifStmt(hasCondition(findAll(implicitCastExpr( - allOf(unless(hasParent(unaryOperator(hasOperatorName("!")))), - hasSourceExpression(expr( - hasType(pointerType(pointee(isBoolean()))), - ignoringParenImpCasts(declRefExpr().bind("expr")))), - isPointerToBoolean())))), - unless(isInTemplateInstantiation())).bind("if"), - this); -} - -void -BoolPointerImplicitConversion::check(const MatchFinder::MatchResult &Result) { - auto *If = Result.Nodes.getStmtAs("if"); - auto *Var = Result.Nodes.getStmtAs("expr"); - - // Ignore macros. - if (Var->getLocStart().isMacroID()) - return; - - // Only allow variable accesses for now, no function calls or member exprs. - // Check that we don't dereference the variable anywhere within the if. This - // avoids false positives for checks of the pointer for nullptr before it is - // dereferenced. If there is a dereferencing operator on this variable don't - // emit a diagnostic. Also ignore array subscripts. - const Decl *D = Var->getDecl(); - auto DeclRef = ignoringParenImpCasts(declRefExpr(to(equalsNode(D)))); - if (!match(findAll( - unaryOperator(hasOperatorName("*"), hasUnaryOperand(DeclRef))), - *If, *Result.Context).empty() || - !match(findAll(arraySubscriptExpr(hasBase(DeclRef))), *If, - *Result.Context).empty() || - // FIXME: We should still warn if the paremater is implicitly converted to - // bool. - !match(findAll(callExpr(hasAnyArgument(DeclRef))), *If, *Result.Context) - .empty() || - !match(findAll(deleteExpr(has(expr(DeclRef)))), *If, *Result.Context) - .empty()) - return; - - diag(Var->getLocStart(), "dubious check of 'bool *' against 'nullptr', did " - "you mean to dereference it?") - << FixItHint::CreateInsertion(Var->getLocStart(), "*"); -} - -} // namespace misc -} // namespace tidy -} // namespace clang Index: clang-tidy/misc/BoolPointerImplicitConversionCheck.h =================================================================== --- /dev/null +++ clang-tidy/misc/BoolPointerImplicitConversionCheck.h @@ -0,0 +1,38 @@ +//===--- BoolPointerImplicitConversionCheck.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_BOOL_POINTER_IMPLICIT_CONVERSION_CHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BOOL_POINTER_IMPLICIT_CONVERSION_CHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace misc { + +/// \brief Checks for conditions based on implicit conversion from a bool +/// pointer to bool e.g. +/// bool *p; +/// if (p) { +/// // Never used in a pointer-specific way. +/// } +class BoolPointerImplicitConversionCheck : public ClangTidyCheck { +public: + BoolPointerImplicitConversionCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace misc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BOOL_POINTER_IMPLICIT_CONVERSION_CHECK_H + Index: clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp @@ -0,0 +1,77 @@ +//===--- BoolPointerImplicitConversionCheck.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 "BoolPointerImplicitConversionCheck.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace ast_matchers { + +AST_MATCHER(CastExpr, isPointerToBoolean) { + return Node.getCastKind() == CK_PointerToBoolean; +} +AST_MATCHER(QualType, isBoolean) { return Node->isBooleanType(); } + +} // namespace ast_matchers + +namespace tidy { +namespace misc { + +void BoolPointerImplicitConversionCheck::registerMatchers(MatchFinder *Finder) { + // Look for ifs that have an implicit bool* to bool conversion in the + // condition. Filter negations. + Finder->addMatcher( + ifStmt(hasCondition(findAll(implicitCastExpr( + allOf(unless(hasParent(unaryOperator(hasOperatorName("!")))), + hasSourceExpression(expr( + hasType(pointerType(pointee(isBoolean()))), + ignoringParenImpCasts(declRefExpr().bind("expr")))), + isPointerToBoolean())))), + unless(isInTemplateInstantiation())).bind("if"), + this); +} + +void +BoolPointerImplicitConversionCheck::check(const MatchFinder::MatchResult &Result) { + auto *If = Result.Nodes.getStmtAs("if"); + auto *Var = Result.Nodes.getStmtAs("expr"); + + // Ignore macros. + if (Var->getLocStart().isMacroID()) + return; + + // Only allow variable accesses for now, no function calls or member exprs. + // Check that we don't dereference the variable anywhere within the if. This + // avoids false positives for checks of the pointer for nullptr before it is + // dereferenced. If there is a dereferencing operator on this variable don't + // emit a diagnostic. Also ignore array subscripts. + const Decl *D = Var->getDecl(); + auto DeclRef = ignoringParenImpCasts(declRefExpr(to(equalsNode(D)))); + if (!match(findAll( + unaryOperator(hasOperatorName("*"), hasUnaryOperand(DeclRef))), + *If, *Result.Context).empty() || + !match(findAll(arraySubscriptExpr(hasBase(DeclRef))), *If, + *Result.Context).empty() || + // FIXME: We should still warn if the paremater is implicitly converted to + // bool. + !match(findAll(callExpr(hasAnyArgument(DeclRef))), *If, *Result.Context) + .empty() || + !match(findAll(deleteExpr(has(expr(DeclRef)))), *If, *Result.Context) + .empty()) + return; + + diag(Var->getLocStart(), "dubious check of 'bool *' against 'nullptr', did " + "you mean to dereference it?") + << FixItHint::CreateInsertion(Var->getLocStart(), "*"); +} + +} // namespace misc +} // namespace tidy +} // namespace clang Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -4,16 +4,16 @@ ArgumentCommentCheck.cpp AssertSideEffectCheck.cpp AssignOperatorSignatureCheck.cpp - BoolPointerImplicitConversion.cpp + BoolPointerImplicitConversionCheck.cpp InaccurateEraseCheck.cpp InefficientAlgorithmCheck.cpp MiscTidyModule.cpp StaticAssertCheck.cpp SwappedArgumentsCheck.cpp UndelegatedConstructor.cpp - UnusedRAII.cpp - UniqueptrResetRelease.cpp - UseOverride.cpp + UnusedRAIICheck.cpp + UniqueptrResetReleaseCheck.cpp + UseOverrideCheck.cpp LINK_LIBS clangAST Index: clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tidy/misc/MiscTidyModule.cpp +++ clang-tidy/misc/MiscTidyModule.cpp @@ -13,15 +13,15 @@ #include "ArgumentCommentCheck.h" #include "AssertSideEffectCheck.h" #include "AssignOperatorSignatureCheck.h" -#include "BoolPointerImplicitConversion.h" +#include "BoolPointerImplicitConversionCheck.h" #include "InaccurateEraseCheck.h" #include "InefficientAlgorithmCheck.h" #include "StaticAssertCheck.h" #include "SwappedArgumentsCheck.h" #include "UndelegatedConstructor.h" -#include "UniqueptrResetRelease.h" -#include "UnusedRAII.h" -#include "UseOverride.h" +#include "UniqueptrResetReleaseCheck.h" +#include "UnusedRAIICheck.h" +#include "UseOverrideCheck.h" namespace clang { namespace tidy { @@ -35,7 +35,7 @@ "misc-assert-side-effect"); CheckFactories.registerCheck( "misc-assign-operator-signature"); - CheckFactories.registerCheck( + CheckFactories.registerCheck( "misc-bool-pointer-implicit-conversion"); CheckFactories.registerCheck( "misc-inaccurate-erase"); @@ -47,10 +47,10 @@ "misc-swapped-arguments"); CheckFactories.registerCheck( "misc-undelegated-constructor"); - CheckFactories.registerCheck( + CheckFactories.registerCheck( "misc-uniqueptr-reset-release"); CheckFactories.registerCheck("misc-unused-raii"); - CheckFactories.registerCheck("misc-use-override"); + CheckFactories.registerCheck("misc-use-override"); } }; Index: clang-tidy/misc/UniqueptrResetRelease.h =================================================================== --- clang-tidy/misc/UniqueptrResetRelease.h +++ /dev/null @@ -1,40 +0,0 @@ -//===--- UniqueptrResetRelease.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_UNIQUEPTR_RESET_RELEASE_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNIQUEPTR_RESET_RELEASE_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace misc { - -/// \brief Find and replace unique_ptr::reset(release()) with std::move -/// -/// Example: -/// std::unique_ptr x, y; -/// x.reset(y.release()); -> x = std::move(y); -/// -/// If "y" is already rvalue, std::move is not added. -/// "x" and "y" can also be std::unique_ptr*. -class UniqueptrResetRelease : public ClangTidyCheck { -public: - UniqueptrResetRelease(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} - - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; -}; - -} // namespace misc -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNIQUEPTR_RESET_RELEASE_H Index: clang-tidy/misc/UniqueptrResetRelease.cpp =================================================================== --- clang-tidy/misc/UniqueptrResetRelease.cpp +++ /dev/null @@ -1,73 +0,0 @@ -//===--- UniqueptrResetRelease.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 "UniqueptrResetRelease.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Lex/Lexer.h" - -using namespace clang::ast_matchers; - -namespace clang { -namespace tidy { -namespace misc { - -void UniqueptrResetRelease::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher( - memberCallExpr( - on(expr().bind("left")), callee(memberExpr().bind("reset_member")), - callee(methodDecl(hasName("reset"), - ofClass(hasName("::std::unique_ptr")))), - has(memberCallExpr( - on(expr().bind("right")), - callee(memberExpr().bind("release_member")), - callee(methodDecl(hasName("release"), - ofClass(hasName("::std::unique_ptr"))))))) - .bind("reset_call"), - this); -} - -void UniqueptrResetRelease::check(const MatchFinder::MatchResult &Result) { - const auto *ResetMember = Result.Nodes.getNodeAs("reset_member"); - const auto *ReleaseMember = - Result.Nodes.getNodeAs("release_member"); - const auto *Right = Result.Nodes.getNodeAs("right"); - const auto *Left = Result.Nodes.getNodeAs("left"); - const auto *ResetCall = - Result.Nodes.getNodeAs("reset_call"); - - std::string LeftText = clang::Lexer::getSourceText( - CharSourceRange::getTokenRange(Left->getSourceRange()), - *Result.SourceManager, Result.Context->getLangOpts()); - std::string RightText = clang::Lexer::getSourceText( - CharSourceRange::getTokenRange(Right->getSourceRange()), - *Result.SourceManager, Result.Context->getLangOpts()); - - if (ResetMember->isArrow()) - LeftText = "*" + LeftText; - if (ReleaseMember->isArrow()) - RightText = "*" + RightText; - std::string DiagText; - // Even if x was rvalue, *x is not rvalue anymore. - if (!Right->isRValue() || ReleaseMember->isArrow()) { - RightText = "std::move(" + RightText + ")"; - DiagText = "prefer ptr1 = std::move(ptr2) over ptr1.reset(ptr2.release())"; - } else { - DiagText = - "prefer ptr = ReturnUnique() over ptr.reset(ReturnUnique().release())"; - } - std::string NewText = LeftText + " = " + RightText; - - diag(ResetMember->getExprLoc(), DiagText) - << FixItHint::CreateReplacement( - CharSourceRange::getTokenRange(ResetCall->getSourceRange()), NewText); -} - -} // namespace misc -} // namespace tidy -} // namespace clang Index: clang-tidy/misc/UniqueptrResetReleaseCheck.h =================================================================== --- /dev/null +++ clang-tidy/misc/UniqueptrResetReleaseCheck.h @@ -0,0 +1,40 @@ +//===--- UniqueptrResetReleaseCheck.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_UNIQUEPTR_RESET_RELEASE_CHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNIQUEPTR_RESET_RELEASE_CHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace misc { + +/// \brief Find and replace unique_ptr::reset(release()) with std::move +/// +/// Example: +/// std::unique_ptr x, y; +/// x.reset(y.release()); -> x = std::move(y); +/// +/// If "y" is already rvalue, std::move is not added. +/// "x" and "y" can also be std::unique_ptr*. +class UniqueptrResetReleaseCheck : public ClangTidyCheck { +public: + UniqueptrResetReleaseCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace misc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNIQUEPTR_RESET_RELEASE_CHECK_H Index: clang-tidy/misc/UniqueptrResetReleaseCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/misc/UniqueptrResetReleaseCheck.cpp @@ -0,0 +1,73 @@ +//===--- UniqueptrResetReleaseCheck.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 "UniqueptrResetReleaseCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace misc { + +void UniqueptrResetReleaseCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + memberCallExpr( + on(expr().bind("left")), callee(memberExpr().bind("reset_member")), + callee(methodDecl(hasName("reset"), + ofClass(hasName("::std::unique_ptr")))), + has(memberCallExpr( + on(expr().bind("right")), + callee(memberExpr().bind("release_member")), + callee(methodDecl(hasName("release"), + ofClass(hasName("::std::unique_ptr"))))))) + .bind("reset_call"), + this); +} + +void UniqueptrResetReleaseCheck::check(const MatchFinder::MatchResult &Result) { + const auto *ResetMember = Result.Nodes.getNodeAs("reset_member"); + const auto *ReleaseMember = + Result.Nodes.getNodeAs("release_member"); + const auto *Right = Result.Nodes.getNodeAs("right"); + const auto *Left = Result.Nodes.getNodeAs("left"); + const auto *ResetCall = + Result.Nodes.getNodeAs("reset_call"); + + std::string LeftText = clang::Lexer::getSourceText( + CharSourceRange::getTokenRange(Left->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); + std::string RightText = clang::Lexer::getSourceText( + CharSourceRange::getTokenRange(Right->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); + + if (ResetMember->isArrow()) + LeftText = "*" + LeftText; + if (ReleaseMember->isArrow()) + RightText = "*" + RightText; + std::string DiagText; + // Even if x was rvalue, *x is not rvalue anymore. + if (!Right->isRValue() || ReleaseMember->isArrow()) { + RightText = "std::move(" + RightText + ")"; + DiagText = "prefer ptr1 = std::move(ptr2) over ptr1.reset(ptr2.release())"; + } else { + DiagText = + "prefer ptr = ReturnUnique() over ptr.reset(ReturnUnique().release())"; + } + std::string NewText = LeftText + " = " + RightText; + + diag(ResetMember->getExprLoc(), DiagText) + << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(ResetCall->getSourceRange()), NewText); +} + +} // namespace misc +} // namespace tidy +} // namespace clang Index: clang-tidy/misc/UnusedRAII.h =================================================================== --- clang-tidy/misc/UnusedRAII.h +++ /dev/null @@ -1,51 +0,0 @@ -//===--- UnusedRAII.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_UNUSED_RAII_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_RAII_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace misc { - -/// \brief Finds temporaries that look like RAII objects. -/// -/// The canonical example for this is a scoped lock. -/// \code -/// { -/// scoped_lock(&global_mutex); -/// critical_section(); -/// } -/// \endcode -/// The destructor of the scoped_lock is called before the critical_section is -/// entered, leaving it unprotected. -/// -/// We apply a number of heuristics to reduce the false positive count of this -/// check: -/// - Ignore code expanded from macros. Testing frameworks make heavy use of -/// this. -/// - Ignore types with no user-declared constructor. Those are very unlikely -/// to be RAII objects. -/// - Ignore objects at the end of a compound statement (doesn't change behavior). -/// - Ignore objects returned from a call. -class UnusedRAIICheck : public ClangTidyCheck { -public: - UnusedRAIICheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; -}; - -} // namespace misc -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_RAII_H Index: clang-tidy/misc/UnusedRAII.cpp =================================================================== --- clang-tidy/misc/UnusedRAII.cpp +++ /dev/null @@ -1,85 +0,0 @@ -//===--- UnusedRAII.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 "UnusedRAII.h" -#include "clang/AST/ASTContext.h" -#include "clang/Lex/Lexer.h" - -using namespace clang::ast_matchers; - -namespace clang { -namespace ast_matchers { -AST_MATCHER(CXXRecordDecl, hasUserDeclaredDestructor) { - // TODO: If the dtor is there but empty we don't want to warn either. - return Node.hasDefinition() && Node.hasUserDeclaredDestructor(); -} -} // namespace ast_matchers - -namespace tidy { -namespace misc { - -void UnusedRAIICheck::registerMatchers(MatchFinder *Finder) { - // Look for temporaries that are constructed in-place and immediately - // destroyed. Look for temporaries created by a functional cast but not for - // those returned from a call. - auto BindTemp = bindTemporaryExpr(unless(has(callExpr()))).bind("temp"); - Finder->addMatcher( - exprWithCleanups(unless(isInTemplateInstantiation()), - hasParent(compoundStmt().bind("compound")), - hasType(recordDecl(hasUserDeclaredDestructor())), - anyOf(has(BindTemp), has(functionalCastExpr( - has(BindTemp))))).bind("expr"), - this); -} - -void UnusedRAIICheck::check(const MatchFinder::MatchResult &Result) { - const auto *E = Result.Nodes.getStmtAs("expr"); - - // We ignore code expanded from macros to reduce the number of false - // positives. - if (E->getLocStart().isMacroID()) - return; - - // Don't emit a warning for the last statement in the surrounding compund - // statement. - const auto *CS = Result.Nodes.getStmtAs("compound"); - if (E == CS->body_back()) - return; - - // Emit a warning. - auto D = diag(E->getLocStart(), "object destroyed immediately after " - "creation; did you mean to name the object?"); - const char *Replacement = " give_me_a_name"; - - // If this is a default ctor we have to remove the parens or we'll introduce a - // most vexing parse. - const auto *BTE = Result.Nodes.getStmtAs("temp"); - if (const auto *TOE = dyn_cast(BTE->getSubExpr())) - if (TOE->getNumArgs() == 0) { - D << FixItHint::CreateReplacement( - CharSourceRange::getTokenRange(TOE->getParenOrBraceRange()), - Replacement); - return; - } - - // Otherwise just suggest adding a name. To find the place to insert the name - // find the first TypeLoc in the children of E, which always points to the - // written type. - auto Matches = - match(expr(hasDescendant(typeLoc().bind("t"))), *E, *Result.Context); - const auto *TL = selectFirst("t", Matches); - D << FixItHint::CreateInsertion( - Lexer::getLocForEndOfToken(TL->getLocEnd(), 0, *Result.SourceManager, - Result.Context->getLangOpts()), - Replacement); -} - -} // namespace misc -} // namespace tidy -} // namespace clang Index: clang-tidy/misc/UnusedRAIICheck.h =================================================================== --- /dev/null +++ clang-tidy/misc/UnusedRAIICheck.h @@ -0,0 +1,51 @@ +//===--- UnusedRAIICheck.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_UNUSED_RAII_CHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_RAII_CHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace misc { + +/// \brief Finds temporaries that look like RAII objects. +/// +/// The canonical example for this is a scoped lock. +/// \code +/// { +/// scoped_lock(&global_mutex); +/// critical_section(); +/// } +/// \endcode +/// The destructor of the scoped_lock is called before the critical_section is +/// entered, leaving it unprotected. +/// +/// We apply a number of heuristics to reduce the false positive count of this +/// check: +/// - Ignore code expanded from macros. Testing frameworks make heavy use of +/// this. +/// - Ignore types with no user-declared constructor. Those are very unlikely +/// to be RAII objects. +/// - Ignore objects at the end of a compound statement (doesn't change behavior). +/// - Ignore objects returned from a call. +class UnusedRAIICheck : public ClangTidyCheck { +public: + UnusedRAIICheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace misc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_RAII_CHECK_H Index: clang-tidy/misc/UnusedRAIICheck.cpp =================================================================== --- /dev/null +++ clang-tidy/misc/UnusedRAIICheck.cpp @@ -0,0 +1,85 @@ +//===--- UnusedRAIICheck.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 "UnusedRAIICheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace ast_matchers { +AST_MATCHER(CXXRecordDecl, hasUserDeclaredDestructor) { + // TODO: If the dtor is there but empty we don't want to warn either. + return Node.hasDefinition() && Node.hasUserDeclaredDestructor(); +} +} // namespace ast_matchers + +namespace tidy { +namespace misc { + +void UnusedRAIICheck::registerMatchers(MatchFinder *Finder) { + // Look for temporaries that are constructed in-place and immediately + // destroyed. Look for temporaries created by a functional cast but not for + // those returned from a call. + auto BindTemp = bindTemporaryExpr(unless(has(callExpr()))).bind("temp"); + Finder->addMatcher( + exprWithCleanups(unless(isInTemplateInstantiation()), + hasParent(compoundStmt().bind("compound")), + hasType(recordDecl(hasUserDeclaredDestructor())), + anyOf(has(BindTemp), has(functionalCastExpr( + has(BindTemp))))).bind("expr"), + this); +} + +void UnusedRAIICheck::check(const MatchFinder::MatchResult &Result) { + const auto *E = Result.Nodes.getStmtAs("expr"); + + // We ignore code expanded from macros to reduce the number of false + // positives. + if (E->getLocStart().isMacroID()) + return; + + // Don't emit a warning for the last statement in the surrounding compund + // statement. + const auto *CS = Result.Nodes.getStmtAs("compound"); + if (E == CS->body_back()) + return; + + // Emit a warning. + auto D = diag(E->getLocStart(), "object destroyed immediately after " + "creation; did you mean to name the object?"); + const char *Replacement = " give_me_a_name"; + + // If this is a default ctor we have to remove the parens or we'll introduce a + // most vexing parse. + const auto *BTE = Result.Nodes.getStmtAs("temp"); + if (const auto *TOE = dyn_cast(BTE->getSubExpr())) + if (TOE->getNumArgs() == 0) { + D << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(TOE->getParenOrBraceRange()), + Replacement); + return; + } + + // Otherwise just suggest adding a name. To find the place to insert the name + // find the first TypeLoc in the children of E, which always points to the + // written type. + auto Matches = + match(expr(hasDescendant(typeLoc().bind("t"))), *E, *Result.Context); + const auto *TL = selectFirst("t", Matches); + D << FixItHint::CreateInsertion( + Lexer::getLocForEndOfToken(TL->getLocEnd(), 0, *Result.SourceManager, + Result.Context->getLangOpts()), + Replacement); +} + +} // namespace misc +} // namespace tidy +} // namespace clang Index: clang-tidy/misc/UseOverride.h =================================================================== --- clang-tidy/misc/UseOverride.h +++ /dev/null @@ -1,32 +0,0 @@ -//===--- UseOverride.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_USE_OVERRIDE_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USE_OVERRIDE_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace misc { - -/// \brief Use C++11's 'override' and remove 'virtual' where applicable. -class UseOverride : public ClangTidyCheck { -public: - UseOverride(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; -}; - -} // namespace misc -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USE_OVERRIDE_H Index: clang-tidy/misc/UseOverride.cpp =================================================================== --- clang-tidy/misc/UseOverride.cpp +++ /dev/null @@ -1,181 +0,0 @@ -//===--- UseOverride.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 "UseOverride.h" -#include "clang/AST/ASTContext.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Lex/Lexer.h" - -using namespace clang::ast_matchers; - -namespace clang { -namespace tidy { -namespace misc { - -void UseOverride::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher(methodDecl(isOverride()).bind("method"), this); -} - -// Re-lex the tokens to get precise locations to insert 'override' and remove -// 'virtual'. -static SmallVector -ParseTokens(CharSourceRange Range, const MatchFinder::MatchResult &Result) { - const SourceManager &Sources = *Result.SourceManager; - std::pair LocInfo = - Sources.getDecomposedLoc(Range.getBegin()); - StringRef File = Sources.getBufferData(LocInfo.first); - const char *TokenBegin = File.data() + LocInfo.second; - Lexer RawLexer(Sources.getLocForStartOfFile(LocInfo.first), - Result.Context->getLangOpts(), File.begin(), TokenBegin, - File.end()); - SmallVector Tokens; - Token Tok; - while (!RawLexer.LexFromRawLexer(Tok)) { - if (Tok.is(tok::semi) || Tok.is(tok::l_brace)) - break; - if (Sources.isBeforeInTranslationUnit(Range.getEnd(), Tok.getLocation())) - break; - if (Tok.is(tok::raw_identifier)) { - IdentifierInfo &Info = Result.Context->Idents.get(StringRef( - Sources.getCharacterData(Tok.getLocation()), Tok.getLength())); - Tok.setIdentifierInfo(&Info); - Tok.setKind(Info.getTokenID()); - } - Tokens.push_back(Tok); - } - return Tokens; -} - -static StringRef GetText(const Token &Tok, const SourceManager &Sources) { - return StringRef(Sources.getCharacterData(Tok.getLocation()), - Tok.getLength()); -} - -void UseOverride::check(const MatchFinder::MatchResult &Result) { - if (!Result.Context->getLangOpts().CPlusPlus11) - return; - - const FunctionDecl *Method = Result.Nodes.getStmtAs("method"); - const SourceManager &Sources = *Result.SourceManager; - - assert(Method != nullptr); - if (Method->getInstantiatedFromMemberFunction() != nullptr) - Method = Method->getInstantiatedFromMemberFunction(); - - if (Method->isImplicit() || Method->getLocation().isMacroID() || - Method->isOutOfLine()) - return; - - bool HasVirtual = Method->isVirtualAsWritten(); - bool HasOverride = Method->getAttr(); - bool HasFinal = Method->getAttr(); - - bool OnlyVirtualSpecified = HasVirtual && !HasOverride && !HasFinal; - unsigned KeywordCount = HasVirtual + HasOverride + HasFinal; - - if (!OnlyVirtualSpecified && KeywordCount == 1) - return; // Nothing to do. - - std::string Message; - - if (OnlyVirtualSpecified) { - Message = - "prefer using 'override' or (rarely) 'final' instead of 'virtual'"; - } else if (KeywordCount == 0) { - Message = "annotate this function with 'override' or (rarely) 'final'"; - } else { - StringRef Redundant = - HasVirtual ? (HasOverride && HasFinal ? "'virtual' and 'override' are" - : "'virtual' is") - : "'override' is"; - StringRef Correct = HasFinal ? "'final'" : "'override'"; - - Message = - (llvm::Twine(Redundant) + - " redundant since the function is already declared " + Correct).str(); - } - - DiagnosticBuilder Diag = diag(Method->getLocation(), Message); - - CharSourceRange FileRange = Lexer::makeFileCharRange( - CharSourceRange::getTokenRange(Method->getSourceRange()), Sources, - Result.Context->getLangOpts()); - - if (!FileRange.isValid()) - return; - - // FIXME: Instead of re-lexing and looking for specific macros such as - // 'ABSTRACT', properly store the location of 'virtual' and '= 0' in each - // FunctionDecl. - SmallVector Tokens = ParseTokens(FileRange, Result); - - // Add 'override' on inline declarations that don't already have it. - if (!HasFinal && !HasOverride) { - SourceLocation InsertLoc; - StringRef ReplacementText = "override "; - - for (Token T : Tokens) { - if (T.is(tok::kw___attribute)) { - InsertLoc = T.getLocation(); - break; - } - } - - if (Method->hasAttrs()) { - for (const clang::Attr *A : Method->getAttrs()) { - if (!A->isImplicit()) { - SourceLocation Loc = - Sources.getExpansionLoc(A->getRange().getBegin()); - if (!InsertLoc.isValid() || - Sources.isBeforeInTranslationUnit(Loc, InsertLoc)) - InsertLoc = Loc; - } - } - } - - if (InsertLoc.isInvalid() && Method->doesThisDeclarationHaveABody() && - Method->getBody() && !Method->isDefaulted()) - InsertLoc = Method->getBody()->getLocStart(); - - if (!InsertLoc.isValid()) { - if (Tokens.size() > 2 && GetText(Tokens.back(), Sources) == "0" && - GetText(Tokens[Tokens.size() - 2], Sources) == "=") { - InsertLoc = Tokens[Tokens.size() - 2].getLocation(); - } else if (GetText(Tokens.back(), Sources) == "ABSTRACT") { - InsertLoc = Tokens.back().getLocation(); - } - } - - if (!InsertLoc.isValid()) { - InsertLoc = FileRange.getEnd(); - ReplacementText = " override"; - } - Diag << FixItHint::CreateInsertion(InsertLoc, ReplacementText); - } - - if (HasFinal && HasOverride) { - SourceLocation OverrideLoc = Method->getAttr()->getLocation(); - Diag << FixItHint::CreateRemoval( - CharSourceRange::getTokenRange(OverrideLoc, OverrideLoc)); - } - - if (HasVirtual) { - for (Token Tok : Tokens) { - if (Tok.is(tok::kw_virtual)) { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Tok.getLocation(), Tok.getLocation())); - break; - } - } - } -} - -} // namespace misc -} // namespace tidy -} // namespace clang Index: clang-tidy/misc/UseOverrideCheck.h =================================================================== --- /dev/null +++ clang-tidy/misc/UseOverrideCheck.h @@ -0,0 +1,32 @@ +//===--- UseOverrideCheck.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_USE_OVERRIDE_CHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USE_OVERRIDE_CHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace misc { + +/// \brief Use C++11's 'override' and remove 'virtual' where applicable. +class UseOverrideCheck : public ClangTidyCheck { +public: + UseOverrideCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace misc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USE_OVERRIDE_CHECK_H Index: clang-tidy/misc/UseOverrideCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/misc/UseOverrideCheck.cpp @@ -0,0 +1,181 @@ +//===--- UseOverrideCheck.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 "UseOverrideCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace misc { + +void UseOverrideCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(methodDecl(isOverride()).bind("method"), this); +} + +// Re-lex the tokens to get precise locations to insert 'override' and remove +// 'virtual'. +static SmallVector +ParseTokens(CharSourceRange Range, const MatchFinder::MatchResult &Result) { + const SourceManager &Sources = *Result.SourceManager; + std::pair LocInfo = + Sources.getDecomposedLoc(Range.getBegin()); + StringRef File = Sources.getBufferData(LocInfo.first); + const char *TokenBegin = File.data() + LocInfo.second; + Lexer RawLexer(Sources.getLocForStartOfFile(LocInfo.first), + Result.Context->getLangOpts(), File.begin(), TokenBegin, + File.end()); + SmallVector Tokens; + Token Tok; + while (!RawLexer.LexFromRawLexer(Tok)) { + if (Tok.is(tok::semi) || Tok.is(tok::l_brace)) + break; + if (Sources.isBeforeInTranslationUnit(Range.getEnd(), Tok.getLocation())) + break; + if (Tok.is(tok::raw_identifier)) { + IdentifierInfo &Info = Result.Context->Idents.get(StringRef( + Sources.getCharacterData(Tok.getLocation()), Tok.getLength())); + Tok.setIdentifierInfo(&Info); + Tok.setKind(Info.getTokenID()); + } + Tokens.push_back(Tok); + } + return Tokens; +} + +static StringRef GetText(const Token &Tok, const SourceManager &Sources) { + return StringRef(Sources.getCharacterData(Tok.getLocation()), + Tok.getLength()); +} + +void UseOverrideCheck::check(const MatchFinder::MatchResult &Result) { + if (!Result.Context->getLangOpts().CPlusPlus11) + return; + + const FunctionDecl *Method = Result.Nodes.getStmtAs("method"); + const SourceManager &Sources = *Result.SourceManager; + + assert(Method != nullptr); + if (Method->getInstantiatedFromMemberFunction() != nullptr) + Method = Method->getInstantiatedFromMemberFunction(); + + if (Method->isImplicit() || Method->getLocation().isMacroID() || + Method->isOutOfLine()) + return; + + bool HasVirtual = Method->isVirtualAsWritten(); + bool HasOverride = Method->getAttr(); + bool HasFinal = Method->getAttr(); + + bool OnlyVirtualSpecified = HasVirtual && !HasOverride && !HasFinal; + unsigned KeywordCount = HasVirtual + HasOverride + HasFinal; + + if (!OnlyVirtualSpecified && KeywordCount == 1) + return; // Nothing to do. + + std::string Message; + + if (OnlyVirtualSpecified) { + Message = + "prefer using 'override' or (rarely) 'final' instead of 'virtual'"; + } else if (KeywordCount == 0) { + Message = "annotate this function with 'override' or (rarely) 'final'"; + } else { + StringRef Redundant = + HasVirtual ? (HasOverride && HasFinal ? "'virtual' and 'override' are" + : "'virtual' is") + : "'override' is"; + StringRef Correct = HasFinal ? "'final'" : "'override'"; + + Message = + (llvm::Twine(Redundant) + + " redundant since the function is already declared " + Correct).str(); + } + + DiagnosticBuilder Diag = diag(Method->getLocation(), Message); + + CharSourceRange FileRange = Lexer::makeFileCharRange( + CharSourceRange::getTokenRange(Method->getSourceRange()), Sources, + Result.Context->getLangOpts()); + + if (!FileRange.isValid()) + return; + + // FIXME: Instead of re-lexing and looking for specific macros such as + // 'ABSTRACT', properly store the location of 'virtual' and '= 0' in each + // FunctionDecl. + SmallVector Tokens = ParseTokens(FileRange, Result); + + // Add 'override' on inline declarations that don't already have it. + if (!HasFinal && !HasOverride) { + SourceLocation InsertLoc; + StringRef ReplacementText = "override "; + + for (Token T : Tokens) { + if (T.is(tok::kw___attribute)) { + InsertLoc = T.getLocation(); + break; + } + } + + if (Method->hasAttrs()) { + for (const clang::Attr *A : Method->getAttrs()) { + if (!A->isImplicit()) { + SourceLocation Loc = + Sources.getExpansionLoc(A->getRange().getBegin()); + if (!InsertLoc.isValid() || + Sources.isBeforeInTranslationUnit(Loc, InsertLoc)) + InsertLoc = Loc; + } + } + } + + if (InsertLoc.isInvalid() && Method->doesThisDeclarationHaveABody() && + Method->getBody() && !Method->isDefaulted()) + InsertLoc = Method->getBody()->getLocStart(); + + if (!InsertLoc.isValid()) { + if (Tokens.size() > 2 && GetText(Tokens.back(), Sources) == "0" && + GetText(Tokens[Tokens.size() - 2], Sources) == "=") { + InsertLoc = Tokens[Tokens.size() - 2].getLocation(); + } else if (GetText(Tokens.back(), Sources) == "ABSTRACT") { + InsertLoc = Tokens.back().getLocation(); + } + } + + if (!InsertLoc.isValid()) { + InsertLoc = FileRange.getEnd(); + ReplacementText = " override"; + } + Diag << FixItHint::CreateInsertion(InsertLoc, ReplacementText); + } + + if (HasFinal && HasOverride) { + SourceLocation OverrideLoc = Method->getAttr()->getLocation(); + Diag << FixItHint::CreateRemoval( + CharSourceRange::getTokenRange(OverrideLoc, OverrideLoc)); + } + + if (HasVirtual) { + for (Token Tok : Tokens) { + if (Tok.is(tok::kw_virtual)) { + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + Tok.getLocation(), Tok.getLocation())); + break; + } + } + } +} + +} // namespace misc +} // namespace tidy +} // namespace clang