Index: clang-tidy/bugprone/BugproneTidyModule.cpp =================================================================== --- clang-tidy/bugprone/BugproneTidyModule.cpp +++ 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 @@ "bugprone-fold-init-type"); CheckFactories.registerCheck( "bugprone-forward-declaration-namespace"); + CheckFactories.registerCheck( + "bugprone-forwarding-reference-overload"); CheckFactories.registerCheck( "bugprone-inaccurate-erase"); CheckFactories.registerCheck( "bugprone-incorrect-roundings"); CheckFactories.registerCheck( "bugprone-integer-division"); + CheckFactories.registerCheck( + "bugprone-lambda-function-name"); + CheckFactories.registerCheck( + "bugprone-macro-repeated-side-effects"); CheckFactories.registerCheck( "bugprone-misplaced-operator-in-strlen-in-alloc"); + CheckFactories.registerCheck( + "bugprone-misplaced-widening-cast"); CheckFactories.registerCheck( "bugprone-move-forwarding-reference"); CheckFactories.registerCheck( Index: clang-tidy/bugprone/CMakeLists.txt =================================================================== --- clang-tidy/bugprone/CMakeLists.txt +++ clang-tidy/bugprone/CMakeLists.txt @@ -9,10 +9,14 @@ 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 Index: clang-tidy/bugprone/ForwardingReferenceOverloadCheck.h =================================================================== --- clang-tidy/bugprone/ForwardingReferenceOverloadCheck.h +++ 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 Index: clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp =================================================================== --- clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp +++ 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 + +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>). + if (const auto *Dependent = BaseType->getAs()) { + BaseType = Dependent->getQualifier()->getAsType(); + } + if (!BaseType) + return false; + if (CheckTemplate(BaseType->getAs())) { + return true; // Case: enable_if_t< >. + } else if (const auto *Elaborated = BaseType->getAs()) { + if (const auto *Qualifier = Elaborated->getQualifier()->getAsType()) { + if (CheckTemplate(Qualifier->getAs())) { + return true; // Case: enable_if< >::type. + } + } + } + return false; +} +AST_MATCHER_P(TemplateTypeParmDecl, hasDefaultArgument, + clang::ast_matchers::internal::Matcher, 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("parm-var"); + const auto *TypeParmDecl = + Result.Nodes.getNodeAs("type-parm-decl"); + + // Get the FunctionDecl and FunctionTemplateDecl containing the function + // parameter. + const auto *FuncForParam = dyn_cast(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("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 Index: clang-tidy/bugprone/LambdaFunctionNameCheck.h =================================================================== --- clang-tidy/bugprone/LambdaFunctionNameCheck.h +++ 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; + + 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 Index: clang-tidy/bugprone/LambdaFunctionNameCheck.cpp =================================================================== --- clang-tidy/bugprone/LambdaFunctionNameCheck.cpp +++ 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( + &SuppressMacroExpansions)); +} + +void LambdaFunctionNameCheck::check(const MatchFinder::MatchResult &Result) { + const auto *E = Result.Nodes.getNodeAs("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 Index: clang-tidy/bugprone/MacroRepeatedSideEffectsCheck.h =================================================================== --- clang-tidy/bugprone/MacroRepeatedSideEffectsCheck.h +++ 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 Index: clang-tidy/bugprone/MacroRepeatedSideEffectsCheck.cpp =================================================================== --- clang-tidy/bugprone/MacroRepeatedSideEffectsCheck.cpp +++ 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> 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( + *this, Compiler.getPreprocessor())); +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang Index: clang-tidy/bugprone/MisplacedWideningCastCheck.h =================================================================== --- clang-tidy/bugprone/MisplacedWideningCastCheck.h +++ 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 Index: clang-tidy/bugprone/MisplacedWideningCastCheck.cpp =================================================================== --- clang-tidy/bugprone/MisplacedWideningCastCheck.cpp +++ 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(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(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(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("Cast"); + if (!CheckImplicitCasts && isa(Cast)) + return; + if (Cast->getLocStart().isMacroID()) + return; + + const auto *Calc = Result.Nodes.getNodeAs("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(CastType->getUnqualifiedDesugaredType()); + const auto *CalcBuiltinType = + dyn_cast(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 Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -1,15 +1,11 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyMiscModule - ForwardingReferenceOverloadCheck.cpp - LambdaFunctionNameCheck.cpp MisplacedConstCheck.cpp UnconventionalAssignOperatorCheck.cpp DefinitionsInHeadersCheck.cpp MacroParenthesesCheck.cpp - MacroRepeatedSideEffectsCheck.cpp MiscTidyModule.cpp - MisplacedWideningCastCheck.cpp NewDeleteOverloadsCheck.cpp NonCopyableObjects.cpp RedundantExpressionCheck.cpp Index: clang-tidy/misc/ForwardingReferenceOverloadCheck.h =================================================================== --- clang-tidy/misc/ForwardingReferenceOverloadCheck.h +++ clang-tidy/misc/ForwardingReferenceOverloadCheck.h @@ -1,42 +0,0 @@ -//===--- 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_MISC_FORWARDING_REFERENCE_OVERLOAD_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_FORWARDING_REFERENCE_OVERLOAD_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace misc { - -/// 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/misc-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 misc -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_FORWARDING_REFERENCE_OVERLOAD_H Index: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp =================================================================== --- clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp +++ clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp @@ -1,148 +0,0 @@ -//===--- 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 - -using namespace clang::ast_matchers; - -namespace clang { -namespace tidy { -namespace misc { - -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>). - if (const auto *Dependent = BaseType->getAs()) { - BaseType = Dependent->getQualifier()->getAsType(); - } - if (!BaseType) - return false; - if (CheckTemplate(BaseType->getAs())) { - return true; // Case: enable_if_t< >. - } else if (const auto *Elaborated = BaseType->getAs()) { - if (const auto *Qualifier = Elaborated->getQualifier()->getAsType()) { - if (CheckTemplate(Qualifier->getAs())) { - return true; // Case: enable_if< >::type. - } - } - } - return false; -} -AST_MATCHER_P(TemplateTypeParmDecl, hasDefaultArgument, - clang::ast_matchers::internal::Matcher, 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("parm-var"); - const auto *TypeParmDecl = - Result.Nodes.getNodeAs("type-parm-decl"); - - // Get the FunctionDecl and FunctionTemplateDecl containing the function - // parameter. - const auto *FuncForParam = dyn_cast(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("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 misc -} // namespace tidy -} // namespace clang Index: clang-tidy/misc/LambdaFunctionNameCheck.h =================================================================== --- clang-tidy/misc/LambdaFunctionNameCheck.h +++ clang-tidy/misc/LambdaFunctionNameCheck.h @@ -1,51 +0,0 @@ -//===--- 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_MISC_LAMBDA_FUNCTION_NAME_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_LAMBDA_FUNCTION_NAME_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace misc { - -/// 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/misc-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; - - 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 misc -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_LAMBDA_FUNCTION_NAME_H Index: clang-tidy/misc/LambdaFunctionNameCheck.cpp =================================================================== --- clang-tidy/misc/LambdaFunctionNameCheck.cpp +++ clang-tidy/misc/LambdaFunctionNameCheck.cpp @@ -1,99 +0,0 @@ -//===--- 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 misc { - -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( - &SuppressMacroExpansions)); -} - -void LambdaFunctionNameCheck::check(const MatchFinder::MatchResult &Result) { - const auto *E = Result.Nodes.getNodeAs("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 misc -} // namespace tidy -} // namespace clang Index: clang-tidy/misc/MacroRepeatedSideEffectsCheck.h =================================================================== --- clang-tidy/misc/MacroRepeatedSideEffectsCheck.h +++ clang-tidy/misc/MacroRepeatedSideEffectsCheck.h @@ -1,31 +0,0 @@ -//===--- 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_MISC_MACRO_REPEATED_SIDE_EFFECTS_CHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MACRO_REPEATED_SIDE_EFFECTS_CHECK_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace misc { - -/// 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 misc -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MACRO_REPEATED_SIDE_EFFECTS_CHECK_H Index: clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp =================================================================== --- clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp +++ clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp @@ -1,184 +0,0 @@ -//===--- 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 misc { - -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> 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( - *this, Compiler.getPreprocessor())); -} - -} // namespace misc -} // namespace tidy -} // namespace clang Index: clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tidy/misc/MiscTidyModule.cpp +++ clang-tidy/misc/MiscTidyModule.cpp @@ -11,12 +11,8 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "DefinitionsInHeadersCheck.h" -#include "ForwardingReferenceOverloadCheck.h" -#include "LambdaFunctionNameCheck.h" #include "MacroParenthesesCheck.h" -#include "MacroRepeatedSideEffectsCheck.h" #include "MisplacedConstCheck.h" -#include "MisplacedWideningCastCheck.h" #include "NewDeleteOverloadsCheck.h" #include "NonCopyableObjects.h" #include "RedundantExpressionCheck.h" @@ -46,10 +42,6 @@ class MiscModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { - CheckFactories.registerCheck( - "misc-forwarding-reference-overload"); - CheckFactories.registerCheck( - "misc-lambda-function-name"); CheckFactories.registerCheck("misc-misplaced-const"); CheckFactories.registerCheck( "misc-unconventional-assign-operator"); @@ -57,10 +49,6 @@ "misc-definitions-in-headers"); CheckFactories.registerCheck( "misc-macro-parentheses"); - CheckFactories.registerCheck( - "misc-macro-repeated-side-effects"); - CheckFactories.registerCheck( - "misc-misplaced-widening-cast"); CheckFactories.registerCheck( "misc-new-delete-overloads"); CheckFactories.registerCheck( Index: clang-tidy/misc/MisplacedWideningCastCheck.h =================================================================== --- clang-tidy/misc/MisplacedWideningCastCheck.h +++ clang-tidy/misc/MisplacedWideningCastCheck.h @@ -1,46 +0,0 @@ -//===--- 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_MISC_MISPLACED_WIDENING_CAST_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISPLACED_WIDENING_CAST_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace misc { - -/// 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/misc-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 misc -} // namespace tidy -} // namespace clang - -#endif Index: clang-tidy/misc/MisplacedWideningCastCheck.cpp =================================================================== --- clang-tidy/misc/MisplacedWideningCastCheck.cpp +++ clang-tidy/misc/MisplacedWideningCastCheck.cpp @@ -1,233 +0,0 @@ -//===--- 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 misc { - -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(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(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(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("Cast"); - if (!CheckImplicitCasts && isa(Cast)) - return; - if (Cast->getLocStart().isMacroID()) - return; - - const auto *Calc = Result.Nodes.getNodeAs("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(CastType->getUnqualifiedDesugaredType()); - const auto *CalcBuiltinType = - dyn_cast(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 misc -} // namespace tidy -} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -57,6 +57,18 @@ Improvements to clang-tidy -------------------------- +- The 'misc-misplaced-widening-cast' check was renamed to `bugprone-misplaced-widening-cast + `_ + +- The 'misc-lambda-function-name' check was renamed to `bugprone-lambda-function-name + `_ + +- The 'misc-macro-repeated-side-effects' check was renamed to `bugprone-macro-repeated-side-effects + `_ + +- The 'misc-forwarding-reference-overload' check was renamed to `bugprone-forwarding-reference-overload + `_ + - The 'misc-incorrect-roundings' check was renamed to `bugprone-incorrect-roundings `_ Index: docs/clang-tidy/checks/bugprone-forwarding-reference-overload.rst =================================================================== --- docs/clang-tidy/checks/bugprone-forwarding-reference-overload.rst +++ docs/clang-tidy/checks/bugprone-forwarding-reference-overload.rst @@ -0,0 +1,49 @@ +.. title:: clang-tidy - bugprone-forwarding-reference-overload + +bugprone-forwarding-reference-overload +====================================== + +The check looks for perfect forwarding constructors that can hide copy or move +constructors. If a non const lvalue reference is passed to the constructor, the +forwarding reference parameter will be a better match than the const reference +parameter of the copy constructor, so the perfect forwarding constructor will be +called, which can be confusing. +For detailed description of this issue see: Scott Meyers, Effective Modern C++, +Item 26. + +Consider the following example: + + .. code-block:: c++ + + class Person { + public: + // C1: perfect forwarding ctor + template + explicit Person(T&& n) {} + + // C2: perfect forwarding ctor with parameter default value + template + explicit Person(T&& n, int x = 1) {} + + // C3: perfect forwarding ctor guarded with enable_if + template,void>> + explicit Person(T&& n) {} + + // (possibly compiler generated) copy ctor + Person(const Person& rhs); + }; + +The check warns for constructors C1 and C2, because those can hide copy and move +constructors. We suppress warnings if the copy and the move constructors are both +disabled (deleted or private), because there is nothing the perfect forwarding +constructor could hide in this case. We also suppress warnings for constructors +like C3 that are guarded with an enable_if, assuming the programmer was aware of +the possible hiding. + +Background +---------- + +For deciding whether a constructor is guarded with enable_if, we consider the +default values of the type parameters and the types of the constructor +parameters. If any part of these types is std::enable_if or std::enable_if_t, we +assume the constructor is guarded. Index: docs/clang-tidy/checks/bugprone-lambda-function-name.rst =================================================================== --- docs/clang-tidy/checks/bugprone-lambda-function-name.rst +++ docs/clang-tidy/checks/bugprone-lambda-function-name.rst @@ -0,0 +1,27 @@ +.. title:: clang-tidy - bugprone-lambda-function-name + +bugprone-lambda-function-name +============================= + +Checks for attempts to get the name of a function from within a lambda +expression. The name of a lambda is always something like ``operator()``, which +is almost never what was intended. + +Example: + +.. code-block:: c++ + + void FancyFunction() { + [] { printf("Called from %s\n", __func__); }(); + [] { printf("Now called from %s\n", __FUNCTION__); }(); + } + +Output:: + + Called from operator() + Now called from operator() + +Likely intended output:: + + Called from FancyFunction + Now called from FancyFunction Index: docs/clang-tidy/checks/bugprone-macro-repeated-side-effects.rst =================================================================== --- docs/clang-tidy/checks/bugprone-macro-repeated-side-effects.rst +++ docs/clang-tidy/checks/bugprone-macro-repeated-side-effects.rst @@ -0,0 +1,7 @@ +.. title:: clang-tidy - bugprone-macro-repeated-side-effects + +bugprone-macro-repeated-side-effects +==================================== + + +Checks for repeated argument with side effects in macros. Index: docs/clang-tidy/checks/bugprone-misplaced-widening-cast.rst =================================================================== --- docs/clang-tidy/checks/bugprone-misplaced-widening-cast.rst +++ docs/clang-tidy/checks/bugprone-misplaced-widening-cast.rst @@ -0,0 +1,65 @@ +.. title:: clang-tidy - bugprone-misplaced-widening-cast + +bugprone-misplaced-widening-cast +================================ + +This check will warn when there is a cast of a calculation result to a bigger +type. 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 the cast is +ineffective. + +Example code: + +.. code-block:: c++ + + long f(int x) { + return (long)(x * 1000); + } + +The result ``x * 1000`` is first calculated using ``int`` precision. If the +result exceeds ``int`` precision there is loss of precision. Then the result is +casted to ``long``. + +If there is no loss of precision then the cast can be removed or you can +explicitly cast to ``int`` instead. + +If you want to avoid loss of precision then put the cast in a proper location, +for instance: + +.. code-block:: c++ + + long f(int x) { + return (long)x * 1000; + } + +Implicit casts +-------------- + +Forgetting to place the cast at all is at least as dangerous and at least as +common as misplacing it. If :option:`CheckImplicitCasts` is enabled the check +also detects these cases, for instance: + +.. code-block:: c++ + + long f(int x) { + return x * 1000; + } + +Floating point +-------------- + +Currently warnings are only written for integer conversion. No warning is +written for this code: + +.. code-block:: c++ + + double f(float x) { + return (double)(x * 10.0f); + } + +Options +------- + +.. option:: CheckImplicitCasts + + If non-zero, enables detection of implicit casts. Default is non-zero. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -24,10 +24,14 @@ bugprone-dangling-handle bugprone-fold-init-type bugprone-forward-declaration-namespace + bugprone-forwarding-reference-overload bugprone-inaccurate-erase bugprone-incorrect-roundings bugprone-integer-division + bugprone-lambda-function-name + bugprone-macro-repeated-side-effects bugprone-misplaced-operator-in-strlen-in-alloc + bugprone-misplaced-widening-cast bugprone-move-forwarding-reference bugprone-multiple-statement-macro bugprone-string-constructor @@ -127,12 +131,8 @@ llvm-namespace-comment llvm-twine-local misc-definitions-in-headers - misc-forwarding-reference-overload - misc-lambda-function-name misc-macro-parentheses - misc-macro-repeated-side-effects misc-misplaced-const - misc-misplaced-widening-cast misc-new-delete-overloads misc-non-copyable-objects misc-redundant-expression Index: docs/clang-tidy/checks/misc-forwarding-reference-overload.rst =================================================================== --- docs/clang-tidy/checks/misc-forwarding-reference-overload.rst +++ docs/clang-tidy/checks/misc-forwarding-reference-overload.rst @@ -1,49 +0,0 @@ -.. title:: clang-tidy - misc-forwarding-reference-overload - -misc-forwarding-reference-overload -================================== - -The check looks for perfect forwarding constructors that can hide copy or move -constructors. If a non const lvalue reference is passed to the constructor, the -forwarding reference parameter will be a better match than the const reference -parameter of the copy constructor, so the perfect forwarding constructor will be -called, which can be confusing. -For detailed description of this issue see: Scott Meyers, Effective Modern C++, -Item 26. - -Consider the following example: - - .. code-block:: c++ - - class Person { - public: - // C1: perfect forwarding ctor - template - explicit Person(T&& n) {} - - // C2: perfect forwarding ctor with parameter default value - template - explicit Person(T&& n, int x = 1) {} - - // C3: perfect forwarding ctor guarded with enable_if - template,void>> - explicit Person(T&& n) {} - - // (possibly compiler generated) copy ctor - Person(const Person& rhs); - }; - -The check warns for constructors C1 and C2, because those can hide copy and move -constructors. We suppress warnings if the copy and the move constructors are both -disabled (deleted or private), because there is nothing the perfect forwarding -constructor could hide in this case. We also suppress warnings for constructors -like C3 that are guarded with an enable_if, assuming the programmer was aware of -the possible hiding. - -Background ----------- - -For deciding whether a constructor is guarded with enable_if, we consider the -default values of the type parameters and the types of the constructor -parameters. If any part of these types is std::enable_if or std::enable_if_t, we -assume the constructor is guarded. Index: docs/clang-tidy/checks/misc-lambda-function-name.rst =================================================================== --- docs/clang-tidy/checks/misc-lambda-function-name.rst +++ docs/clang-tidy/checks/misc-lambda-function-name.rst @@ -1,27 +0,0 @@ -.. title:: clang-tidy - misc-lambda-function-name - -misc-lambda-function-name -========================= - -Checks for attempts to get the name of a function from within a lambda -expression. The name of a lambda is always something like ``operator()``, which -is almost never what was intended. - -Example: - -.. code-block:: c++ - - void FancyFunction() { - [] { printf("Called from %s\n", __func__); }(); - [] { printf("Now called from %s\n", __FUNCTION__); }(); - } - -Output:: - - Called from operator() - Now called from operator() - -Likely intended output:: - - Called from FancyFunction - Now called from FancyFunction Index: docs/clang-tidy/checks/misc-macro-repeated-side-effects.rst =================================================================== --- docs/clang-tidy/checks/misc-macro-repeated-side-effects.rst +++ docs/clang-tidy/checks/misc-macro-repeated-side-effects.rst @@ -1,7 +0,0 @@ -.. title:: clang-tidy - misc-macro-repeated-side-effects - -misc-macro-repeated-side-effects -================================ - - -Checks for repeated argument with side effects in macros. Index: docs/clang-tidy/checks/misc-misplaced-widening-cast.rst =================================================================== --- docs/clang-tidy/checks/misc-misplaced-widening-cast.rst +++ docs/clang-tidy/checks/misc-misplaced-widening-cast.rst @@ -1,65 +0,0 @@ -.. title:: clang-tidy - misc-misplaced-widening-cast - -misc-misplaced-widening-cast -============================ - -This check will warn when there is a cast of a calculation result to a bigger -type. 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 the cast is -ineffective. - -Example code: - -.. code-block:: c++ - - long f(int x) { - return (long)(x * 1000); - } - -The result ``x * 1000`` is first calculated using ``int`` precision. If the -result exceeds ``int`` precision there is loss of precision. Then the result is -casted to ``long``. - -If there is no loss of precision then the cast can be removed or you can -explicitly cast to ``int`` instead. - -If you want to avoid loss of precision then put the cast in a proper location, -for instance: - -.. code-block:: c++ - - long f(int x) { - return (long)x * 1000; - } - -Implicit casts --------------- - -Forgetting to place the cast at all is at least as dangerous and at least as -common as misplacing it. If :option:`CheckImplicitCasts` is enabled the check -also detects these cases, for instance: - -.. code-block:: c++ - - long f(int x) { - return x * 1000; - } - -Floating point --------------- - -Currently warnings are only written for integer conversion. No warning is -written for this code: - -.. code-block:: c++ - - double f(float x) { - return (double)(x * 10.0f); - } - -Options -------- - -.. option:: CheckImplicitCasts - - If non-zero, enables detection of implicit casts. Default is non-zero. Index: test/clang-tidy/bugprone-forwarding-reference-overload.cpp =================================================================== --- test/clang-tidy/bugprone-forwarding-reference-overload.cpp +++ test/clang-tidy/bugprone-forwarding-reference-overload.cpp @@ -0,0 +1,145 @@ +// RUN: %check_clang_tidy %s bugprone-forwarding-reference-overload %t -- -- -std=c++14 + +namespace std { +template struct enable_if { typedef T type; }; + +template struct enable_if { typedef T type; }; + +template +using enable_if_t = typename enable_if::type; + +template struct enable_if_nice { typedef T type; }; +} // namespace std + +namespace foo { +template struct enable_if { typedef T type; }; +} // namespace foo + +template constexpr bool just_true = true; + +class Test1 { +public: + template Test1(T &&n); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: constructor accepting a forwarding reference can hide the copy and move constructors [bugprone-forwarding-reference-overload] + + template Test1(T &&n, int i = 5, ...); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: constructor accepting a forwarding reference can hide the copy and move constructors + + template ::type> + Test1(T &&n); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide the copy and move constructors + + template + Test1(T &&n, typename foo::enable_if::type i = 5, ...); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide the copy and move constructors + + Test1(const Test1 &other) {} + // CHECK-MESSAGES: :[[@LINE-1]]:3: note: copy constructor declared here + + Test1(Test1 &other) {} + // CHECK-MESSAGES: :[[@LINE-1]]:3: note: copy constructor declared here + + Test1(Test1 &&other) {} + // CHECK-MESSAGES: :[[@LINE-1]]:3: note: move constructor declared here +}; + +template class Test2 { +public: + // Two parameters without default value, can't act as copy / move constructor. + template Test2(T &&n, V &&m, int i = 5, ...); + + // Guarded with enable_if. + template + Test2(T &&n, int i = 5, + std::enable_if_t a = 5, ...); + + // Guarded with enable_if. + template ::type &> + Test2(T &&n); + + // Guarded with enable_if. + template + Test2(T &&n, typename std::enable_if>::type **a = nullptr); + + // Guarded with enable_if. + template > *&&> + Test2(T &&n, double d = 0.0); + + // Not a forwarding reference parameter. + template Test2(const T &&n); + + // Not a forwarding reference parameter. + Test2(int &&x); + + // Two parameters without default value, can't act as copy / move constructor. + template Test2(T &&n, int x); + + // Not a forwarding reference parameter. + template Test2(U &&n); +}; + +// The copy and move constructors are both disabled. +class Test3 { +public: + template Test3(T &&n); + + template Test3(T &&n, int I = 5, ...); + + Test3(const Test3 &rhs) = delete; + +private: + Test3(Test3 &&rhs); +}; + +// Both the copy and the (compiler generated) move constructors can be hidden. +class Test4 { +public: + template Test4(T &&n); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: constructor accepting a forwarding reference can hide the copy and move constructors + + Test4(const Test4 &rhs); + // CHECK-MESSAGES: :[[@LINE-1]]:3: note: copy constructor declared here +}; + +// Nothing can be hidden, the copy constructor is implicitly deleted. +class Test5 { +public: + template Test5(T &&n); + + Test5(Test5 &&rhs) = delete; +}; + +// Only the move constructor can be hidden. +class Test6 { +public: + template Test6(T &&n); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: constructor accepting a forwarding reference can hide the move constructor + + Test6(Test6 &&rhs); + // CHECK-MESSAGES: :[[@LINE-1]]:3: note: move constructor declared here +private: + Test6(const Test6 &rhs); +}; + +// Do not dereference a null BaseType. +template class result_of; +template class result_of<_Fp(_Args...)> { }; +template using result_of_t = typename result_of<_Tp>::type; + +template struct __overload; +template +struct __overload<_Tp, _Types...> : __overload<_Types...> { + using __overload<_Types...>::operator(); +}; + +template +using __best_match_t = typename result_of_t<__overload<_Types...>(_Tp&&)>::type; + +template +class variant { +public: + template > + constexpr variant(_Arg&& __arg) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: constructor accepting a forwarding reference can hide the copy and move constructors +}; Index: test/clang-tidy/bugprone-lambda-function-name.cpp =================================================================== --- test/clang-tidy/bugprone-lambda-function-name.cpp +++ test/clang-tidy/bugprone-lambda-function-name.cpp @@ -0,0 +1,41 @@ +// RUN: %check_clang_tidy %s bugprone-lambda-function-name %t + +void Foo(const char* a, const char* b, int c) {} + +#define FUNC_MACRO Foo(__func__, "", 0) +#define FUNCTION_MACRO Foo(__FUNCTION__, "", 0) +#define EMBED_IN_ANOTHER_MACRO1 FUNC_MACRO + +void Positives() { + [] { __func__; }(); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name] + [] { __FUNCTION__; }(); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name] + [] { FUNC_MACRO; }(); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name] + [] { FUNCTION_MACRO; }(); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name] + [] { EMBED_IN_ANOTHER_MACRO1; }(); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name] +} + +#define FUNC_MACRO_WITH_FILE_AND_LINE Foo(__func__, __FILE__, __LINE__) +#define FUNCTION_MACRO_WITH_FILE_AND_LINE Foo(__FUNCTION__, __FILE__, __LINE__) +#define EMBED_IN_ANOTHER_MACRO2 FUNC_MACRO_WITH_FILE_AND_LINE + +void Negatives() { + __func__; + __FUNCTION__; + + // __PRETTY_FUNCTION__ should not trigger a warning because its value is + // actually potentially useful. + __PRETTY_FUNCTION__; + [] { __PRETTY_FUNCTION__; }(); + + // Don't warn if __func__/__FUNCTION is used inside a macro that also uses + // __FILE__ and __LINE__, on the assumption that __FILE__ and __LINE__ will + // be useful even if __func__/__FUNCTION__ is not. + [] { FUNC_MACRO_WITH_FILE_AND_LINE; }(); + [] { FUNCTION_MACRO_WITH_FILE_AND_LINE; }(); + [] { EMBED_IN_ANOTHER_MACRO2; }(); +} Index: test/clang-tidy/bugprone-macro-repeated-side-effects.c =================================================================== --- test/clang-tidy/bugprone-macro-repeated-side-effects.c +++ test/clang-tidy/bugprone-macro-repeated-side-effects.c @@ -0,0 +1,106 @@ +// RUN: %check_clang_tidy %s bugprone-macro-repeated-side-effects %t + +#define badA(x,y) ((x)+((x)+(y))+(y)) +void bad(int ret, int a, int b) { + ret = badA(a++, b); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: side effects in the 1st macro argument 'x' are repeated in macro expansion [bugprone-macro-repeated-side-effects] + ret = badA(++a, b); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: side effects in the 1st macro argument 'x' + ret = badA(a--, b); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: side effects in the 1st macro argument 'x' + ret = badA(--a, b); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: side effects in the 1st macro argument 'x' + ret = badA(a, b++); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the 2nd macro argument 'y' + ret = badA(a, ++b); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the 2nd macro argument 'y' + ret = badA(a, b--); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the 2nd macro argument 'y' + ret = badA(a, --b); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the 2nd macro argument 'y' +} + + +#define MIN(A,B) ((A) < (B) ? (A) : (B)) // single ?: +#define LIMIT(X,A,B) ((X) < (A) ? (A) : ((X) > (B) ? (B) : (X))) // two ?: +void question(int x) { + MIN(x++, 12); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: side effects in the 1st macro argument 'A' + MIN(34, x++); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: side effects in the 2nd macro argument 'B' + LIMIT(x++, 0, 100); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: side effects in the 1st macro argument 'X' + LIMIT(20, x++, 100); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: side effects in the 2nd macro argument 'A' + LIMIT(20, 0, x++); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: side effects in the 3rd macro argument 'B' +} + +// False positive: Repeated side effects is intentional. +// It is hard to know when it's done by intention so right now we warn. +#define UNROLL(A) {A A} +void fp1(int i) { + UNROLL({ i++; }); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: side effects in the 1st macro argument 'A' +} + +// Do not produce a false positive on a strchr() macro. Explanation; Currently the '?' +// triggers the test to bail out, because it cannot evaluate __builtin_constant_p(c). +# define strchrs(s, c) \ + (__extension__ (__builtin_constant_p (c) && !__builtin_constant_p (s) \ + && (c) == '\0' \ + ? (char *) __rawmemchr (s, c) \ + : __builtin_strchr (s, c))) +char* __rawmemchr(char* a, char b) { + return a; +} +void pass(char* pstr, char ch) { + strchrs(pstr, ch++); // No error. +} + +// Check large arguments (t=20, u=21). +#define largeA(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, x, y, z) \ + ((a) + (a) + (b) + (b) + (c) + (c) + (d) + (d) + (e) + (e) + (f) + (f) + (g) + (g) + \ + (h) + (h) + (i) + (i) + (j) + (j) + (k) + (k) + (l) + (l) + (m) + (m) + (n) + (n) + \ + (o) + (o) + (p) + (p) + (q) + (q) + (r) + (r) + (s) + (s) + (t) + (t) + (u) + (u) + \ + (v) + (v) + (x) + (x) + (y) + (y) + (z) + (z)) +void large(int a) { + largeA(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, a++, 0, 0, 0, 0, 0, 0); + // CHECK-MESSAGES: :[[@LINE-1]]:64: warning: side effects in the 19th macro argument 's' + largeA(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, a++, 0, 0, 0, 0, 0); + // CHECK-MESSAGES: :[[@LINE-1]]:67: warning: side effects in the 20th macro argument 't' + largeA(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, a++, 0, 0, 0, 0); + // CHECK-MESSAGES: :[[@LINE-1]]:70: warning: side effects in the 21st macro argument 'u' +} + +// Passing macro argument as argument to __builtin_constant_p and macros. +#define builtinbad(x) (__builtin_constant_p(x) + (x) + (x)) +#define builtingood1(x) (__builtin_constant_p(x) + (x)) +#define builtingood2(x) ((__builtin_constant_p(x) && (x)) || (x)) +#define macrobad(x) (builtingood1(x) + (x) + (x)) +#define macrogood(x) (builtingood1(x) + (x)) +void builtins(int ret, int a) { + ret += builtinbad(a++); + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: side effects in the 1st macro argument 'x' + + ret += builtingood1(a++); + ret += builtingood2(a++); + + ret += macrobad(a++); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: side effects in the 1st macro argument 'x' + + ret += macrogood(a++); +} + +// Bail out for conditionals. +#define condB(x,y) if(x) {x=y;} else {x=y + 1;} +void conditionals(int a, int b) +{ + condB(a, b++); +} + +void log(const char *s, int v); +#define LOG(val) log(#val, (val)) +void test_log(int a) { + LOG(a++); +} Index: test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp =================================================================== --- test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp +++ test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp @@ -0,0 +1,64 @@ +// RUN: %check_clang_tidy %s bugprone-misplaced-widening-cast %t -- -config="{CheckOptions: [{key: bugprone-misplaced-widening-cast.CheckImplicitCasts, value: 0}]}" -- + +void func(long arg) {} + +void assign(int a, int b) { + long l; + + l = a * b; + l = (long)(a * b); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' is ineffective, or there is loss of precision before the conversion [bugprone-misplaced-widening-cast] + l = (long)a * b; + + l = a << 8; + l = (long)(a << 8); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' + l = (long)b << 8; + + l = static_cast(a * b); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' +} + +void compare(int a, int b, long c) { + bool l; + + l = a * b == c; + l = c == a * b; + l = (long)(a * b) == c; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' + l = c == (long)(a * b); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' + l = (long)a * b == c; + l = c == (long)a * b; +} + +void init(unsigned int n) { + long l1 = n << 8; + long l2 = (long)(n << 8); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' to 'long' + long l3 = (long)n << 8; +} + +void call(unsigned int n) { + func(n << 8); + func((long)(n << 8)); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long' + func((long)n << 8); +} + +long ret(int a) { + if (a < 0) { + return a * 1000; + } else if (a > 0) { + return (long)(a * 1000); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' + } else { + return (long)a * 1000; + } +} + +// Shall not generate an assert. https://bugs.llvm.org/show_bug.cgi?id=33660 +template class A { + enum Type {}; + static char *m_fn1() { char p = (Type)(&p - m_fn1()); } +}; Index: test/clang-tidy/bugprone-misplaced-widening-cast-implicit-enabled.cpp =================================================================== --- test/clang-tidy/bugprone-misplaced-widening-cast-implicit-enabled.cpp +++ test/clang-tidy/bugprone-misplaced-widening-cast-implicit-enabled.cpp @@ -0,0 +1,101 @@ +// RUN: %check_clang_tidy %s bugprone-misplaced-widening-cast %t -- -config="{CheckOptions: [{key: bugprone-misplaced-widening-cast.CheckImplicitCasts, value: 1}]}" -- + +void func(long arg) {} + +void assign(int a, int b) { + long l; + + l = a * b; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' is ineffective, or there is loss of precision before the conversion [bugprone-misplaced-widening-cast] + l = (long)(a * b); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' + l = (long)a * b; + + l = a << 8; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' + l = (long)(a << 8); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' + l = (long)b << 8; + + l = static_cast(a * b); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' +} + +void compare(int a, int b, long c) { + bool l; + + l = a * b == c; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' + l = c == a * b; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' + l = (long)(a * b) == c; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' + l = c == (long)(a * b); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' + l = (long)a * b == c; + l = c == (long)a * b; +} + +void init(unsigned int n) { + long l1 = n << 8; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' to 'long' + long l2 = (long)(n << 8); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' to 'long' + long l3 = (long)n << 8; +} + +void call(unsigned int n) { + func(n << 8); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long' + func((long)(n << 8)); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long' + func((long)n << 8); +} + +long ret(int a) { + if (a < 0) { + return a * 1000; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' + } else if (a > 0) { + return (long)(a * 1000); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' + } else { + return (long)a * 1000; + } +} + +void dontwarn1(unsigned char a, int i, unsigned char *p) { + long l; + // The result is a 9 bit value, there is no truncation in the implicit cast. + l = (long)(a + 15); + // The result is a 12 bit value, there is no truncation in the implicit cast. + l = (long)(a << 4); + // The result is a 3 bit value, there is no truncation in the implicit cast. + l = (long)((i % 5) + 1); + // The result is a 16 bit value, there is no truncation in the implicit cast. + l = (long)(((*p) << 8) + *(p + 1)); +} + +template struct DontWarn2 { + void assign(T a, T b) { + long l; + l = (long)(a * b); + } +}; +DontWarn2 DW2; + +// Cast is not suspicious when casting macro. +#define A (X<<2) +long macro1(int X) { + return (long)A; +} + +// Don't warn about cast in macro. +#define B(X,Y) (long)(X*Y) +long macro2(int x, int y) { + return B(x,y); +} + +void floatingpoint(float a, float b) { + double d = (double)(a * b); // Currently we don't warn for this. +} Index: test/clang-tidy/misc-forwarding-reference-overload.cpp =================================================================== --- test/clang-tidy/misc-forwarding-reference-overload.cpp +++ test/clang-tidy/misc-forwarding-reference-overload.cpp @@ -1,145 +0,0 @@ -// RUN: %check_clang_tidy %s misc-forwarding-reference-overload %t -- -- -std=c++14 - -namespace std { -template struct enable_if { typedef T type; }; - -template struct enable_if { typedef T type; }; - -template -using enable_if_t = typename enable_if::type; - -template struct enable_if_nice { typedef T type; }; -} // namespace std - -namespace foo { -template struct enable_if { typedef T type; }; -} // namespace foo - -template constexpr bool just_true = true; - -class Test1 { -public: - template Test1(T &&n); - // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: constructor accepting a forwarding reference can hide the copy and move constructors [misc-forwarding-reference-overload] - - template Test1(T &&n, int i = 5, ...); - // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: constructor accepting a forwarding reference can hide the copy and move constructors - - template ::type> - Test1(T &&n); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide the copy and move constructors - - template - Test1(T &&n, typename foo::enable_if::type i = 5, ...); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide the copy and move constructors - - Test1(const Test1 &other) {} - // CHECK-MESSAGES: :[[@LINE-1]]:3: note: copy constructor declared here - - Test1(Test1 &other) {} - // CHECK-MESSAGES: :[[@LINE-1]]:3: note: copy constructor declared here - - Test1(Test1 &&other) {} - // CHECK-MESSAGES: :[[@LINE-1]]:3: note: move constructor declared here -}; - -template class Test2 { -public: - // Two parameters without default value, can't act as copy / move constructor. - template Test2(T &&n, V &&m, int i = 5, ...); - - // Guarded with enable_if. - template - Test2(T &&n, int i = 5, - std::enable_if_t a = 5, ...); - - // Guarded with enable_if. - template ::type &> - Test2(T &&n); - - // Guarded with enable_if. - template - Test2(T &&n, typename std::enable_if>::type **a = nullptr); - - // Guarded with enable_if. - template > *&&> - Test2(T &&n, double d = 0.0); - - // Not a forwarding reference parameter. - template Test2(const T &&n); - - // Not a forwarding reference parameter. - Test2(int &&x); - - // Two parameters without default value, can't act as copy / move constructor. - template Test2(T &&n, int x); - - // Not a forwarding reference parameter. - template Test2(U &&n); -}; - -// The copy and move constructors are both disabled. -class Test3 { -public: - template Test3(T &&n); - - template Test3(T &&n, int I = 5, ...); - - Test3(const Test3 &rhs) = delete; - -private: - Test3(Test3 &&rhs); -}; - -// Both the copy and the (compiler generated) move constructors can be hidden. -class Test4 { -public: - template Test4(T &&n); - // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: constructor accepting a forwarding reference can hide the copy and move constructors - - Test4(const Test4 &rhs); - // CHECK-MESSAGES: :[[@LINE-1]]:3: note: copy constructor declared here -}; - -// Nothing can be hidden, the copy constructor is implicitly deleted. -class Test5 { -public: - template Test5(T &&n); - - Test5(Test5 &&rhs) = delete; -}; - -// Only the move constructor can be hidden. -class Test6 { -public: - template Test6(T &&n); - // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: constructor accepting a forwarding reference can hide the move constructor - - Test6(Test6 &&rhs); - // CHECK-MESSAGES: :[[@LINE-1]]:3: note: move constructor declared here -private: - Test6(const Test6 &rhs); -}; - -// Do not dereference a null BaseType. -template class result_of; -template class result_of<_Fp(_Args...)> { }; -template using result_of_t = typename result_of<_Tp>::type; - -template struct __overload; -template -struct __overload<_Tp, _Types...> : __overload<_Types...> { - using __overload<_Types...>::operator(); -}; - -template -using __best_match_t = typename result_of_t<__overload<_Types...>(_Tp&&)>::type; - -template -class variant { -public: - template > - constexpr variant(_Arg&& __arg) {} - // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: constructor accepting a forwarding reference can hide the copy and move constructors -}; Index: test/clang-tidy/misc-lambda-function-name.cpp =================================================================== --- test/clang-tidy/misc-lambda-function-name.cpp +++ test/clang-tidy/misc-lambda-function-name.cpp @@ -1,41 +0,0 @@ -// RUN: %check_clang_tidy %s misc-lambda-function-name %t - -void Foo(const char* a, const char* b, int c) {} - -#define FUNC_MACRO Foo(__func__, "", 0) -#define FUNCTION_MACRO Foo(__FUNCTION__, "", 0) -#define EMBED_IN_ANOTHER_MACRO1 FUNC_MACRO - -void Positives() { - [] { __func__; }(); - // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [misc-lambda-function-name] - [] { __FUNCTION__; }(); - // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [misc-lambda-function-name] - [] { FUNC_MACRO; }(); - // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [misc-lambda-function-name] - [] { FUNCTION_MACRO; }(); - // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [misc-lambda-function-name] - [] { EMBED_IN_ANOTHER_MACRO1; }(); - // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [misc-lambda-function-name] -} - -#define FUNC_MACRO_WITH_FILE_AND_LINE Foo(__func__, __FILE__, __LINE__) -#define FUNCTION_MACRO_WITH_FILE_AND_LINE Foo(__FUNCTION__, __FILE__, __LINE__) -#define EMBED_IN_ANOTHER_MACRO2 FUNC_MACRO_WITH_FILE_AND_LINE - -void Negatives() { - __func__; - __FUNCTION__; - - // __PRETTY_FUNCTION__ should not trigger a warning because its value is - // actually potentially useful. - __PRETTY_FUNCTION__; - [] { __PRETTY_FUNCTION__; }(); - - // Don't warn if __func__/__FUNCTION is used inside a macro that also uses - // __FILE__ and __LINE__, on the assumption that __FILE__ and __LINE__ will - // be useful even if __func__/__FUNCTION__ is not. - [] { FUNC_MACRO_WITH_FILE_AND_LINE; }(); - [] { FUNCTION_MACRO_WITH_FILE_AND_LINE; }(); - [] { EMBED_IN_ANOTHER_MACRO2; }(); -} Index: test/clang-tidy/misc-macro-repeated-side-effects.c =================================================================== --- test/clang-tidy/misc-macro-repeated-side-effects.c +++ test/clang-tidy/misc-macro-repeated-side-effects.c @@ -1,106 +0,0 @@ -// RUN: %check_clang_tidy %s misc-macro-repeated-side-effects %t - -#define badA(x,y) ((x)+((x)+(y))+(y)) -void bad(int ret, int a, int b) { - ret = badA(a++, b); - // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: side effects in the 1st macro argument 'x' are repeated in macro expansion [misc-macro-repeated-side-effects] - ret = badA(++a, b); - // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: side effects in the 1st macro argument 'x' - ret = badA(a--, b); - // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: side effects in the 1st macro argument 'x' - ret = badA(--a, b); - // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: side effects in the 1st macro argument 'x' - ret = badA(a, b++); - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the 2nd macro argument 'y' - ret = badA(a, ++b); - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the 2nd macro argument 'y' - ret = badA(a, b--); - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the 2nd macro argument 'y' - ret = badA(a, --b); - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the 2nd macro argument 'y' -} - - -#define MIN(A,B) ((A) < (B) ? (A) : (B)) // single ?: -#define LIMIT(X,A,B) ((X) < (A) ? (A) : ((X) > (B) ? (B) : (X))) // two ?: -void question(int x) { - MIN(x++, 12); - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: side effects in the 1st macro argument 'A' - MIN(34, x++); - // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: side effects in the 2nd macro argument 'B' - LIMIT(x++, 0, 100); - // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: side effects in the 1st macro argument 'X' - LIMIT(20, x++, 100); - // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: side effects in the 2nd macro argument 'A' - LIMIT(20, 0, x++); - // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: side effects in the 3rd macro argument 'B' -} - -// False positive: Repeated side effects is intentional. -// It is hard to know when it's done by intention so right now we warn. -#define UNROLL(A) {A A} -void fp1(int i) { - UNROLL({ i++; }); - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: side effects in the 1st macro argument 'A' -} - -// Do not produce a false positive on a strchr() macro. Explanation; Currently the '?' -// triggers the test to bail out, because it cannot evaluate __builtin_constant_p(c). -# define strchrs(s, c) \ - (__extension__ (__builtin_constant_p (c) && !__builtin_constant_p (s) \ - && (c) == '\0' \ - ? (char *) __rawmemchr (s, c) \ - : __builtin_strchr (s, c))) -char* __rawmemchr(char* a, char b) { - return a; -} -void pass(char* pstr, char ch) { - strchrs(pstr, ch++); // No error. -} - -// Check large arguments (t=20, u=21). -#define largeA(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, x, y, z) \ - ((a) + (a) + (b) + (b) + (c) + (c) + (d) + (d) + (e) + (e) + (f) + (f) + (g) + (g) + \ - (h) + (h) + (i) + (i) + (j) + (j) + (k) + (k) + (l) + (l) + (m) + (m) + (n) + (n) + \ - (o) + (o) + (p) + (p) + (q) + (q) + (r) + (r) + (s) + (s) + (t) + (t) + (u) + (u) + \ - (v) + (v) + (x) + (x) + (y) + (y) + (z) + (z)) -void large(int a) { - largeA(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, a++, 0, 0, 0, 0, 0, 0); - // CHECK-MESSAGES: :[[@LINE-1]]:64: warning: side effects in the 19th macro argument 's' - largeA(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, a++, 0, 0, 0, 0, 0); - // CHECK-MESSAGES: :[[@LINE-1]]:67: warning: side effects in the 20th macro argument 't' - largeA(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, a++, 0, 0, 0, 0); - // CHECK-MESSAGES: :[[@LINE-1]]:70: warning: side effects in the 21st macro argument 'u' -} - -// Passing macro argument as argument to __builtin_constant_p and macros. -#define builtinbad(x) (__builtin_constant_p(x) + (x) + (x)) -#define builtingood1(x) (__builtin_constant_p(x) + (x)) -#define builtingood2(x) ((__builtin_constant_p(x) && (x)) || (x)) -#define macrobad(x) (builtingood1(x) + (x) + (x)) -#define macrogood(x) (builtingood1(x) + (x)) -void builtins(int ret, int a) { - ret += builtinbad(a++); - // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: side effects in the 1st macro argument 'x' - - ret += builtingood1(a++); - ret += builtingood2(a++); - - ret += macrobad(a++); - // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: side effects in the 1st macro argument 'x' - - ret += macrogood(a++); -} - -// Bail out for conditionals. -#define condB(x,y) if(x) {x=y;} else {x=y + 1;} -void conditionals(int a, int b) -{ - condB(a, b++); -} - -void log(const char *s, int v); -#define LOG(val) log(#val, (val)) -void test_log(int a) { - LOG(a++); -} Index: test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp =================================================================== --- test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp +++ test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp @@ -1,64 +0,0 @@ -// RUN: %check_clang_tidy %s misc-misplaced-widening-cast %t -- -config="{CheckOptions: [{key: misc-misplaced-widening-cast.CheckImplicitCasts, value: 0}]}" -- - -void func(long arg) {} - -void assign(int a, int b) { - long l; - - l = a * b; - l = (long)(a * b); - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' is ineffective, or there is loss of precision before the conversion [misc-misplaced-widening-cast] - l = (long)a * b; - - l = a << 8; - l = (long)(a << 8); - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' - l = (long)b << 8; - - l = static_cast(a * b); - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' -} - -void compare(int a, int b, long c) { - bool l; - - l = a * b == c; - l = c == a * b; - l = (long)(a * b) == c; - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' - l = c == (long)(a * b); - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' - l = (long)a * b == c; - l = c == (long)a * b; -} - -void init(unsigned int n) { - long l1 = n << 8; - long l2 = (long)(n << 8); - // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' to 'long' - long l3 = (long)n << 8; -} - -void call(unsigned int n) { - func(n << 8); - func((long)(n << 8)); - // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long' - func((long)n << 8); -} - -long ret(int a) { - if (a < 0) { - return a * 1000; - } else if (a > 0) { - return (long)(a * 1000); - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' - } else { - return (long)a * 1000; - } -} - -// Shall not generate an assert. https://bugs.llvm.org/show_bug.cgi?id=33660 -template class A { - enum Type {}; - static char *m_fn1() { char p = (Type)(&p - m_fn1()); } -}; Index: test/clang-tidy/misc-misplaced-widening-cast-implicit-enabled.cpp =================================================================== --- test/clang-tidy/misc-misplaced-widening-cast-implicit-enabled.cpp +++ test/clang-tidy/misc-misplaced-widening-cast-implicit-enabled.cpp @@ -1,101 +0,0 @@ -// RUN: %check_clang_tidy %s misc-misplaced-widening-cast %t -- -config="{CheckOptions: [{key: misc-misplaced-widening-cast.CheckImplicitCasts, value: 1}]}" -- - -void func(long arg) {} - -void assign(int a, int b) { - long l; - - l = a * b; - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' is ineffective, or there is loss of precision before the conversion [misc-misplaced-widening-cast] - l = (long)(a * b); - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' - l = (long)a * b; - - l = a << 8; - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' - l = (long)(a << 8); - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' - l = (long)b << 8; - - l = static_cast(a * b); - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' -} - -void compare(int a, int b, long c) { - bool l; - - l = a * b == c; - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' - l = c == a * b; - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' - l = (long)(a * b) == c; - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' - l = c == (long)(a * b); - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' - l = (long)a * b == c; - l = c == (long)a * b; -} - -void init(unsigned int n) { - long l1 = n << 8; - // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' to 'long' - long l2 = (long)(n << 8); - // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' to 'long' - long l3 = (long)n << 8; -} - -void call(unsigned int n) { - func(n << 8); - // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long' - func((long)(n << 8)); - // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long' - func((long)n << 8); -} - -long ret(int a) { - if (a < 0) { - return a * 1000; - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' - } else if (a > 0) { - return (long)(a * 1000); - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' - } else { - return (long)a * 1000; - } -} - -void dontwarn1(unsigned char a, int i, unsigned char *p) { - long l; - // The result is a 9 bit value, there is no truncation in the implicit cast. - l = (long)(a + 15); - // The result is a 12 bit value, there is no truncation in the implicit cast. - l = (long)(a << 4); - // The result is a 3 bit value, there is no truncation in the implicit cast. - l = (long)((i % 5) + 1); - // The result is a 16 bit value, there is no truncation in the implicit cast. - l = (long)(((*p) << 8) + *(p + 1)); -} - -template struct DontWarn2 { - void assign(T a, T b) { - long l; - l = (long)(a * b); - } -}; -DontWarn2 DW2; - -// Cast is not suspicious when casting macro. -#define A (X<<2) -long macro1(int X) { - return (long)A; -} - -// Don't warn about cast in macro. -#define B(X,Y) (long)(X*Y) -long macro2(int x, int y) { - return B(x,y); -} - -void floatingpoint(float a, float b) { - double d = (double)(a * b); // Currently we don't warn for this. -}