diff options
author | Alexander Kornienko <alexfh@google.com> | 2018-02-28 14:47:20 +0000 |
---|---|---|
committer | Alexander Kornienko <alexfh@google.com> | 2018-02-28 14:47:20 +0000 |
commit | 275bc46a02c68e59f8d0c5a292b6309a22051346 (patch) | |
tree | 2950475c335cf9a9050fb4de1c955cdb5a0de699 /clang-tidy/bugprone | |
parent | c51bb3d45b9a101597dd59d235dfde1261a20eb8 (diff) |
Rename a few checks from misc- to bugprone-.
Summary:
rename_check.py {misc,bugprone}-forwarding-reference-overload
rename_check.py {misc,bugprone}-macro-repeated-side-effects
rename_check.py {misc,bugprone}-lambda-function-name
rename_check.py {misc,bugprone}-misplaced-widening-cast
Reviewers: hokein, sammccall, aaron.ballman
Reviewed By: aaron.ballman
Subscribers: klimek, cfe-commits, mgorny
Differential Revision: https://reviews.llvm.org/D43867
git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@326327 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'clang-tidy/bugprone')
-rw-r--r-- | clang-tidy/bugprone/BugproneTidyModule.cpp | 12 | ||||
-rw-r--r-- | clang-tidy/bugprone/CMakeLists.txt | 4 | ||||
-rw-r--r-- | clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp | 148 | ||||
-rw-r--r-- | clang-tidy/bugprone/ForwardingReferenceOverloadCheck.h | 42 | ||||
-rw-r--r-- | clang-tidy/bugprone/LambdaFunctionNameCheck.cpp | 99 | ||||
-rw-r--r-- | clang-tidy/bugprone/LambdaFunctionNameCheck.h | 51 | ||||
-rw-r--r-- | clang-tidy/bugprone/MacroRepeatedSideEffectsCheck.cpp | 184 | ||||
-rw-r--r-- | clang-tidy/bugprone/MacroRepeatedSideEffectsCheck.h | 31 | ||||
-rw-r--r-- | clang-tidy/bugprone/MisplacedWideningCastCheck.cpp | 233 | ||||
-rw-r--r-- | clang-tidy/bugprone/MisplacedWideningCastCheck.h | 46 |
10 files changed, 850 insertions, 0 deletions
diff --git a/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tidy/bugprone/BugproneTidyModule.cpp index 68effd14..5e67672f 100644 --- a/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -17,10 +17,14 @@ #include "DanglingHandleCheck.h" #include "FoldInitTypeCheck.h" #include "ForwardDeclarationNamespaceCheck.h" +#include "ForwardingReferenceOverloadCheck.h" #include "InaccurateEraseCheck.h" #include "IncorrectRoundingsCheck.h" #include "IntegerDivisionCheck.h" +#include "LambdaFunctionNameCheck.h" +#include "MacroRepeatedSideEffectsCheck.h" #include "MisplacedOperatorInStrlenInAllocCheck.h" +#include "MisplacedWideningCastCheck.h" #include "MoveForwardingReferenceCheck.h" #include "MultipleStatementMacroCheck.h" #include "StringConstructorCheck.h" @@ -51,14 +55,22 @@ public: "bugprone-fold-init-type"); CheckFactories.registerCheck<ForwardDeclarationNamespaceCheck>( "bugprone-forward-declaration-namespace"); + CheckFactories.registerCheck<ForwardingReferenceOverloadCheck>( + "bugprone-forwarding-reference-overload"); CheckFactories.registerCheck<InaccurateEraseCheck>( "bugprone-inaccurate-erase"); CheckFactories.registerCheck<IncorrectRoundingsCheck>( "bugprone-incorrect-roundings"); CheckFactories.registerCheck<IntegerDivisionCheck>( "bugprone-integer-division"); + CheckFactories.registerCheck<LambdaFunctionNameCheck>( + "bugprone-lambda-function-name"); + CheckFactories.registerCheck<MacroRepeatedSideEffectsCheck>( + "bugprone-macro-repeated-side-effects"); CheckFactories.registerCheck<MisplacedOperatorInStrlenInAllocCheck>( "bugprone-misplaced-operator-in-strlen-in-alloc"); + CheckFactories.registerCheck<MisplacedWideningCastCheck>( + "bugprone-misplaced-widening-cast"); CheckFactories.registerCheck<MoveForwardingReferenceCheck>( "bugprone-move-forwarding-reference"); CheckFactories.registerCheck<MultipleStatementMacroCheck>( diff --git a/clang-tidy/bugprone/CMakeLists.txt b/clang-tidy/bugprone/CMakeLists.txt index 54d93ef2..73b4e55f 100644 --- a/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tidy/bugprone/CMakeLists.txt @@ -9,10 +9,14 @@ add_clang_library(clangTidyBugproneModule DanglingHandleCheck.cpp FoldInitTypeCheck.cpp ForwardDeclarationNamespaceCheck.cpp + ForwardingReferenceOverloadCheck.cpp InaccurateEraseCheck.cpp IncorrectRoundingsCheck.cpp IntegerDivisionCheck.cpp + LambdaFunctionNameCheck.cpp + MacroRepeatedSideEffectsCheck.cpp MisplacedOperatorInStrlenInAllocCheck.cpp + MisplacedWideningCastCheck.cpp MoveForwardingReferenceCheck.cpp MultipleStatementMacroCheck.cpp StringConstructorCheck.cpp diff --git a/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp b/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp new file mode 100644 index 00000000..17bdc76c --- /dev/null +++ b/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp @@ -0,0 +1,148 @@ +//===--- ForwardingReferenceOverloadCheck.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 "ForwardingReferenceOverloadCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include <algorithm> + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +namespace { +// Check if the given type is related to std::enable_if. +AST_MATCHER(QualType, isEnableIf) { + auto CheckTemplate = [](const TemplateSpecializationType *Spec) { + if (!Spec || !Spec->getTemplateName().getAsTemplateDecl()) { + return false; + } + const NamedDecl *TypeDecl = + Spec->getTemplateName().getAsTemplateDecl()->getTemplatedDecl(); + return TypeDecl->isInStdNamespace() && + (TypeDecl->getName().equals("enable_if") || + TypeDecl->getName().equals("enable_if_t")); + }; + const Type *BaseType = Node.getTypePtr(); + // Case: pointer or reference to enable_if. + while (BaseType->isPointerType() || BaseType->isReferenceType()) { + BaseType = BaseType->getPointeeType().getTypePtr(); + } + // Case: type parameter dependent (enable_if<is_integral<T>>). + if (const auto *Dependent = BaseType->getAs<DependentNameType>()) { + BaseType = Dependent->getQualifier()->getAsType(); + } + if (!BaseType) + return false; + if (CheckTemplate(BaseType->getAs<TemplateSpecializationType>())) { + return true; // Case: enable_if_t< >. + } else if (const auto *Elaborated = BaseType->getAs<ElaboratedType>()) { + if (const auto *Qualifier = Elaborated->getQualifier()->getAsType()) { + if (CheckTemplate(Qualifier->getAs<TemplateSpecializationType>())) { + return true; // Case: enable_if< >::type. + } + } + } + return false; +} +AST_MATCHER_P(TemplateTypeParmDecl, hasDefaultArgument, + clang::ast_matchers::internal::Matcher<QualType>, TypeMatcher) { + return Node.hasDefaultArgument() && + TypeMatcher.matches(Node.getDefaultArgument(), Finder, Builder); +} +} // namespace + +void ForwardingReferenceOverloadCheck::registerMatchers(MatchFinder *Finder) { + // Forwarding references require C++11 or later. + if (!getLangOpts().CPlusPlus11) + return; + + auto ForwardingRefParm = + parmVarDecl( + hasType(qualType(rValueReferenceType(), + references(templateTypeParmType(hasDeclaration( + templateTypeParmDecl().bind("type-parm-decl")))), + unless(references(isConstQualified()))))) + .bind("parm-var"); + + DeclarationMatcher findOverload = + cxxConstructorDecl( + hasParameter(0, ForwardingRefParm), + unless(hasAnyParameter( + // No warning: enable_if as constructor parameter. + parmVarDecl(hasType(isEnableIf())))), + unless(hasParent(functionTemplateDecl(has(templateTypeParmDecl( + // No warning: enable_if as type parameter. + hasDefaultArgument(isEnableIf()))))))) + .bind("ctor"); + Finder->addMatcher(findOverload, this); +} + +void ForwardingReferenceOverloadCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *ParmVar = Result.Nodes.getNodeAs<ParmVarDecl>("parm-var"); + const auto *TypeParmDecl = + Result.Nodes.getNodeAs<TemplateTypeParmDecl>("type-parm-decl"); + + // Get the FunctionDecl and FunctionTemplateDecl containing the function + // parameter. + const auto *FuncForParam = dyn_cast<FunctionDecl>(ParmVar->getDeclContext()); + if (!FuncForParam) + return; + const FunctionTemplateDecl *FuncTemplate = + FuncForParam->getDescribedFunctionTemplate(); + if (!FuncTemplate) + return; + + // Check that the template type parameter belongs to the same function + // template as the function parameter of that type. (This implies that type + // deduction will happen on the type.) + const TemplateParameterList *Params = FuncTemplate->getTemplateParameters(); + if (std::find(Params->begin(), Params->end(), TypeParmDecl) == Params->end()) + return; + + // Every parameter after the first must have a default value. + const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor"); + for (auto Iter = Ctor->param_begin() + 1; Iter != Ctor->param_end(); ++Iter) { + if (!(*Iter)->hasDefaultArg()) + return; + } + bool EnabledCopy = false, DisabledCopy = false, EnabledMove = false, + DisabledMove = false; + for (const auto *OtherCtor : Ctor->getParent()->ctors()) { + if (OtherCtor->isCopyOrMoveConstructor()) { + if (OtherCtor->isDeleted() || OtherCtor->getAccess() == AS_private) + (OtherCtor->isCopyConstructor() ? DisabledCopy : DisabledMove) = true; + else + (OtherCtor->isCopyConstructor() ? EnabledCopy : EnabledMove) = true; + } + } + bool Copy = (!EnabledMove && !DisabledMove && !DisabledCopy) || EnabledCopy; + bool Move = !DisabledMove || EnabledMove; + if (!Copy && !Move) + return; + diag(Ctor->getLocation(), + "constructor accepting a forwarding reference can " + "hide the %select{copy|move|copy and move}0 constructor%s1") + << (Copy && Move ? 2 : (Copy ? 0 : 1)) << Copy + Move; + for (const auto *OtherCtor : Ctor->getParent()->ctors()) { + if (OtherCtor->isCopyOrMoveConstructor() && !OtherCtor->isDeleted() && + OtherCtor->getAccess() != AS_private) { + diag(OtherCtor->getLocation(), + "%select{copy|move}0 constructor declared here", DiagnosticIDs::Note) + << OtherCtor->isMoveConstructor(); + } + } +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang diff --git a/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.h b/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.h new file mode 100644 index 00000000..4b00ab29 --- /dev/null +++ b/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.h @@ -0,0 +1,42 @@ +//===--- ForwardingReferenceOverloadCheck.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_FORWARDINGREFERENCEOVERLOADCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_FORWARDINGREFERENCEOVERLOADCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// The checker looks for constructors that can act as copy or move constructors +/// through their forwarding reference parameters. If a non const lvalue +/// reference is passed to the constructor, the forwarding reference parameter +/// can be a perfect match while the const reference parameter of the copy +/// constructor can't. The forwarding reference constructor will be called, +/// which can lead to confusion. +/// For detailed description of this problem see: Scott Meyers, Effective Modern +/// C++ Design, item 26. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-forwarding-reference-overload.html +class ForwardingReferenceOverloadCheck : public ClangTidyCheck { +public: + ForwardingReferenceOverloadCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_FORWARDINGREFERENCEOVERLOADCHECK_H diff --git a/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp b/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp new file mode 100644 index 00000000..60eb245c --- /dev/null +++ b/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp @@ -0,0 +1,99 @@ +//===--- LambdaFunctionNameCheck.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 "LambdaFunctionNameCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/MacroInfo.h" +#include "clang/Lex/Preprocessor.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +namespace { + +// Keep track of macro expansions that contain both __FILE__ and __LINE__. If +// such a macro also uses __func__ or __FUNCTION__, we don't want to issue a +// warning because __FILE__ and __LINE__ may be useful even if __func__ or +// __FUNCTION__ is not, especially if the macro could be used in the context of +// either a function body or a lambda body. +class MacroExpansionsWithFileAndLine : public PPCallbacks { +public: + explicit MacroExpansionsWithFileAndLine( + LambdaFunctionNameCheck::SourceRangeSet *SME) + : SuppressMacroExpansions(SME) {} + + void MacroExpands(const Token &MacroNameTok, + const MacroDefinition &MD, SourceRange Range, + const MacroArgs *Args) override { + bool has_file = false; + bool has_line = false; + for (const auto& T : MD.getMacroInfo()->tokens()) { + if (T.is(tok::identifier)) { + StringRef IdentName = T.getIdentifierInfo()->getName(); + if (IdentName == "__FILE__") { + has_file = true; + } else if (IdentName == "__LINE__") { + has_line = true; + } + } + } + if (has_file && has_line) { + SuppressMacroExpansions->insert(Range); + } + } + +private: + LambdaFunctionNameCheck::SourceRangeSet* SuppressMacroExpansions; +}; + +} // namespace + +void LambdaFunctionNameCheck::registerMatchers(MatchFinder *Finder) { + // Match on PredefinedExprs inside a lambda. + Finder->addMatcher(predefinedExpr(hasAncestor(lambdaExpr())).bind("E"), + this); +} + +void LambdaFunctionNameCheck::registerPPCallbacks(CompilerInstance &Compiler) { + Compiler.getPreprocessor().addPPCallbacks( + llvm::make_unique<MacroExpansionsWithFileAndLine>( + &SuppressMacroExpansions)); +} + +void LambdaFunctionNameCheck::check(const MatchFinder::MatchResult &Result) { + const auto *E = Result.Nodes.getNodeAs<PredefinedExpr>("E"); + if (E->getIdentType() != PredefinedExpr::Func && + E->getIdentType() != PredefinedExpr::Function) { + // We don't care about other PredefinedExprs. + return; + } + if (E->getLocation().isMacroID()) { + auto ER = + Result.SourceManager->getImmediateExpansionRange(E->getLocation()); + if (SuppressMacroExpansions.find(SourceRange(ER.first, ER.second)) != + SuppressMacroExpansions.end()) { + // This is a macro expansion for which we should not warn. + return; + } + } + diag(E->getLocation(), + "inside a lambda, '%0' expands to the name of the function call " + "operator; consider capturing the name of the enclosing function " + "explicitly") + << PredefinedExpr::getIdentTypeName(E->getIdentType()); +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang diff --git a/clang-tidy/bugprone/LambdaFunctionNameCheck.h b/clang-tidy/bugprone/LambdaFunctionNameCheck.h new file mode 100644 index 00000000..b7b1b441 --- /dev/null +++ b/clang-tidy/bugprone/LambdaFunctionNameCheck.h @@ -0,0 +1,51 @@ +//===--- LambdaFunctionNameCheck.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_LAMBDAFUNCTIONNAMECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_LAMBDAFUNCTIONNAMECHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Detect when __func__ or __FUNCTION__ is being used from within a lambda. In +/// that context, those expressions expand to the name of the call operator +/// (i.e., `operator()`). +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-lambda-function-name.html +class LambdaFunctionNameCheck : public ClangTidyCheck { +public: + struct SourceRangeLessThan { + bool operator()(const SourceRange &L, const SourceRange &R) const { + if (L.getBegin() == R.getBegin()) { + return L.getEnd() < R.getEnd(); + } + return L.getBegin() < R.getBegin(); + } + }; + using SourceRangeSet = std::set<SourceRange, SourceRangeLessThan>; + + LambdaFunctionNameCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void registerPPCallbacks(CompilerInstance &Compiler) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + SourceRangeSet SuppressMacroExpansions; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_LAMBDAFUNCTIONNAMECHECK_H diff --git a/clang-tidy/bugprone/MacroRepeatedSideEffectsCheck.cpp b/clang-tidy/bugprone/MacroRepeatedSideEffectsCheck.cpp new file mode 100644 index 00000000..30c770e3 --- /dev/null +++ b/clang-tidy/bugprone/MacroRepeatedSideEffectsCheck.cpp @@ -0,0 +1,184 @@ +//===--- MacroRepeatedSideEffectsCheck.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 "MacroRepeatedSideEffectsCheck.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/MacroArgs.h" +#include "clang/Lex/PPCallbacks.h" +#include "clang/Lex/Preprocessor.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +namespace { +class MacroRepeatedPPCallbacks : public PPCallbacks { +public: + MacroRepeatedPPCallbacks(ClangTidyCheck &Check, Preprocessor &PP) + : Check(Check), PP(PP) {} + + void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD, + SourceRange Range, const MacroArgs *Args) override; + +private: + ClangTidyCheck &Check; + Preprocessor &PP; + + unsigned countArgumentExpansions(const MacroInfo *MI, + const IdentifierInfo *Arg) const; + + bool hasSideEffects(const Token *ResultArgToks) const; +}; +} // End of anonymous namespace. + +void MacroRepeatedPPCallbacks::MacroExpands(const Token &MacroNameTok, + const MacroDefinition &MD, + SourceRange Range, + const MacroArgs *Args) { + // Ignore macro argument expansions. + if (!Range.getBegin().isFileID()) + return; + + const MacroInfo *MI = MD.getMacroInfo(); + + // Bail out if the contents of the macro are containing keywords that are + // making the macro too complex. + if (std::find_if( + MI->tokens().begin(), MI->tokens().end(), [](const Token &T) { + return T.isOneOf(tok::kw_if, tok::kw_else, tok::kw_switch, + tok::kw_case, tok::kw_break, tok::kw_while, + tok::kw_do, tok::kw_for, tok::kw_continue, + tok::kw_goto, tok::kw_return); + }) != MI->tokens().end()) + return; + + for (unsigned ArgNo = 0U; ArgNo < MI->getNumParams(); ++ArgNo) { + const IdentifierInfo *Arg = *(MI->param_begin() + ArgNo); + const Token *ResultArgToks = Args->getUnexpArgument(ArgNo); + + if (hasSideEffects(ResultArgToks) && + countArgumentExpansions(MI, Arg) >= 2) { + Check.diag(ResultArgToks->getLocation(), + "side effects in the %ordinal0 macro argument %1 are " + "repeated in macro expansion") + << (ArgNo + 1) << Arg; + Check.diag(MI->getDefinitionLoc(), "macro %0 defined here", + DiagnosticIDs::Note) + << MacroNameTok.getIdentifierInfo(); + } + } +} + +unsigned MacroRepeatedPPCallbacks::countArgumentExpansions( + const MacroInfo *MI, const IdentifierInfo *Arg) const { + // Current argument count. When moving forward to a different control-flow + // path this can decrease. + unsigned Current = 0; + // Max argument count. + unsigned Max = 0; + bool SkipParen = false; + int SkipParenCount = 0; + // Has a __builtin_constant_p been found? + bool FoundBuiltin = false; + bool PrevTokenIsHash = false; + // Count when "?" is reached. The "Current" will get this value when the ":" + // is reached. + std::stack<unsigned, SmallVector<unsigned, 8>> CountAtQuestion; + for (const auto &T : MI->tokens()) { + // The result of __builtin_constant_p(x) is 0 if x is a macro argument + // with side effects. If we see a __builtin_constant_p(x) followed by a + // "?" "&&" or "||", then we need to reason about control flow to report + // warnings correctly. Until such reasoning is added, bail out when this + // happens. + if (FoundBuiltin && T.isOneOf(tok::question, tok::ampamp, tok::pipepipe)) + return Max; + + // Skip stringified tokens. + if (T.is(tok::hash)) { + PrevTokenIsHash = true; + continue; + } + if (PrevTokenIsHash) { + PrevTokenIsHash = false; + continue; + } + + // Handling of ? and :. + if (T.is(tok::question)) { + CountAtQuestion.push(Current); + } else if (T.is(tok::colon)) { + if (CountAtQuestion.empty()) + return 0; + Current = CountAtQuestion.top(); + CountAtQuestion.pop(); + } + + // If current token is a parenthesis, skip it. + if (SkipParen) { + if (T.is(tok::l_paren)) + SkipParenCount++; + else if (T.is(tok::r_paren)) + SkipParenCount--; + SkipParen = (SkipParenCount != 0); + if (SkipParen) + continue; + } + + IdentifierInfo *TII = T.getIdentifierInfo(); + // If not existent, skip it. + if (TII == nullptr) + continue; + + // If a __builtin_constant_p is found within the macro definition, don't + // count arguments inside the parentheses and remember that it has been + // seen in case there are "?", "&&" or "||" operators later. + if (TII->getBuiltinID() == Builtin::BI__builtin_constant_p) { + FoundBuiltin = true; + SkipParen = true; + continue; + } + + // If another macro is found within the macro definition, skip the macro + // and the eventual arguments. + if (TII->hasMacroDefinition()) { + const MacroInfo *M = PP.getMacroDefinition(TII).getMacroInfo(); + if (M != nullptr && M->isFunctionLike()) + SkipParen = true; + continue; + } + + // Count argument. + if (TII == Arg) { + Current++; + if (Current > Max) + Max = Current; + } + } + return Max; +} + +bool MacroRepeatedPPCallbacks::hasSideEffects( + const Token *ResultArgToks) const { + for (; ResultArgToks->isNot(tok::eof); ++ResultArgToks) { + if (ResultArgToks->isOneOf(tok::plusplus, tok::minusminus)) + return true; + } + return false; +} + +void MacroRepeatedSideEffectsCheck::registerPPCallbacks( + CompilerInstance &Compiler) { + Compiler.getPreprocessor().addPPCallbacks( + ::llvm::make_unique<MacroRepeatedPPCallbacks>( + *this, Compiler.getPreprocessor())); +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang diff --git a/clang-tidy/bugprone/MacroRepeatedSideEffectsCheck.h b/clang-tidy/bugprone/MacroRepeatedSideEffectsCheck.h new file mode 100644 index 00000000..a2a31341 --- /dev/null +++ b/clang-tidy/bugprone/MacroRepeatedSideEffectsCheck.h @@ -0,0 +1,31 @@ +//===--- MacroRepeatedSideEffectsCheck.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_MACROREPEATEDSIDEEFFECTSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MACROREPEATEDSIDEEFFECTSCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Checks for repeated argument with side effects in macros. +class MacroRepeatedSideEffectsCheck : public ClangTidyCheck { +public: + MacroRepeatedSideEffectsCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerPPCallbacks(CompilerInstance &Compiler) override; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MACROREPEATEDSIDEEFFECTSCHECK_H diff --git a/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp b/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp new file mode 100644 index 00000000..e263366b --- /dev/null +++ b/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp @@ -0,0 +1,233 @@ +//===--- MisplacedWideningCastCheck.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 "MisplacedWideningCastCheck.h" +#include "../utils/Matchers.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +MisplacedWideningCastCheck::MisplacedWideningCastCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + CheckImplicitCasts(Options.get("CheckImplicitCasts", false)) {} + +void MisplacedWideningCastCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "CheckImplicitCasts", CheckImplicitCasts); +} + +void MisplacedWideningCastCheck::registerMatchers(MatchFinder *Finder) { + const auto Calc = + expr(anyOf(binaryOperator( + anyOf(hasOperatorName("+"), hasOperatorName("-"), + hasOperatorName("*"), hasOperatorName("<<"))), + unaryOperator(hasOperatorName("~"))), + hasType(isInteger())) + .bind("Calc"); + + const auto ExplicitCast = explicitCastExpr(hasDestinationType(isInteger()), + has(ignoringParenImpCasts(Calc))); + const auto ImplicitCast = + implicitCastExpr(hasImplicitDestinationType(isInteger()), + has(ignoringParenImpCasts(Calc))); + const auto Cast = expr(anyOf(ExplicitCast, ImplicitCast)).bind("Cast"); + + Finder->addMatcher(varDecl(hasInitializer(Cast)), this); + Finder->addMatcher(returnStmt(hasReturnValue(Cast)), this); + Finder->addMatcher(callExpr(hasAnyArgument(Cast)), this); + Finder->addMatcher(binaryOperator(hasOperatorName("="), hasRHS(Cast)), this); + Finder->addMatcher( + binaryOperator(matchers::isComparisonOperator(), hasEitherOperand(Cast)), + this); +} + +static unsigned getMaxCalculationWidth(const ASTContext &Context, + const Expr *E) { + E = E->IgnoreParenImpCasts(); + + if (const auto *Bop = dyn_cast<BinaryOperator>(E)) { + unsigned LHSWidth = getMaxCalculationWidth(Context, Bop->getLHS()); + unsigned RHSWidth = getMaxCalculationWidth(Context, Bop->getRHS()); + if (Bop->getOpcode() == BO_Mul) + return LHSWidth + RHSWidth; + if (Bop->getOpcode() == BO_Add) + return std::max(LHSWidth, RHSWidth) + 1; + if (Bop->getOpcode() == BO_Rem) { + llvm::APSInt Val; + if (Bop->getRHS()->EvaluateAsInt(Val, Context)) + return Val.getActiveBits(); + } else if (Bop->getOpcode() == BO_Shl) { + llvm::APSInt Bits; + if (Bop->getRHS()->EvaluateAsInt(Bits, Context)) { + // We don't handle negative values and large values well. It is assumed + // that compiler warnings are written for such values so the user will + // fix that. + return LHSWidth + Bits.getExtValue(); + } + + // Unknown bitcount, assume there is truncation. + return 1024U; + } + } else if (const auto *Uop = dyn_cast<UnaryOperator>(E)) { + // There is truncation when ~ is used. + if (Uop->getOpcode() == UO_Not) + return 1024U; + + QualType T = Uop->getType(); + return T->isIntegerType() ? Context.getIntWidth(T) : 1024U; + } else if (const auto *I = dyn_cast<IntegerLiteral>(E)) { + return I->getValue().getActiveBits(); + } + + return Context.getIntWidth(E->getType()); +} + +static int relativeIntSizes(BuiltinType::Kind Kind) { + switch (Kind) { + case BuiltinType::UChar: + return 1; + case BuiltinType::SChar: + return 1; + case BuiltinType::Char_U: + return 1; + case BuiltinType::Char_S: + return 1; + case BuiltinType::UShort: + return 2; + case BuiltinType::Short: + return 2; + case BuiltinType::UInt: + return 3; + case BuiltinType::Int: + return 3; + case BuiltinType::ULong: + return 4; + case BuiltinType::Long: + return 4; + case BuiltinType::ULongLong: + return 5; + case BuiltinType::LongLong: + return 5; + case BuiltinType::UInt128: + return 6; + case BuiltinType::Int128: + return 6; + default: + return 0; + } +} + +static int relativeCharSizes(BuiltinType::Kind Kind) { + switch (Kind) { + case BuiltinType::UChar: + return 1; + case BuiltinType::SChar: + return 1; + case BuiltinType::Char_U: + return 1; + case BuiltinType::Char_S: + return 1; + case BuiltinType::Char16: + return 2; + case BuiltinType::Char32: + return 3; + default: + return 0; + } +} + +static int relativeCharSizesW(BuiltinType::Kind Kind) { + switch (Kind) { + case BuiltinType::UChar: + return 1; + case BuiltinType::SChar: + return 1; + case BuiltinType::Char_U: + return 1; + case BuiltinType::Char_S: + return 1; + case BuiltinType::WChar_U: + return 2; + case BuiltinType::WChar_S: + return 2; + default: + return 0; + } +} + +static bool isFirstWider(BuiltinType::Kind First, BuiltinType::Kind Second) { + int FirstSize, SecondSize; + if ((FirstSize = relativeIntSizes(First)) != 0 && + (SecondSize = relativeIntSizes(Second)) != 0) + return FirstSize > SecondSize; + if ((FirstSize = relativeCharSizes(First)) != 0 && + (SecondSize = relativeCharSizes(Second)) != 0) + return FirstSize > SecondSize; + if ((FirstSize = relativeCharSizesW(First)) != 0 && + (SecondSize = relativeCharSizesW(Second)) != 0) + return FirstSize > SecondSize; + return false; +} + +void MisplacedWideningCastCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Cast = Result.Nodes.getNodeAs<CastExpr>("Cast"); + if (!CheckImplicitCasts && isa<ImplicitCastExpr>(Cast)) + return; + if (Cast->getLocStart().isMacroID()) + return; + + const auto *Calc = Result.Nodes.getNodeAs<Expr>("Calc"); + if (Calc->getLocStart().isMacroID()) + return; + + if (Cast->isTypeDependent() || Cast->isValueDependent() || + Calc->isTypeDependent() || Calc->isValueDependent()) + return; + + ASTContext &Context = *Result.Context; + + QualType CastType = Cast->getType(); + QualType CalcType = Calc->getType(); + + // Explicit truncation using cast. + if (Context.getIntWidth(CastType) < Context.getIntWidth(CalcType)) + return; + + // If CalcType and CastType have same size then there is no real danger, but + // there can be a portability problem. + + if (Context.getIntWidth(CastType) == Context.getIntWidth(CalcType)) { + const auto *CastBuiltinType = + dyn_cast<BuiltinType>(CastType->getUnqualifiedDesugaredType()); + const auto *CalcBuiltinType = + dyn_cast<BuiltinType>(CalcType->getUnqualifiedDesugaredType()); + if (CastBuiltinType && CalcBuiltinType && + !isFirstWider(CastBuiltinType->getKind(), CalcBuiltinType->getKind())) + return; + } + + // Don't write a warning if we can easily see that the result is not + // truncated. + if (Context.getIntWidth(CalcType) >= getMaxCalculationWidth(Context, Calc)) + return; + + diag(Cast->getLocStart(), "either cast from %0 to %1 is ineffective, or " + "there is loss of precision before the conversion") + << CalcType << CastType; +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang diff --git a/clang-tidy/bugprone/MisplacedWideningCastCheck.h b/clang-tidy/bugprone/MisplacedWideningCastCheck.h new file mode 100644 index 00000000..b61556fd --- /dev/null +++ b/clang-tidy/bugprone/MisplacedWideningCastCheck.h @@ -0,0 +1,46 @@ +//===--- MisplacedWideningCastCheck.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_MISPLACEDWIDENINGCASTCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MISPLACEDWIDENINGCASTCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Find casts of calculation results to bigger type. Typically from int to +/// long. If the intention of the cast is to avoid loss of precision then +/// the cast is misplaced, and there can be loss of precision. Otherwise +/// such cast is ineffective. +/// +/// There is one option: +/// +/// - `CheckImplicitCasts`: Whether to check implicit casts as well which may +// be the most common case. Enabled by default. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-misplaced-widening-cast.html +class MisplacedWideningCastCheck : public ClangTidyCheck { +public: + MisplacedWideningCastCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const bool CheckImplicitCasts; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif |