diff options
Diffstat (limited to 'clang-tools-extra/clang-tidy/bugprone')
7 files changed, 301 insertions, 46 deletions
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index ed1fd13..824ebdf 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -38,6 +38,7 @@ #include "IncorrectRoundingsCheck.h" #include "InfiniteLoopCheck.h" #include "IntegerDivisionCheck.h" +#include "InvalidEnumDefaultInitializationCheck.h" #include "LambdaFunctionNameCheck.h" #include "MacroParenthesesCheck.h" #include "MacroRepeatedSideEffectsCheck.h" @@ -165,6 +166,8 @@ public: CheckFactories.registerCheck<InfiniteLoopCheck>("bugprone-infinite-loop"); CheckFactories.registerCheck<IntegerDivisionCheck>( "bugprone-integer-division"); + CheckFactories.registerCheck<InvalidEnumDefaultInitializationCheck>( + "bugprone-invalid-enum-default-initialization"); CheckFactories.registerCheck<LambdaFunctionNameCheck>( "bugprone-lambda-function-name"); CheckFactories.registerCheck<MacroParenthesesCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index d862794..59928e5 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -30,6 +30,7 @@ add_clang_library(clangTidyBugproneModule STATIC InaccurateEraseCheck.cpp IncorrectEnableIfCheck.cpp IncorrectEnableSharedFromThisCheck.cpp + InvalidEnumDefaultInitializationCheck.cpp UnintendedCharOstreamOutputCheck.cpp ReturnConstRefFromParameterCheck.cpp SuspiciousStringviewDataUsageCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp new file mode 100644 index 0000000..33fcf45 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp @@ -0,0 +1,180 @@ +//===--- InvalidEnumDefaultInitializationCheck.cpp - clang-tidy -----------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "InvalidEnumDefaultInitializationCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/TypeVisitor.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include <algorithm> + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +namespace { + +bool isCompleteAndHasNoZeroValue(const EnumDecl *D) { + const EnumDecl *Definition = D->getDefinition(); + return Definition && Definition->isComplete() && + !Definition->enumerators().empty() && + std::none_of(Definition->enumerator_begin(), + Definition->enumerator_end(), + [](const EnumConstantDecl *Value) { + return Value->getInitVal().isZero(); + }); +} + +AST_MATCHER(EnumDecl, isCompleteAndHasNoZeroValue) { + return isCompleteAndHasNoZeroValue(&Node); +} + +// Find an initialization which initializes the value (if it has enum type) to a +// default zero value. +AST_MATCHER(Expr, isEmptyInit) { + if (isa<CXXScalarValueInitExpr, ImplicitValueInitExpr>(&Node)) + return true; + if (const auto *Init = dyn_cast<InitListExpr>(&Node)) { + if (Init->getNumInits() == 0) + return true; + } + return false; +} + +AST_MATCHER(InitListExpr, hasArrayFiller) { return Node.hasArrayFiller(); } + +// Check if any type has a "child" type that is an enum without zero value. +// The "child" type can be an array element type or member type of a record +// type (or a recursive combination of these). In this case, if the "root" type +// is statically initialized, the enum component is initialized to zero. +class FindEnumMember : public TypeVisitor<FindEnumMember, bool> { +public: + const EnumType *FoundEnum = nullptr; + + bool VisitType(const Type *T) { + const Type *DesT = T->getUnqualifiedDesugaredType(); + if (DesT != T) + return Visit(DesT); + return false; + } + bool VisitArrayType(const ArrayType *T) { + return Visit(T->getElementType().getTypePtr()); + } + bool VisitConstantArrayType(const ConstantArrayType *T) { + return Visit(T->getElementType().getTypePtr()); + } + bool VisitEnumType(const EnumType *T) { + if (isCompleteAndHasNoZeroValue(T->getDecl())) { + FoundEnum = T; + return true; + } + return false; + } + bool VisitRecordType(const RecordType *T) { + const RecordDecl *RD = T->getDecl(); + if (RD->isUnion()) + return false; + auto VisitField = [this](const FieldDecl *F) { + return Visit(F->getType().getTypePtr()); + }; + return llvm::any_of(RD->fields(), VisitField); + } +}; + +} // namespace + +InvalidEnumDefaultInitializationCheck::InvalidEnumDefaultInitializationCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + +void InvalidEnumDefaultInitializationCheck::registerMatchers( + MatchFinder *Finder) { + auto EnumWithoutZeroValue = enumType( + hasDeclaration(enumDecl(isCompleteAndHasNoZeroValue()).bind("enum"))); + auto EnumOrArrayOfEnum = qualType(hasUnqualifiedDesugaredType( + anyOf(EnumWithoutZeroValue, + arrayType(hasElementType(qualType( + hasUnqualifiedDesugaredType(EnumWithoutZeroValue))))))); + Finder->addMatcher( + expr(isEmptyInit(), hasType(EnumOrArrayOfEnum)).bind("expr"), this); + + // Array initialization can contain an "array filler" for the (syntactically) + // unspecified elements. This expression is not found by AST matchers and can + // have any type (the array's element type). This is an implicitly generated + // initialization, so if the type contains somewhere an enum without zero + // enumerator, the zero initialization applies here. We search this array + // element type for the specific enum type manually when this matcher matches. + Finder->addMatcher(initListExpr(hasArrayFiller()).bind("array_filler_expr"), + this); +} + +void InvalidEnumDefaultInitializationCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *InitExpr = Result.Nodes.getNodeAs<Expr>("expr"); + const auto *Enum = Result.Nodes.getNodeAs<EnumDecl>("enum"); + if (!InitExpr) { + const auto *InitList = + Result.Nodes.getNodeAs<InitListExpr>("array_filler_expr"); + // Initialization of omitted array elements with array filler was found. + // Check the type for enum without zero value. + // FIXME: In this way only one enum-typed value is found, not all of these. + FindEnumMember Finder; + if (!Finder.Visit(InitList->getArrayFiller()->getType().getTypePtr())) + return; + InitExpr = InitList; + Enum = Finder.FoundEnum->getDecl(); + } + + if (!InitExpr || !Enum) + return; + + ASTContext &ACtx = Enum->getASTContext(); + SourceLocation Loc = InitExpr->getExprLoc(); + if (Loc.isInvalid()) { + if (isa<ImplicitValueInitExpr, InitListExpr>(InitExpr)) { + DynTypedNodeList Parents = ACtx.getParents(*InitExpr); + if (Parents.empty()) + return; + + if (const auto *Ctor = Parents[0].get<CXXConstructorDecl>()) { + // Try to find member initializer with the found expression and get the + // source location from it. + CXXCtorInitializer *const *CtorInit = std::find_if( + Ctor->init_begin(), Ctor->init_end(), + [InitExpr](const CXXCtorInitializer *Init) { + return Init->isMemberInitializer() && Init->getInit() == InitExpr; + }); + if (!CtorInit) + return; + Loc = (*CtorInit)->getLParenLoc(); + } else if (const auto *InitList = Parents[0].get<InitListExpr>()) { + // The expression may be implicitly generated for an initialization. + // Search for a parent initialization list with valid source location. + while (InitList->getExprLoc().isInvalid()) { + DynTypedNodeList Parents = ACtx.getParents(*InitList); + if (Parents.empty()) + return; + InitList = Parents[0].get<InitListExpr>(); + if (!InitList) + return; + } + Loc = InitList->getExprLoc(); + } + } + // If still not found a source location, omit the warning. + // Ideally all such cases (if they exist) should be handled to make the + // check more precise. + if (Loc.isInvalid()) + return; + } + diag(Loc, "enum value of type %0 initialized with invalid value of 0, " + "enum doesn't have a zero-value enumerator") + << Enum; + diag(Enum->getLocation(), "enum is defined here", DiagnosticIDs::Note); +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h new file mode 100644 index 0000000..0746c4d --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h @@ -0,0 +1,31 @@ +//===--- InvalidEnumDefaultInitializationCheck.h - clang-tidy -*- C++ -*---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INVALIDENUMDEFAULTINITIALIZATIONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INVALIDENUMDEFAULTINITIALIZATIONCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Detects default initialization (to 0) of variables with `enum` type where +/// the enum has no enumerator with value of 0. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/invalid-enum-default-initialization.html +class InvalidEnumDefaultInitializationCheck : public ClangTidyCheck { +public: + InvalidEnumDefaultInitializationCheck(StringRef Name, + ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INVALIDENUMDEFAULTINITIALIZATIONCHECK_H diff --git a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp index 8b2ca69..5378223 100644 --- a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp @@ -381,10 +381,11 @@ void NarrowingConversionsCheck::diagNarrowTypeOrConstant( const Expr &Rhs) { APValue Constant = getConstantExprValue(Context, Rhs); if (Constant.isInt()) - return diagNarrowIntegerConstant(SourceLoc, Lhs, Rhs, Constant.getInt()); - if (Constant.isFloat()) - return diagNarrowConstant(SourceLoc, Lhs, Rhs); - return diagNarrowType(SourceLoc, Lhs, Rhs); + diagNarrowIntegerConstant(SourceLoc, Lhs, Rhs, Constant.getInt()); + else if (Constant.isFloat()) + diagNarrowConstant(SourceLoc, Lhs, Rhs); + else + diagNarrowType(SourceLoc, Lhs, Rhs); } void NarrowingConversionsCheck::handleIntegralCast(const ASTContext &Context, @@ -460,10 +461,10 @@ void NarrowingConversionsCheck::handleFloatingToIntegral( llvm::APFloat FloatConstant(0.0); if (getFloatingConstantExprValue(Context, Rhs, FloatConstant)) { if (!isFloatExactlyRepresentable(Context, FloatConstant, Lhs.getType())) - return diagNarrowConstant(SourceLoc, Lhs, Rhs); + diagNarrowConstant(SourceLoc, Lhs, Rhs); - if (PedanticMode) - return diagConstantCast(SourceLoc, Lhs, Rhs); + else if (PedanticMode) + diagConstantCast(SourceLoc, Lhs, Rhs); return; } @@ -478,7 +479,7 @@ void NarrowingConversionsCheck::handleFloatingToIntegral( void NarrowingConversionsCheck::handleFloatingToBoolean( const ASTContext &Context, SourceLocation SourceLoc, const Expr &Lhs, const Expr &Rhs) { - return diagNarrowTypeOrConstant(Context, SourceLoc, Lhs, Rhs); + diagNarrowTypeOrConstant(Context, SourceLoc, Lhs, Rhs); } void NarrowingConversionsCheck::handleBooleanToSignedIntegral( @@ -532,19 +533,20 @@ void NarrowingConversionsCheck::handleBinaryOperator(const ASTContext &Context, if (LhsType == RhsType) return; if (RhsType->getKind() == BuiltinType::Bool && LhsType->isSignedInteger()) - return handleBooleanToSignedIntegral(Context, SourceLoc, Lhs, Rhs); - if (RhsType->isInteger() && LhsType->getKind() == BuiltinType::Bool) - return handleIntegralToBoolean(Context, SourceLoc, Lhs, Rhs); - if (RhsType->isInteger() && LhsType->isFloatingPoint()) - return handleIntegralToFloating(Context, SourceLoc, Lhs, Rhs); - if (RhsType->isInteger() && LhsType->isInteger()) - return handleIntegralCast(Context, SourceLoc, Lhs, Rhs); - if (RhsType->isFloatingPoint() && LhsType->getKind() == BuiltinType::Bool) - return handleFloatingToBoolean(Context, SourceLoc, Lhs, Rhs); - if (RhsType->isFloatingPoint() && LhsType->isInteger()) - return handleFloatingToIntegral(Context, SourceLoc, Lhs, Rhs); - if (RhsType->isFloatingPoint() && LhsType->isFloatingPoint()) - return handleFloatingCast(Context, SourceLoc, Lhs, Rhs); + handleBooleanToSignedIntegral(Context, SourceLoc, Lhs, Rhs); + else if (RhsType->isInteger() && LhsType->getKind() == BuiltinType::Bool) + handleIntegralToBoolean(Context, SourceLoc, Lhs, Rhs); + else if (RhsType->isInteger() && LhsType->isFloatingPoint()) + handleIntegralToFloating(Context, SourceLoc, Lhs, Rhs); + else if (RhsType->isInteger() && LhsType->isInteger()) + handleIntegralCast(Context, SourceLoc, Lhs, Rhs); + else if (RhsType->isFloatingPoint() && + LhsType->getKind() == BuiltinType::Bool) + handleFloatingToBoolean(Context, SourceLoc, Lhs, Rhs); + else if (RhsType->isFloatingPoint() && LhsType->isInteger()) + handleFloatingToIntegral(Context, SourceLoc, Lhs, Rhs); + else if (RhsType->isFloatingPoint() && LhsType->isFloatingPoint()) + handleFloatingCast(Context, SourceLoc, Lhs, Rhs); } bool NarrowingConversionsCheck::handleConditionalOperator( @@ -577,21 +579,28 @@ void NarrowingConversionsCheck::handleImplicitCast( SourceLocation SourceLoc = Lhs.getExprLoc(); switch (Cast.getCastKind()) { case CK_BooleanToSignedIntegral: - return handleBooleanToSignedIntegral(Context, SourceLoc, Lhs, Rhs); + handleBooleanToSignedIntegral(Context, SourceLoc, Lhs, Rhs); + return; case CK_IntegralToBoolean: - return handleIntegralToBoolean(Context, SourceLoc, Lhs, Rhs); + handleIntegralToBoolean(Context, SourceLoc, Lhs, Rhs); + return; case CK_IntegralToFloating: - return handleIntegralToFloating(Context, SourceLoc, Lhs, Rhs); + handleIntegralToFloating(Context, SourceLoc, Lhs, Rhs); + return; case CK_IntegralCast: - return handleIntegralCast(Context, SourceLoc, Lhs, Rhs); + handleIntegralCast(Context, SourceLoc, Lhs, Rhs); + return; case CK_FloatingToBoolean: - return handleFloatingToBoolean(Context, SourceLoc, Lhs, Rhs); + handleFloatingToBoolean(Context, SourceLoc, Lhs, Rhs); + return; case CK_FloatingToIntegral: - return handleFloatingToIntegral(Context, SourceLoc, Lhs, Rhs); + handleFloatingToIntegral(Context, SourceLoc, Lhs, Rhs); + return; case CK_FloatingCast: - return handleFloatingCast(Context, SourceLoc, Lhs, Rhs); + handleFloatingCast(Context, SourceLoc, Lhs, Rhs); + return; default: - break; + return; } } @@ -610,9 +619,10 @@ void NarrowingConversionsCheck::handleBinaryOperator(const ASTContext &Context, void NarrowingConversionsCheck::check(const MatchFinder::MatchResult &Result) { if (const auto *Op = Result.Nodes.getNodeAs<BinaryOperator>("binary_op")) - return handleBinaryOperator(*Result.Context, *Op); - if (const auto *Cast = Result.Nodes.getNodeAs<ImplicitCastExpr>("cast")) - return handleImplicitCast(*Result.Context, *Cast); - llvm_unreachable("must be binary operator or cast expression"); + handleBinaryOperator(*Result.Context, *Op); + else if (const auto *Cast = Result.Nodes.getNodeAs<ImplicitCastExpr>("cast")) + handleImplicitCast(*Result.Context, *Cast); + else + llvm_unreachable("must be binary operator or cast expression"); } } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp index ea3b8b8..dfd3cbf 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp @@ -74,17 +74,25 @@ void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) { charCastExpression(true, IntegerType, "signedCastExpression"); const auto UnSignedCharCastExpr = charCastExpression(false, IntegerType, "unsignedCastExpression"); + const bool IsC23 = getLangOpts().C23; - // Catch assignments with signed char -> integer conversion. + // Catch assignments with signed char -> integer conversion. Ignore false + // positives on C23 enums with the fixed underlying type of signed char. const auto AssignmentOperatorExpr = expr(binaryOperator(hasOperatorName("="), hasLHS(hasType(IntegerType)), - hasRHS(SignedCharCastExpr))); + hasRHS(SignedCharCastExpr)), + IsC23 ? unless(binaryOperator( + hasLHS(hasType(hasCanonicalType(enumType()))))) + : Matcher<Stmt>(anything())); Finder->addMatcher(AssignmentOperatorExpr, this); - // Catch declarations with signed char -> integer conversion. - const auto Declaration = varDecl(isDefinition(), hasType(IntegerType), - hasInitializer(SignedCharCastExpr)); + // Catch declarations with signed char -> integer conversion. Ignore false + // positives on C23 enums with the fixed underlying type of signed char. + const auto Declaration = varDecl( + isDefinition(), hasType(IntegerType), hasInitializer(SignedCharCastExpr), + IsC23 ? unless(hasType(hasCanonicalType(enumType()))) + : Matcher<VarDecl>(anything())); Finder->addMatcher(Declaration, this); diff --git a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp index 1f432c4..c4c4267 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp @@ -69,6 +69,28 @@ void UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) { cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl( hasName("operator="), ofClass(equalsBoundNode("class")))))))); + // Checking that some kind of constructor is called and followed by a `swap`: + // T& operator=(const T& other) { + // T tmp{this->internal_data(), some, other, args}; + // swap(tmp); + // return *this; + // } + const auto HasCopyAndSwap = cxxMethodDecl( + ofClass(cxxRecordDecl()), + hasBody(compoundStmt( + hasDescendant( + varDecl(hasType(cxxRecordDecl(equalsBoundNode("class")))) + .bind("tmp_var")), + hasDescendant(stmt(anyOf( + cxxMemberCallExpr(hasArgument( + 0, declRefExpr(to(varDecl(equalsBoundNode("tmp_var")))))), + callExpr( + callee(functionDecl(hasName("swap"))), argumentCountIs(2), + hasAnyArgument( + declRefExpr(to(varDecl(equalsBoundNode("tmp_var"))))), + hasAnyArgument(unaryOperator(has(cxxThisExpr()), + hasOperatorName("*")))))))))); + DeclarationMatcher AdditionalMatcher = cxxMethodDecl(); if (WarnOnlyIfThisHasSuspiciousField) { // Matcher for standard smart pointers. @@ -89,14 +111,14 @@ void UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) { hasType(arrayType()))))))); } - Finder->addMatcher(cxxMethodDecl(ofClass(cxxRecordDecl().bind("class")), - isCopyAssignmentOperator(), IsUserDefined, - HasReferenceParam, HasNoSelfCheck, - unless(HasNonTemplateSelfCopy), - unless(HasTemplateSelfCopy), - HasNoNestedSelfAssign, AdditionalMatcher) - .bind("copyAssignmentOperator"), - this); + Finder->addMatcher( + cxxMethodDecl( + ofClass(cxxRecordDecl().bind("class")), isCopyAssignmentOperator(), + IsUserDefined, HasReferenceParam, HasNoSelfCheck, + unless(HasNonTemplateSelfCopy), unless(HasTemplateSelfCopy), + unless(HasCopyAndSwap), HasNoNestedSelfAssign, AdditionalMatcher) + .bind("copyAssignmentOperator"), + this); } void UnhandledSelfAssignmentCheck::check( |