diff options
12 files changed, 393 insertions, 1 deletions
diff --git a/clang-tools-extra/clang-tidy/CMakeLists.txt b/clang-tools-extra/clang-tidy/CMakeLists.txt index af106091192..a870eb5d6e3 100644 --- a/clang-tools-extra/clang-tidy/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/CMakeLists.txt @@ -32,5 +32,6 @@ add_subdirectory(cppcoreguidelines) add_subdirectory(google) add_subdirectory(misc) add_subdirectory(modernize) +add_subdirectory(performance) add_subdirectory(readability) add_subdirectory(utils) diff --git a/clang-tools-extra/clang-tidy/Makefile b/clang-tools-extra/clang-tidy/Makefile index 9c26da2723d..c6509ac04e7 100644 --- a/clang-tools-extra/clang-tidy/Makefile +++ b/clang-tools-extra/clang-tidy/Makefile @@ -11,6 +11,6 @@ CLANG_LEVEL := ../../.. LIBRARYNAME := clangTidy include $(CLANG_LEVEL)/../../Makefile.config -DIRS = utils cert cppcoreguidelines readability llvm google misc modernize tool +DIRS = utils cert cppcoreguidelines readability llvm google misc modernize performance tool include $(CLANG_LEVEL)/Makefile diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt new file mode 100644 index 00000000000..97225781356 --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt @@ -0,0 +1,14 @@ +set(LLVM_LINK_COMPONENTS support) + +add_clang_library(clangTidyPerformanceModule + PerformanceTidyModule.cpp + UnnecessaryCopyInitialization.cpp + + LINK_LIBS + clangAST + clangASTMatchers + clangBasic + clangLex + clangTidy + clangTidyUtils + ) diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp new file mode 100644 index 00000000000..ab0dbc8012a --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp @@ -0,0 +1,39 @@ +//===--- PeformanceTidyModule.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 "../ClangTidy.h" +#include "../ClangTidyModule.h" +#include "../ClangTidyModuleRegistry.h" + +#include "UnnecessaryCopyInitialization.h" + +namespace clang { +namespace tidy { +namespace performance { + +class PerformanceModule : public ClangTidyModule { +public: + void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck<UnnecessaryCopyInitialization>( + "performance-unnecessary-copy-initialization"); + } +}; + +// Register the PerformanceModule using this statically initialized variable. +static ClangTidyModuleRegistry::Add<PerformanceModule> + X("performance-module", "Adds performance checks."); + +} // namespace performance + +// This anchor is used to force the linker to link in the generated object file +// and thus register the PerformanceModule. +volatile int PerformanceModuleAnchorSource = 0; + +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp new file mode 100644 index 00000000000..f1c24f8ca89 --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -0,0 +1,73 @@ +//===--- UnnecessaryCopyInitialization.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 "UnnecessaryCopyInitialization.h" + +#include "../utils/LexerUtils.h" +#include "../utils/Matchers.h" + +namespace clang { +namespace tidy { +namespace performance { + +using namespace ::clang::ast_matchers; + +namespace { +AST_MATCHER(VarDecl, isLocalVarDecl) { return Node.isLocalVarDecl(); } +AST_MATCHER(QualType, isPointerType) { return Node->isPointerType(); } +} // namespace + +void UnnecessaryCopyInitialization::registerMatchers( + ast_matchers::MatchFinder *Finder) { + auto ConstReference = referenceType(pointee(qualType(isConstQualified()))); + auto ConstOrConstReference = + allOf(anyOf(ConstReference, isConstQualified()), + unless(allOf(isPointerType(), unless(pointerType(pointee(qualType( + isConstQualified()))))))); + // Match method call expressions where the this argument is a const + // type or const reference. This returned const reference is highly likely to + // outlive the local const reference of the variable being declared. + // The assumption is that the const reference being returned either points + // to a global static variable or to a member of the called object. + auto ConstRefReturningMethodCallOfConstParam = cxxMemberCallExpr( + callee(cxxMethodDecl(returns(ConstReference))), + on(declRefExpr(to(varDecl(hasType(qualType(ConstOrConstReference))))))); + auto ConstRefReturningFunctionCall = + callExpr(callee(functionDecl(returns(ConstReference))), + unless(callee(cxxMethodDecl()))); + Finder->addMatcher( + varDecl( + isLocalVarDecl(), hasType(isConstQualified()), + hasType(matchers::isExpensiveToCopy()), + hasInitializer(cxxConstructExpr( + hasDeclaration(cxxConstructorDecl(isCopyConstructor())), + hasArgument(0, anyOf(ConstRefReturningFunctionCall, + ConstRefReturningMethodCallOfConstParam))))) + .bind("varDecl"), + this); +} + +void UnnecessaryCopyInitialization::check( + const ast_matchers::MatchFinder::MatchResult &Result) { + const auto *Var = Result.Nodes.getNodeAs<VarDecl>("varDecl"); + SourceLocation AmpLocation = Var->getLocation(); + auto Token = lexer_utils::getPreviousNonCommentToken(*Result.Context, + Var->getLocation()); + if (!Token.is(tok::unknown)) { + AmpLocation = Token.getLocation().getLocWithOffset(Token.getLength()); + } + diag(Var->getLocation(), + "the const qualified variable '%0' is copy-constructed from a " + "const reference; consider making it a const reference") + << Var->getName() << FixItHint::CreateInsertion(AmpLocation, "&"); +} + +} // namespace performance +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h new file mode 100644 index 00000000000..356d4fd3afb --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h @@ -0,0 +1,38 @@ +//===--- UnnecessaryCopyInitialization.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_PERFORMANCE_UNNECESSARY_COPY_INITIALIZATION_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_COPY_INITIALIZATION_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace performance { + +// A check that detects const local variable declarations that are copy +// initialized with the const reference of a function call or the const +// reference of a method call whose object is guaranteed to outlive the +// variable's scope and suggests to use a const reference. +// +// The check currently only understands a subset of variables that are +// guaranteed to outlive the const reference returned, namely: const variables, +// const references, and const pointers to const. +class UnnecessaryCopyInitialization : public ClangTidyCheck { +public: + using ClangTidyCheck::ClangTidyCheck; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace performance +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_COPY_INITIALIZATION_H diff --git a/clang-tools-extra/clang-tidy/tool/CMakeLists.txt b/clang-tools-extra/clang-tidy/tool/CMakeLists.txt index cc58148594d..d66aa9ce9f2 100644 --- a/clang-tools-extra/clang-tidy/tool/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/tool/CMakeLists.txt @@ -16,6 +16,7 @@ target_link_libraries(clang-tidy clangTidyLLVMModule clangTidyMiscModule clangTidyModernizeModule + clangTidyPerformanceModule clangTidyReadabilityModule clangTooling ) diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp index 9dc68244a3e..c5f57b12849 100644 --- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp +++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp @@ -377,6 +377,11 @@ extern volatile int ModernizeModuleAnchorSource; static int LLVM_ATTRIBUTE_UNUSED ModernizeModuleAnchorDestination = ModernizeModuleAnchorSource; +// This anchor is used to force the linker to link the PerformanceModule. +extern volatile int PerformanceModuleAnchorSource; +static int LLVM_ATTRIBUTE_UNUSED PerformanceModuleAnchorDestination = + PerformanceModuleAnchorSource; + // This anchor is used to force the linker to link the ReadabilityModule. extern volatile int ReadabilityModuleAnchorSource; static int LLVM_ATTRIBUTE_UNUSED ReadabilityModuleAnchorDestination = diff --git a/clang-tools-extra/clang-tidy/utils/CMakeLists.txt b/clang-tools-extra/clang-tidy/utils/CMakeLists.txt index 5c64cc90801..9b48b7b20a0 100644 --- a/clang-tools-extra/clang-tidy/utils/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/utils/CMakeLists.txt @@ -4,6 +4,7 @@ add_clang_library(clangTidyUtils HeaderGuard.cpp IncludeInserter.cpp IncludeSorter.cpp + LexerUtils.cpp TypeTraits.cpp LINK_LIBS diff --git a/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp new file mode 100644 index 00000000000..239c0f7da3a --- /dev/null +++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp @@ -0,0 +1,39 @@ +//===--- LexerUtils.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 "LexerUtils.h" + +namespace clang { +namespace tidy { +namespace lexer_utils { + +Token getPreviousNonCommentToken(const ASTContext &Context, + SourceLocation Location) { + const auto &SourceManager = Context.getSourceManager(); + Token Token; + Token.setKind(tok::unknown); + Location = Location.getLocWithOffset(-1); + auto StartOfFile = + SourceManager.getLocForStartOfFile(SourceManager.getFileID(Location)); + while (Location != StartOfFile) { + Location = Lexer::GetBeginningOfToken(Location, SourceManager, + Context.getLangOpts()); + if (!Lexer::getRawToken(Location, Token, SourceManager, + Context.getLangOpts()) && + !Token.is(tok::comment)) { + break; + } + Location = Location.getLocWithOffset(-1); + } + return Token; +} + +} // namespace lexer_utils +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/utils/LexerUtils.h b/clang-tools-extra/clang-tidy/utils/LexerUtils.h new file mode 100644 index 00000000000..0997d2f0477 --- /dev/null +++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.h @@ -0,0 +1,29 @@ +//===--- LexerUtils.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_UTILS_LEXER_UTILS_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_LEXER_UTILS_H + +#include "clang/AST/ASTContext.h" +#include "clang/Lex/Lexer.h" + +namespace clang { +namespace tidy { +namespace lexer_utils { + +// Returns previous non-comment token skipping over any comment text or +// tok::unknown if not found. +Token getPreviousNonCommentToken(const ASTContext &Context, + SourceLocation Location); + +} // namespace lexer_utils +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_LEXER_UTILS_H diff --git a/clang-tools-extra/test/clang-tidy/performance-unnecessary-copy-initialization.cpp b/clang-tools-extra/test/clang-tidy/performance-unnecessary-copy-initialization.cpp new file mode 100644 index 00000000000..7eb7d2fef67 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/performance-unnecessary-copy-initialization.cpp @@ -0,0 +1,152 @@ +// RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t + +struct ExpensiveToCopyType { + ExpensiveToCopyType() {} + virtual ~ExpensiveToCopyType() {} + const ExpensiveToCopyType &reference() const { return *this; } +}; + +struct TrivialToCopyType { + const TrivialToCopyType &reference() const { return *this; } +}; + +const ExpensiveToCopyType &ExpensiveTypeReference() { + static const ExpensiveToCopyType *Type = new ExpensiveToCopyType(); + return *Type; +} + +const TrivialToCopyType &TrivialTypeReference() { + static const TrivialToCopyType *Type = new TrivialToCopyType(); + return *Type; +} + +void PositiveFunctionCall() { + const auto AutoAssigned = ExpensiveTypeReference(); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization] + // CHECK-FIXES: const auto& AutoAssigned = ExpensiveTypeReference(); + const auto AutoCopyConstructed(ExpensiveTypeReference()); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-FIXES: const auto& AutoCopyConstructed(ExpensiveTypeReference()); + const ExpensiveToCopyType VarAssigned = ExpensiveTypeReference(); + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable + // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = ExpensiveTypeReference(); + const ExpensiveToCopyType VarCopyConstructed(ExpensiveTypeReference()); + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable + // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(ExpensiveTypeReference()); +} + +void PositiveMethodCallConstReferenceParam(const ExpensiveToCopyType &Obj) { + const auto AutoAssigned = Obj.reference(); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-FIXES: const auto& AutoAssigned = Obj.reference(); + const auto AutoCopyConstructed(Obj.reference()); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-FIXES: const auto& AutoCopyConstructed(Obj.reference()); + const ExpensiveToCopyType VarAssigned = Obj.reference(); + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable + // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = Obj.reference(); + const ExpensiveToCopyType VarCopyConstructed(Obj.reference()); + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable + // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(Obj.reference()); +} + +void PositiveMethodCallConstParam(const ExpensiveToCopyType Obj) { + const auto AutoAssigned = Obj.reference(); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-FIXES: const auto& AutoAssigned = Obj.reference(); + const auto AutoCopyConstructed(Obj.reference()); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-FIXES: const auto& AutoCopyConstructed(Obj.reference()); + const ExpensiveToCopyType VarAssigned = Obj.reference(); + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable + // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = Obj.reference(); + const ExpensiveToCopyType VarCopyConstructed(Obj.reference()); + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable + // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(Obj.reference()); +} + +void PositiveMethodCallConstPointerParam(const ExpensiveToCopyType *const Obj) { + const auto AutoAssigned = Obj->reference(); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-FIXES: const auto& AutoAssigned = Obj->reference(); + const auto AutoCopyConstructed(Obj->reference()); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-FIXES: const auto& AutoCopyConstructed(Obj->reference()); + const ExpensiveToCopyType VarAssigned = Obj->reference(); + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable + // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = Obj->reference(); + const ExpensiveToCopyType VarCopyConstructed(Obj->reference()); + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable + // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(Obj->reference()); +} + +void PositiveLocalConstValue() { + const ExpensiveToCopyType Obj; + const auto UnnecessaryCopy = Obj.reference(); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-FIXES: const auto& UnnecessaryCopy = Obj.reference(); +} + +void PositiveLocalConstRef() { + const ExpensiveToCopyType Obj; + const ExpensiveToCopyType &ConstReference = Obj.reference(); + const auto UnnecessaryCopy = ConstReference.reference(); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-FIXES: const auto& UnnecessaryCopy = ConstReference.reference(); +} + +void PositiveLocalConstPointer() { + const ExpensiveToCopyType Obj; + const ExpensiveToCopyType *const ConstPointer = &Obj; + const auto UnnecessaryCopy = ConstPointer->reference(); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-FIXES: const auto& UnnecessaryCopy = ConstPointer->reference(); +} + +void NegativeFunctionCallTrivialType() { + const auto AutoAssigned = TrivialTypeReference(); + const auto AutoCopyConstructed(TrivialTypeReference()); + const TrivialToCopyType VarAssigned = TrivialTypeReference(); + const TrivialToCopyType VarCopyConstructed(TrivialTypeReference()); +} + +void NegativeFunctionCallExpensiveTypeNonConstVariable() { + auto AutoAssigned = ExpensiveTypeReference(); + auto AutoCopyConstructed(ExpensiveTypeReference()); + ExpensiveToCopyType VarAssigned = ExpensiveTypeReference(); + ExpensiveToCopyType VarCopyConstructed(ExpensiveTypeReference()); +} + +void NegativeMethodCallNonConstRef(ExpensiveToCopyType &Obj) { + const auto AutoAssigned = Obj.reference(); + const auto AutoCopyConstructed(Obj.reference()); + const ExpensiveToCopyType VarAssigned = Obj.reference(); + const ExpensiveToCopyType VarCopyConstructed(Obj.reference()); +} + +void NegativeMethodCallNonConst(ExpensiveToCopyType Obj) { + const auto AutoAssigned = Obj.reference(); + const auto AutoCopyConstructed(Obj.reference()); + const ExpensiveToCopyType VarAssigned = Obj.reference(); + const ExpensiveToCopyType VarCopyConstructed(Obj.reference()); +} + +void NegativeMethodCallNonConstPointer(ExpensiveToCopyType *const Obj) { + const auto AutoAssigned = Obj->reference(); + const auto AutoCopyConstructed(Obj->reference()); + const ExpensiveToCopyType VarAssigned = Obj->reference(); + const ExpensiveToCopyType VarCopyConstructed(Obj->reference()); +} + +void NegativeObjIsNotParam() { + ExpensiveToCopyType Obj; + const auto AutoAssigned = Obj.reference(); + const auto AutoCopyConstructed(Obj.reference()); + const ExpensiveToCopyType VarAssigned = Obj.reference(); + const ExpensiveToCopyType VarCopyConstructed(Obj.reference()); +} + +struct NegativeConstructor { + NegativeConstructor(const ExpensiveToCopyType &Obj) : Obj(Obj) {} + ExpensiveToCopyType Obj; +}; |
