veluca93 updated this revision to Diff 427007.
veluca93 marked 7 inline comments as done.
veluca93 added a comment.
Address first round of review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124918/new/
https://reviews.llvm.org/D124918
Files:
clang-tools-extra/clang-tidy/performance/CMakeLists.txt
clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.cpp
clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp
@@ -0,0 +1,118 @@
+// RUN: %check_clang_tidy %s performance-unused-no-side-effect %t -- \
+// RUN: -config="{CheckOptions: [{key: \"performance-unused-no-side-effect.TypesByBase\", value: '{\"Marker\":[\"swap\"]}'}]}"
+
+namespace std {
+
+template <class CharT>
+struct char_traits {};
+
+template <class CharT>
+struct allocator {};
+
+template <
+ class CharT,
+ class Traits = std::char_traits<CharT>,
+ class Allocator = std::allocator<CharT>>
+class basic_string {
+public:
+ basic_string &operator+=(const char *);
+ ~basic_string();
+};
+
+using string = basic_string<char>;
+
+class thread {
+public:
+ ~thread();
+};
+
+template <typename T, typename Allocator = std::allocator<T>>
+class vector {
+public:
+ void push_back(T);
+};
+
+} // namespace std
+
+struct Marker {
+};
+
+struct SideEffectFree : Marker {
+ SideEffectFree();
+ ~SideEffectFree();
+ void swap(SideEffectFree *);
+};
+
+struct POD {
+ int A;
+};
+
+void f(const std::string &);
+
+void unusedString() {
+ std::string SomeString;
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'SomeString' is never read [performance-unused-no-side-effect]
+}
+
+void unusedStringWithMethodCall() {
+ std::string SomeString;
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'SomeString' is never read [performance-unused-no-side-effect]
+ SomeString += "hi";
+}
+
+void unusedStdThread() {
+ std::thread T;
+}
+
+void usedString() {
+ std::string SomeString;
+ SomeString += "hi";
+ f(SomeString);
+}
+
+void vectorString() {
+ std::vector<std::string> VecString;
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: variable 'VecString' is never read [performance-unused-no-side-effect]
+ VecString.push_back(std::string{});
+}
+
+void vectorThread() {
+ std::vector<std::thread> VecThread;
+ VecThread.push_back(std::thread{});
+}
+
+void withMarker() {
+ SideEffectFree M;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: variable 'M' is never read [performance-unused-no-side-effect]
+}
+
+void withMarkerAndExcludedMethod(SideEffectFree *S) {
+ SideEffectFree M;
+ M.swap(S);
+}
+
+void withPOD() {
+ POD P;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'P' is never read [performance-unused-no-side-effect]
+ P.A = 1;
+}
+
+int withPODReturn() {
+ POD P;
+ P.A = 1;
+ return P.A;
+}
+
+int main() {
+ std::vector<int> VecLoops;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: variable 'VecLoops' is never read [performance-unused-no-side-effect]
+ for (int i = 0; i < 15; i++) {
+ VecLoops.push_back(1);
+ }
+ int j = 0;
+ while (j < 1) {
+ VecLoops.push_back(2);
+ j++;
+ }
+ (VecLoops).push_back(3);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
@@ -0,0 +1,193 @@
+.. title:: clang-tidy - performance-unused-no-side-effect
+
+performance-unused-no-side-effect
+=================================
+
+Finds variables that are not used and do not have user-visible side effects.
+
+A variable is defined to be side-effect-free and unused if:
+
+- It has a side-effect-free type (defined below).
+- It is not passed to a function that reads its content and whose result is used.
+
+A type is side-effect-free if it is any of the following:
+
+- A type with a base class specified in `TypesByBase`.
+- A type listed in `RawTypes`.
+- A template type listed in `TemplateTypes` whose template arguments are
+ themselves side-effect-free types.
+- A smart pointer (listed in `SmartPointerTypes`) whose pointee is of
+ side-effect-free type.
+- A primitive type.
+- A pointer.
+- A struct/class with no user-defined destructor and members of trivial type.
+
+A function is considered to "not read an argument unless the return value is
+used" in the following cases:
+
+- It is a method of the explicitly listed types and the argument is ``this``,
+ except:
+
+ - The exceptions associated with the type (eg. ``std::vector::swap``).
+ - A constructor of a smart pointer type that takes a non-newly-allocated
+ pointer as argument.
+
+- A function listed in FunctionAllowList, if the argument is in index ``i`` and
+ bit ``i`` is set in the corresponding allow list entry.
+
+Examples of snippets that trigger the check with the default configuration:
+
+.. code-block:: c++
+
+ void f() {
+ std::vector<int> vec;
+ vec.push_back(1);
+ }
+
+
+Examples that do *not* trigger the check:
+
+.. code-block:: c++
+
+ void f() {
+ std::vector<int> vec;
+ vec.push_back(1);
+ std::cout << vec[0];
+ }
+
+ void g(std::vector<int>& outp) {
+ std::vector<int> vec;
+ vec.push_back(1);
+ vec.swap(outp);
+ }
+
+
+
+Configuration
+-------------
+The types that are considered unused can be specified in the check
+configuration. As more than just a simple list of types is used for
+configuration, the configuration options are JSON.
+
+`TypesByBase`
+^^^^^^^^^^^^^
+Types in this group will cause any type that derives from them to be considered
+side effect free. All methods on such types will also be considered side effect
+free, except for the exceptions explicitly listed as the value of the mapping.
+
+The default value for this configuration option is
+
+.. code-block:: json
+
+ {"proto2::Message": ["Swap", "SerializeToString"]}
+
+`RawTypes`
+^^^^^^^^^^
+Similar to `TypesByBase`, except it only applies to the types themselves.
+
+The default value for this configuration option is
+
+.. code-block:: json
+
+ {
+ "std::string": ["swap"],
+ "absl::int128": [],
+ "absl::uint128": [],
+ "absl::Time": ["swap"],
+ "absl::Duration": ["swap"],
+ "absl::TimeZone": ["swap"],
+ "absl::AlphaNum": [],
+ "absl::Cord": ["swap", "Swap", "CopyTo", "AppendTo", "CopyToArray",
+ "DumpTreeStructure", "Dump", "operator="],
+ "absl::string_view": ["swap", "copy"],
+ "std::string_view": ["swap", "copy"]
+ }
+
+`TemplateTypes`
+^^^^^^^^^^^^^^^
+Similar to `TypesByBase`, except it only applies to specializations of the
+specified template with side-effect-free types.
+
+The default value for this configuration option is
+
+.. code-block:: json
+
+ {
+ "std::vector": ["swap"],
+ "std::pair": [],
+ "std::tuple": [],
+ "std::optional": ["swap"],
+ "std::variant": ["swap"],
+ "std::list": ["swap"],
+ "std::function": ["operator()", "operator=", "swap"],
+ "absl::FunctionRef": ["operator()", "operator=", "swap"],
+ "absl::node_hash_set": ["swap"],
+ "absl::node_hash_map": ["swap"],
+ "absl::flat_hash_set": ["swap"],
+ "absl::flat_hash_map": ["swap"],
+ "absl::container_internal::hash_default_hash": [],
+ "absl::container_internal::hash_default_eq": [],
+ "absl::Hash": [],
+ "absl::hash_internal::Hash": [],
+ "absl::StatusOr": ["swap"],
+ "std::set": ["swap"],
+ "std::map": ["swap"],
+ "std::multiset": ["swap"],
+ "std::multimap": ["swap"],
+ "std::unordered_set": ["swap"],
+ "std::unordered_map": ["swap"],
+ "std::unordered_multiset": ["swap"],
+ "std::unordered_multimap": ["swap"],
+ "absl::btree_set": ["swap"],
+ "absl::btree_map": ["swap"],
+ "absl::btree_multiset": ["swap"],
+ "absl::btree_multimap": ["swap"],
+ "std::array": [],
+ "std::deque": ["swap"],
+ "std::queue": ["swap"],
+ "std::priority_queue": ["swap"],
+ "std::basic_string": ["swap"],
+ "std::allocator": ["deallocate"],
+ "std::char_traits": []
+ }
+
+`SmartPointerTypes`
+^^^^^^^^^^^^^^^^^^^
+Similar to `TemplateTypes`, except constructors are handled in a special way:
+only constructors that take a newly allocated raw pointer are considered
+side-effect free.
+
+The default value for this configuration option is
+
+.. code-block:: json
+
+ {
+ "std::unique_ptr": ["swap", "operator=", "reset"],
+ "std::shared_ptr": ["swap", "operator=", "reset"]
+ }
+
+`FunctionAllowList`
+^^^^^^^^^^^^^^^^^^^
+List of functions that are considered not to read some of their arguments
+unless their return value is read. Arguments are identified by a bitmask, where
+the i-th LSB being set indicates that the i-th argument is unused.
+
+The default value for this configuration option is
+
+.. code-block:: json
+
+ {
+ "absl::StringPrintf": 4294967295,
+ "absl::StrAppend": 1,
+ "absl::StrContains": 4294967295,
+ "absl::CordContains": 4294967295,
+ "absl::StrSplit": 4294967295,
+ "absl::StrCat": 4294967295,
+ "absl::CordSplit": 4294967295,
+ "absl::Substitute": 4294967295,
+ "absl::CordJoin": 4294967295,
+ "absl::CordCat": 4294967295,
+ "absl::CordAppend": 1,
+ "absl::SubstituteAndAppend": 1,
+ "absl::StrJoin": 4294967295
+ }
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -84,7 +84,7 @@
`bugprone-posix-return <bugprone-posix-return.html>`_, "Yes"
`bugprone-redundant-branch-condition <bugprone-redundant-branch-condition.html>`_, "Yes"
`bugprone-reserved-identifier <bugprone-reserved-identifier.html>`_, "Yes"
- `bugprone-shared-ptr-array-mismatch <bugprone-shared-ptr-array-mismatch.html>`_, "Yes"
+ `bugprone-shared-ptr-array-mismatch <bugprone-shared-ptr-array-mismatch.html>`_,
`bugprone-signal-handler <bugprone-signal-handler.html>`_,
`bugprone-signed-char-misuse <bugprone-signed-char-misuse.html>`_,
`bugprone-sizeof-container <bugprone-sizeof-container.html>`_,
@@ -113,7 +113,7 @@
`bugprone-unused-return-value <bugprone-unused-return-value.html>`_,
`bugprone-use-after-move <bugprone-use-after-move.html>`_,
`bugprone-virtual-near-miss <bugprone-virtual-near-miss.html>`_, "Yes"
- `cert-dcl21-cpp <cert-dcl21-cpp.html>`_, "Yes"
+ `cert-dcl21-cpp <cert-dcl21-cpp.html>`_,
`cert-dcl50-cpp <cert-dcl50-cpp.html>`_,
`cert-dcl58-cpp <cert-dcl58-cpp.html>`_,
`cert-env33-c <cert-env33-c.html>`_,
@@ -287,6 +287,7 @@
`performance-type-promotion-in-math-fn <performance-type-promotion-in-math-fn.html>`_, "Yes"
`performance-unnecessary-copy-initialization <performance-unnecessary-copy-initialization.html>`_, "Yes"
`performance-unnecessary-value-param <performance-unnecessary-value-param.html>`_, "Yes"
+ `performance-unused-no-side-effect <performance-unused-no-side-effect.html>`_, "Yes"
`portability-restrict-system-includes <portability-restrict-system-includes.html>`_, "Yes"
`portability-simd-intrinsics <portability-simd-intrinsics.html>`_,
`portability-std-allocator-const <portability-std-allocator-const.html>`_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -118,6 +118,11 @@
Replaces groups of adjacent macros with an unscoped anonymous enum.
+- New :doc:`performance-unused-no-side-effect
+ <clang-tidy/checks/performance-unused-no-side-effect>` check.
+
+ Finds variables that are not used and do not have user-visible side effects.
+
- New :doc:`portability-std-allocator-const <clang-tidy/checks/portability-std-allocator-const>` check.
Report use of ``std::vector<const T>`` (and similar containers of const
Index: clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.h
@@ -0,0 +1,66 @@
+//===--- UnusedNoSideEffectCheck.h - clang-tidy -----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNUSEDNOSIDEEFFECTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNUSEDNOSIDEEFFECTCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Type.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/StringSet.h"
+
+#include <string>
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+/// Finds variables that are not used and do not have user-visible side effects.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance-unused-no-side-effect.html
+class UnusedNoSideEffectCheck : public ClangTidyCheck {
+public:
+ UnusedNoSideEffectCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+ struct NoSideEffectsInfo {
+ std::string Type;
+ llvm::SmallVector<std::string> ExcludedMethods;
+ };
+
+private:
+ class Visitor;
+
+ bool hasNoSideEffects(ASTContext &AstContext, QualType Type,
+ const CXXMethodDecl *Method = nullptr);
+
+ bool hasNoSideEffectsImpl(ASTContext &AstContext, QualType Type,
+ const CXXMethodDecl *Method, bool AllowReferences,
+ llvm::DenseSet<QualType> &Seen);
+
+ llvm::SmallVector<NoSideEffectsInfo> TypesByBase;
+ llvm::SmallVector<NoSideEffectsInfo> RawTypes;
+ llvm::SmallVector<NoSideEffectsInfo> TemplateTypes;
+ llvm::SmallVector<NoSideEffectsInfo> SmartPointerTypes;
+ // Map of function name to bitmask of arguments that are not read from.
+ llvm::StringMap<uint32_t> FunctionAllowList;
+ // TODO(veluca): The FixIts don't really fix everything and can break code.
+ static constexpr bool EmitFixits = false;
+};
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNUSEDNOSIDEEFFECTCHECK_H
Index: clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.cpp
@@ -0,0 +1,675 @@
+//===--- UnusedNoSideEffectCheck.cpp - clang-tidy -------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "UnusedNoSideEffectCheck.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attrs.inc"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/Type.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersInternal.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/Specifiers.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include <map>
+#include <sstream>
+#include <string>
+#include <utility>
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+using namespace ::clang::ast_matchers; // NOLINT: Too many names.
+
+class UnusedNoSideEffectCheck::Visitor
+ : public RecursiveASTVisitor<UnusedNoSideEffectCheck::Visitor> {
+public:
+ explicit Visitor(UnusedNoSideEffectCheck *Check) : Check(Check) {}
+
+ bool shouldVisitImplicitCode() { return true; }
+ bool shouldVisitTemplateInstantiations() const { return true; }
+
+ bool dataTraverseStmtPre(Stmt *S) {
+ if (CurrentExprOrDeclStmt != nullptr) {
+ return true;
+ }
+ if (isa<DeclStmt, Expr>(S)) {
+ CurrentExprOrDeclStmt = S;
+ }
+ return true;
+ }
+
+ bool dataTraverseStmtPost(Stmt *S) {
+ if (CurrentExprOrDeclStmt == S) {
+ CurrentExprOrDeclStmt = nullptr;
+ }
+ return true;
+ }
+
+ bool VisitStmt(Stmt *S) {
+ // Mark all children as observable by default.
+ for (const auto *Child : S->children()) {
+ markObservable(Child);
+ }
+ return true;
+ }
+
+ bool VisitCompoundStmt(CompoundStmt *Compound) {
+ // Compound statements (i.e. blocks) do not produce a value, nor have side
+ // effects by themselves.
+ for (const auto *Child : Compound->children()) {
+ markSideEffectFree(Child);
+ }
+ return true;
+ }
+
+ bool VisitBinaryOperator(BinaryOperator *Op) {
+ if (isObservable(Op)) {
+ return true;
+ }
+ // If this is not an assignment operation, its arguments are only observed
+ // iff the result of the operation is observed. If this is an assignment,
+ // then its RHS is always observed.
+ if (!Op->isAssignmentOp()) {
+ markSideEffectFree(Op->getRHS());
+ }
+ markSideEffectFree(Op->getLHS());
+ return true;
+ }
+
+ bool VisitUnaryOperator(UnaryOperator *Op) {
+ if (isObservable(Op) || Op->getOpcode() == UnaryOperatorKind::UO_Coawait ||
+ Op->getOpcode() == UnaryOperatorKind::UO_Deref) {
+ return true;
+ }
+ // Unary operators don't have side effects, except co_await and
+ // dereferencing.
+ markSideEffectFree(Op->getSubExpr());
+ return true;
+ }
+
+ bool VisitCallExpr(CallExpr *Call) {
+ if (isObservable(Call)) {
+ return true;
+ }
+ const auto *Callee = Call->getDirectCallee();
+ if (!Callee) {
+ return true;
+ }
+ CalleesForStmts[CurrentExprOrDeclStmt].insert(Callee);
+ const auto *CXXMethod = dyn_cast<CXXMethodDecl>(Callee);
+ if (!CXXMethod || CXXMethod->isStatic()) {
+ std::string QualName = Callee->getQualifiedNameAsString();
+ const auto Iter = Check->FunctionAllowList.find(QualName);
+ if (Iter != Check->FunctionAllowList.end()) {
+ for (size_t i = 0; i < Call->getNumArgs(); i++) {
+ if ((1u << i) & Iter->second) {
+ markSideEffectFree(Call->getArg(i));
+ }
+ }
+ }
+ return true;
+ }
+ if (!Check->hasNoSideEffects(CXXMethod->getASTContext(),
+ CXXMethod->getThisObjectType(), CXXMethod)) {
+ return true;
+ }
+ // Here we know that the function is a method or operator call on a struct
+ // or class, and that there are no side effects on `this`.
+ if (auto *Callee = dyn_cast<MemberExpr>(Call->getCallee())) {
+ markSideEffectFree(Callee);
+ }
+ if (auto *CXXMethodCall = dyn_cast<CXXMemberCallExpr>(Call)) {
+ markSideEffectFree(CXXMethodCall->getImplicitObjectArgument());
+ return true;
+ }
+ // Otherwise, this is a call with operator syntax, and "this" is the first
+ // argument.
+ assert(isa<CXXOperatorCallExpr>(Call));
+ markSideEffectFree(Call->getArg(0));
+ return true;
+ }
+
+ bool VisitMemberExpr(MemberExpr *MemberExpr) {
+ if (isObservable(MemberExpr)) {
+ return true;
+ }
+ // Member access does not have side effects.
+ markSideEffectFree(MemberExpr->getBase());
+ return true;
+ }
+
+ bool VisitDeclRefExpr(DeclRefExpr *DeclRef) {
+ const auto *Decl = dyn_cast<VarDecl>(DeclRef->getDecl());
+ if (!Decl) {
+ return true;
+ }
+ StmtsForVars[Decl].insert(CurrentExprOrDeclStmt);
+ // Also consider used all variables that are used in a non-ODR context, i.e.
+ // array sizes, variables that appear in decltype expressions, or variables
+ // that appear as template parameters.
+ if (!isObservable(DeclRef) && !DeclRef->isNonOdrUse()) {
+ return true;
+ }
+ UsedDecls.insert(Decl);
+ return true;
+ }
+
+ bool VisitExprWithCleanups(ExprWithCleanups *ExprWithCleanups) {
+ if (isObservable(ExprWithCleanups)) {
+ return true;
+ }
+ markSideEffectFree(ExprWithCleanups->getSubExpr());
+ return true;
+ }
+
+ bool VisitImplicitCastExpr(ImplicitCastExpr *ImplicitCast) {
+ if (isObservable(ImplicitCast) ||
+ ImplicitCast->getCastKind() == CastKind::CK_LValueToRValue) {
+ return true;
+ }
+ // Implicit casts don't have side effects, except lvalue-to-rvalue.
+ markSideEffectFree(ImplicitCast->getSubExpr());
+ return true;
+ }
+ bool VisitParenExpr(ParenExpr *Paren) {
+ if (isObservable(Paren)) {
+ return true;
+ }
+ markSideEffectFree(Paren->getSubExpr());
+ return true;
+ }
+ bool VisitDeclStmt(DeclStmt *Decl) {
+ // We do not support producing fixits for variables in multiple
+ // declarations.
+ if (Decl->isSingleDecl() && isa<VarDecl>(Decl->getSingleDecl())) {
+ StmtsForVars[dyn_cast<VarDecl>(Decl->getSingleDecl())].insert(Decl);
+ }
+ return true;
+ }
+
+ // Utility methods to track observability of statements. By default, all
+ // statements are observable, and statements are marked as not observable when
+ // they are part of non-observable statements of a specific kind; for example,
+ // direct members of a CompoundStmt are not observable.
+ void markObservable(const Stmt *Stmt) { ObservableStmts.insert(Stmt); }
+ void markSideEffectFree(const Stmt *Stmt) { ObservableStmts.erase(Stmt); }
+ bool isObservable(const Stmt *Stmt) const {
+ return ObservableStmts.contains(Stmt);
+ }
+
+ UnusedNoSideEffectCheck *Check;
+ llvm::DenseSet<const Stmt *> ObservableStmts;
+ llvm::DenseSet<const VarDecl *> UsedDecls;
+ // Statements in which a given variable is used/declared. Used for fixits.
+ llvm::DenseMap<const VarDecl *, llvm::DenseSet<const Stmt *>> StmtsForVars;
+ llvm::DenseMap<const Stmt *, llvm::DenseSet<const FunctionDecl *>>
+ CalleesForStmts;
+ Stmt *CurrentExprOrDeclStmt = nullptr;
+};
+
+void UnusedNoSideEffectCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(functionDecl(hasBody(compoundStmt())).bind("func"), this);
+}
+
+void UnusedNoSideEffectCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>("func");
+
+ Visitor Vis(this);
+ if (const auto *CXXConstructor = dyn_cast<CXXConstructorDecl>(MatchedDecl)) {
+ for (const auto *Init : CXXConstructor->inits()) {
+ Vis.markObservable(Init->getInit());
+ }
+ }
+ Vis.TraverseFunctionDecl(const_cast<FunctionDecl *>(MatchedDecl));
+
+ for (const auto &KV : Vis.StmtsForVars) {
+ const auto *VarDecl = KV.first;
+ if (Vis.UsedDecls.contains(VarDecl))
+ continue;
+
+ // Common ways to mark variables as unused intentionally.
+ if (VarDecl->getName().empty())
+ continue;
+ if (VarDecl->getName().startswith("_"))
+ continue;
+ if (VarDecl->getName().startswith("unused_"))
+ continue;
+ if (VarDecl->hasAttr<UnusedAttr>())
+ continue;
+
+ // Unused function parameters are very common.
+ // TODO(veluca): consider also handling unused `ParmVarDecl`s of non-cheap
+ // types.
+ if (isa<ParmVarDecl>(VarDecl)) {
+ continue;
+ }
+
+ // Skip variables of types that are not known to be side effect free.
+ if (!hasNoSideEffects(VarDecl->getASTContext(), VarDecl->getType())) {
+ continue;
+ }
+
+ // Handle smart pointer constructors.
+ if (const auto *RecordDecl = VarDecl->getType()->getAsRecordDecl()) {
+ std::string Name = RecordDecl->getQualifiedNameAsString().c_str();
+ bool IsSmartPointer = false;
+ for (const auto &TypeInfo : SmartPointerTypes) {
+ if (Name == TypeInfo.Type) {
+ IsSmartPointer = true;
+ }
+ }
+
+ // TODO(veluca): consider also handling custom deleters.
+
+ // Detect some constructors.
+ if (IsSmartPointer && VarDecl->hasInit()) {
+ const Expr *Init = VarDecl->getInit();
+ if (isa<ExprWithCleanups>(Init)) {
+ Init = dyn_cast<ExprWithCleanups>(Init)->getSubExpr();
+ }
+ const auto *CXXConstruct = dyn_cast<CXXConstructExpr>(Init);
+ if (!CXXConstruct || CXXConstruct->getNumArgs() > 1) {
+ // We don't understand this constructor: conservatively do nothing.
+ continue;
+ }
+ // Bail out on constructors that take a pointer that is not
+ // immediately produced by a new expression.
+ if (CXXConstruct->getNumArgs() == 1 &&
+ CXXConstruct->getArg(0)->getType()->isPointerType() &&
+ !isa<CXXNewExpr>(CXXConstruct->getArg(0))) {
+ continue;
+ }
+ }
+ }
+
+ // Avoid reporting captured variables that are not used in the body of a
+ // lambda, or global/static variables.
+ if (!MatchedDecl->getSourceRange().fullyContains(
+ VarDecl->getSourceRange())) {
+ continue;
+ }
+
+ llvm::DenseSet<const FunctionDecl *> Callees;
+ {
+ auto Diag = diag(VarDecl->getLocation(), "variable %0 is never read")
+ << VarDecl;
+ if (EmitFixits) {
+ for (const auto *StmtToDelete : KV.second) {
+ // TODO(veluca): semicolons.
+ Diag << FixItHint::CreateRemoval(StmtToDelete->getSourceRange());
+ for (const auto *FnDecl : Vis.CalleesForStmts[StmtToDelete]) {
+ Callees.insert(FnDecl);
+ }
+ }
+ }
+ }
+ }
+}
+
+bool fromJSON(
+ const llvm::json::Value &E,
+ llvm::SmallVector<UnusedNoSideEffectCheck::NoSideEffectsInfo> &Out,
+ llvm::json::Path P) {
+ if (const auto *O = E.getAsObject()) {
+ Out.clear();
+ for (const auto &KV : *O) {
+ UnusedNoSideEffectCheck::NoSideEffectsInfo Entry;
+ Entry.Type = KV.first.str();
+ if (const auto *A = KV.second.getAsArray()) {
+ for (const auto &Method : *A) {
+ llvm::Optional<llvm::StringRef> M = Method.getAsString();
+ if (M) {
+ Entry.ExcludedMethods.push_back(M.getValue().str());
+ } else {
+ P.field(KV.first).report("expected array of strings");
+ }
+ }
+ } else {
+ P.field(KV.first).report("expected array");
+ return false;
+ }
+ Out.push_back(std::move(Entry));
+ }
+ return true;
+ }
+ P.report("expected object");
+ return false;
+}
+
+UnusedNoSideEffectCheck::UnusedNoSideEffectCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {
+ // Operator= is disabled by default because often used to force deallocations
+ constexpr const char *kDefaultSmartPointerTypes =
+ R"({"std::unique_ptr":["swap","operator=","reset"],"std::shared_ptr":["swap","operator=", "reset"]})";
+ constexpr const char *kDefaultRawTypes = R"(
+ {
+ "std::string": ["swap"],
+ "absl::int128": [],
+ "absl::uint128": [],
+ "absl::Time": ["swap"],
+ "absl::Duration": ["swap"],
+ "absl::TimeZone": ["swap"],
+ "absl::AlphaNum": [],
+ "absl::Cord": ["swap", "Swap", "CopyTo", "AppendTo", "CopyToArray",
+ "DumpTreeStructure", "Dump", "operator="],
+ "absl::string_view": ["swap", "copy"],
+ "std::string_view": ["swap", "copy"]
+ }
+)";
+ constexpr const char *kDefaultTypesByBase =
+ R"({"proto2::Message":["Swap","SerializeToString"]})";
+
+ constexpr const char *kDefaultTemplateTypes = R"r(
+ {
+ "std::vector": ["swap"],
+ "std::pair": [],
+ "std::tuple": [],
+ "std::optional": ["swap"],
+ "std::variant": ["swap"],
+ "std::list": ["swap"],
+ "std::function": ["operator()", "operator=", "swap"],
+ "absl::FunctionRef": ["operator()", "operator=", "swap"],
+ "absl::node_hash_set": ["swap"],
+ "absl::node_hash_map": ["swap"],
+ "absl::flat_hash_set": ["swap"],
+ "absl::flat_hash_map": ["swap"],
+ "absl::container_internal::hash_default_hash": [],
+ "absl::container_internal::hash_default_eq": [],
+ "absl::Hash": [],
+ "absl::hash_internal::Hash": [],
+ "absl::StatusOr": ["swap"],
+ "std::set": ["swap"],
+ "std::map": ["swap"],
+ "std::multiset": ["swap"],
+ "std::multimap": ["swap"],
+ "std::unordered_set": ["swap"],
+ "std::unordered_map": ["swap"],
+ "std::unordered_multiset": ["swap"],
+ "std::unordered_multimap": ["swap"],
+ "absl::btree_set": ["swap"],
+ "absl::btree_map": ["swap"],
+ "absl::btree_multiset": ["swap"],
+ "absl::btree_multimap": ["swap"],
+ "std::array": [],
+ "std::deque": ["swap"],
+ "std::queue": ["swap"],
+ "std::priority_queue": ["swap"],
+ "std::basic_string": ["swap"],
+ "std::allocator": ["deallocate"],
+ "std::char_traits": []
+ }
+)r";
+
+ static constexpr const char *kDefaultFunctionAllowList = R"(
+ {
+ "absl::StringPrintf": 4294967295,
+ "absl::StrAppend": 1,
+ "absl::StrContains": 4294967295,
+ "absl::CordContains": 4294967295,
+ "absl::StrSplit": 4294967295,
+ "absl::StrCat": 4294967295,
+ "absl::CordSplit": 4294967295,
+ "absl::Substitute": 4294967295,
+ "absl::CordJoin": 4294967295,
+ "absl::CordCat": 4294967295,
+ "absl::CordAppend": 1,
+ "absl::SubstituteAndAppend": 1,
+ "absl::StrJoin": 4294967295
+ }
+)";
+
+ auto FunctionAllowListOrError =
+ llvm::json::parse<std::map<std::string, int64_t>>(
+ Options.get("FunctionAllowList", kDefaultFunctionAllowList),
+ "FunctionAllowList");
+ if (FunctionAllowListOrError) {
+ for (const auto &kv : FunctionAllowListOrError.get()) {
+ FunctionAllowList[kv.first] = kv.second;
+ }
+ }
+
+ if (auto TypesByBaseOrError =
+ llvm::json::parse<llvm::SmallVector<NoSideEffectsInfo>>(
+ Options.get("TypesByBase", kDefaultTypesByBase), "TypesByBase")) {
+ TypesByBase = TypesByBaseOrError.get();
+ }
+
+ if (auto RawTypesOrError =
+ llvm::json::parse<llvm::SmallVector<NoSideEffectsInfo>>(
+ Options.get("RawTypes", kDefaultRawTypes), "RawTypes")) {
+ RawTypes = RawTypesOrError.get();
+ }
+
+ if (auto SmartPointerTypesOrError =
+ llvm::json::parse<llvm::SmallVector<NoSideEffectsInfo>>(
+ Options.get("SmartPointerTypes", kDefaultSmartPointerTypes),
+ "SmartPointerTypes")) {
+ SmartPointerTypes = SmartPointerTypesOrError.get();
+ }
+
+ if (auto TemplateTypesOrError =
+ llvm::json::parse<llvm::SmallVector<NoSideEffectsInfo>>(
+ Options.get("TemplateTypes", kDefaultTemplateTypes),
+ "TemplateTypes")) {
+ TemplateTypes = TemplateTypesOrError.get();
+ }
+}
+
+void UnusedNoSideEffectCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ std::string output;
+ llvm::raw_string_ostream OS(output);
+ {
+ llvm::json::OStream J(OS);
+ J.object([&] {
+ for (const auto &kv : FunctionAllowList) {
+ J.attribute(kv.first(), kv.second);
+ }
+ });
+ Options.store(Opts, "FunctionAllowList", output);
+ }
+
+ auto SerializeTypeInfo =
+ [&](const llvm::SmallVector<NoSideEffectsInfo> &Types, const char *Name) {
+ llvm::json::OStream J(OS);
+ output.clear();
+ J.object([&] {
+ for (const auto &Info : Types) {
+ J.attributeArray(Info.Type, [&] {
+ for (const auto &Excl : Info.ExcludedMethods) {
+ J.value(Excl);
+ }
+ });
+ }
+ });
+ Options.store(Opts, Name, output);
+ };
+ SerializeTypeInfo(TypesByBase, "TypesByBase");
+ SerializeTypeInfo(RawTypes, "RawTypes");
+ SerializeTypeInfo(SmartPointerTypes, "SmartPointerTypes");
+ SerializeTypeInfo(TemplateTypes, "TemplateTypes");
+}
+
+bool UnusedNoSideEffectCheck::hasNoSideEffects(ASTContext &AstContext,
+ QualType Type,
+ const CXXMethodDecl *Method) {
+ llvm::DenseSet<QualType> Seen;
+ return hasNoSideEffectsImpl(AstContext, Type, Method,
+ /*AllowReferences=*/false, Seen);
+}
+
+bool UnusedNoSideEffectCheck::hasNoSideEffectsImpl(
+ ASTContext &AstContext, QualType Type, const CXXMethodDecl *Method,
+ bool AllowReferences, llvm::DenseSet<QualType> &Seen) {
+ if (Type.isNull()) {
+ return true;
+ }
+ Type = Type.getCanonicalType();
+ // With recursive types (i.e. lists or trees), recursively visiting member
+ // variables would lead to infinite loops. OTOH, such members cannot make a
+ // type have side effects unless other members do so already, so we can just
+ // return true here.
+ if (Seen.count(Type)) {
+ return true;
+ }
+ Seen.insert(Type);
+ if (Type->isBuiltinType()) {
+ return true;
+ }
+ if (Type->isFunctionPointerType() || Type->isFunctionType()) {
+ return true;
+ }
+ if (Type->isReferenceType()) {
+ return AllowReferences;
+ }
+ if (Type->isPointerType()) {
+ return true;
+ }
+
+ // Check known types.
+ const auto *Record = Type->getAs<RecordType>();
+ if (Record) {
+ const auto *RecordDecl = Record->getDecl();
+ const llvm::SmallVector<std::string> *ExcludedList = nullptr;
+
+ {
+ std::string RecordDeclName = RecordDecl->getQualifiedNameAsString();
+ for (const auto &TypeInfo : RawTypes) {
+ if (RecordDeclName == TypeInfo.Type) {
+ ExcludedList = &TypeInfo.ExcludedMethods;
+ }
+ }
+ }
+
+ const auto *CXXRecord = dyn_cast<CXXRecordDecl>(RecordDecl);
+
+ if (CXXRecord && CXXRecord->hasDefinition()) {
+ CXXRecord = CXXRecord->getDefinition();
+ for (const auto &Base : CXXRecord->bases()) {
+ const auto *BaseType = Base.getType()->getAs<RecordType>();
+ if (!BaseType)
+ continue;
+ std::string BaseTypeName =
+ BaseType->getDecl()->getQualifiedNameAsString();
+ for (const auto &TypeInfo : TypesByBase) {
+ if (BaseTypeName == TypeInfo.Type) {
+ ExcludedList = &TypeInfo.ExcludedMethods;
+ }
+ }
+ }
+ }
+
+ const auto *CXXTemplateSpecialization =
+ dyn_cast<ClassTemplateSpecializationDecl>(RecordDecl);
+ if (CXXTemplateSpecialization) {
+ for (const auto TmplArg :
+ CXXTemplateSpecialization->getTemplateArgs().asArray()) {
+ if (TmplArg.getKind() == TemplateArgument::ArgKind::Type &&
+ !hasNoSideEffectsImpl(AstContext, TmplArg.getAsType(),
+ /*Method=*/nullptr, AllowReferences, Seen)) {
+ return false;
+ }
+ if (TmplArg.getKind() == TemplateArgument::ArgKind::Pack) {
+ for (auto TArg : TmplArg.getPackAsArray()) {
+ if (TArg.getKind() == TemplateArgument::ArgKind::Type &&
+ !hasNoSideEffectsImpl(AstContext, TArg.getAsType(),
+ /*Method=*/nullptr, AllowReferences,
+ Seen)) {
+ return false;
+ }
+ }
+ }
+ }
+ const auto TemplateName =
+ CXXTemplateSpecialization->getSpecializedTemplate()
+ ->getTemplatedDecl()
+ ->getQualifiedNameAsString();
+ for (const auto &TypeInfo : RawTypes) {
+ if (TemplateName == TypeInfo.Type) {
+ ExcludedList = &TypeInfo.ExcludedMethods;
+ }
+ }
+ for (const auto &TypeInfo : TemplateTypes) {
+ if (TemplateName == TypeInfo.Type) {
+ ExcludedList = &TypeInfo.ExcludedMethods;
+ }
+ }
+ for (const auto &TypeInfo : SmartPointerTypes) {
+ if (TemplateName == TypeInfo.Type) {
+ ExcludedList = &TypeInfo.ExcludedMethods;
+ }
+ }
+ }
+
+ // Unknown type.
+ if (ExcludedList == nullptr) {
+ // Always return false if we are calling a method.
+ if (Method != nullptr) {
+ return false;
+ }
+ // If the type is POD we return true.
+ if (Type.isPODType(AstContext)) {
+ return true;
+ }
+ // If we don't know enough about this type, conservatively return false.
+ if (!CXXRecord || !CXXRecord->hasDefinition()) {
+ return false;
+ }
+ // Otherwise, return true iff all struct members and bases are
+ // side-effect-free themselves, and there is no user-defined destructor.
+ if (CXXRecord->hasUserDeclaredDestructor()) {
+ return false;
+ }
+ for (const auto &Base : CXXRecord->bases()) {
+ if (!hasNoSideEffectsImpl(AstContext, Base.getType(),
+ /*Method=*/nullptr, AllowReferences, Seen))
+ return false;
+ }
+ for (const auto *Field : CXXRecord->fields()) {
+ if (!hasNoSideEffectsImpl(AstContext, Field->getType(),
+ /*Method=*/nullptr,
+ AllowReferences ||
+ Field->getAccess() !=
+ AccessSpecifier::AS_public,
+ Seen))
+ return false;
+ }
+ return true;
+ }
+
+ if (Method == nullptr) {
+ return true;
+ }
+
+ std::string MethodName = Method->getNameAsString();
+ for (const auto &Excl : *ExcludedList) {
+ if (Excl == MethodName) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ return false;
+}
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
+++ clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
@@ -24,6 +24,7 @@
#include "TypePromotionInMathFnCheck.h"
#include "UnnecessaryCopyInitialization.h"
#include "UnnecessaryValueParamCheck.h"
+#include "UnusedNoSideEffectCheck.h"
namespace clang {
namespace tidy {
@@ -61,6 +62,8 @@
"performance-unnecessary-copy-initialization");
CheckFactories.registerCheck<UnnecessaryValueParamCheck>(
"performance-unnecessary-value-param");
+ CheckFactories.registerCheck<UnusedNoSideEffectCheck>(
+ "performance-unused-no-side-effect");
}
};
Index: clang-tools-extra/clang-tidy/performance/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/performance/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/performance/CMakeLists.txt
@@ -20,6 +20,7 @@
TypePromotionInMathFnCheck.cpp
UnnecessaryCopyInitialization.cpp
UnnecessaryValueParamCheck.cpp
+ UnusedNoSideEffectCheck.cpp
LINK_LIBS
clangTidy
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
