Caslyn updated this revision to Diff 533832.
Caslyn marked 4 inline comments as done.
Caslyn added a comment.
Fixes from review.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152953/new/
https://reviews.llvm.org/D152953
Files:
clang-tools-extra/clang-tidy/fuchsia/CMakeLists.txt
clang-tools-extra/clang-tidy/fuchsia/FuchsiaTidyModule.cpp
clang-tools-extra/clang-tidy/fuchsia/GlobalVariablesCheck.cpp
clang-tools-extra/clang-tidy/fuchsia/GlobalVariablesCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/fuchsia/global-variables.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/fuchsia/global-variables.cpp
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/fuchsia/BUILD.gn
Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/fuchsia/BUILD.gn
===================================================================
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/fuchsia/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/fuchsia/BUILD.gn
@@ -16,6 +16,7 @@
"DefaultArgumentsDeclarationsCheck.cpp",
"FuchsiaTidyModule.cpp",
"MultipleInheritanceCheck.cpp",
+ "GlobalVariablesCheck.cpp",
"OverloadedOperatorCheck.cpp",
"StaticallyConstructedObjectsCheck.cpp",
"TrailingReturnCheck.cpp",
Index: clang-tools-extra/test/clang-tidy/checkers/fuchsia/global-variables.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/fuchsia/global-variables.cpp
@@ -0,0 +1,168 @@
+// RUN: %check_clang_tidy %s fuchsia-global-variables %t \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: fuchsia-global-variables.Ignore, value: 'LazyRE2;ThreadLocal'}] \
+// RUN: }"
+
+using size_t = decltype(sizeof(int));
+
+namespace std {
+template<class T, size_t N>
+struct array {
+ ~array() {};
+ T Element;
+};
+template<class T>
+struct array<T, 0> {};
+} // namespace std */
+
+class TriviallyDestructibleClass {
+ public:
+ // This is a trivially destructible class.
+ TriviallyDestructibleClass() : I(0) {}
+ TriviallyDestructibleClass(int I) : I(I) {}
+
+ int I;
+ float F;
+ char C;
+ char Cs[10];
+};
+
+class NonTriviallyDestructibleClass {
+ public:
+ NonTriviallyDestructibleClass() : Var(0) {}
+ NonTriviallyDestructibleClass(int Var) : Var(Var) {}
+ // We need a destructor to make the class non trivially destructible.
+ ~NonTriviallyDestructibleClass() {}
+ int Var;
+};
+
+template <class T>
+class NonTriviallyDestructibleTemplateClass {
+ public:
+ // We need a destructor to make the class non trivially destructible.
+ ~NonTriviallyDestructibleTemplateClass() {}
+ int Var;
+ T Var2;
+};
+
+int global_i;
+_Atomic size_t global_atomic;
+
+TriviallyDestructibleClass trivially_destructible_class;
+
+NonTriviallyDestructibleClass non_trivially_destructible;
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+[[clang::no_destroy]] NonTriviallyDestructibleClass non_trivially_destructible_no_destroy;
+
+TriviallyDestructibleClass trivially_destructible_array[2] = {
+ TriviallyDestructibleClass(1), TriviallyDestructibleClass(2)};
+
+NonTriviallyDestructibleClass non_trivially_destructible_array[2] = {
+ NonTriviallyDestructibleClass(1), NonTriviallyDestructibleClass(2)};
+// CHECK-MESSAGES: [[@LINE-2]]:31: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+const TriviallyDestructibleClass trivially_destructible_const_array[2] = {
+ TriviallyDestructibleClass(1), TriviallyDestructibleClass(2)};
+
+const NonTriviallyDestructibleClass non_trivially_destructible_const_array[2] = {
+ NonTriviallyDestructibleClass(1), NonTriviallyDestructibleClass(2)};
+// CHECK-MESSAGES: [[@LINE-2]]:37: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+TriviallyDestructibleClass trivially_destructible_multi_array[2][2] = {
+ {TriviallyDestructibleClass(1), TriviallyDestructibleClass(2)},
+ {TriviallyDestructibleClass(3), TriviallyDestructibleClass(4)}};
+
+NonTriviallyDestructibleClass non_trivially_destructible_multi_array[2][2] = {
+ {NonTriviallyDestructibleClass(1), NonTriviallyDestructibleClass(2)},
+ {NonTriviallyDestructibleClass(3), NonTriviallyDestructibleClass(4)}};
+// CHECK-MESSAGES: [[@LINE-3]]:31: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+const TriviallyDestructibleClass const_trivially_destructible_multi_array[2][2] = {
+ {TriviallyDestructibleClass(1), TriviallyDestructibleClass(2)},
+ {TriviallyDestructibleClass(3), TriviallyDestructibleClass(4)}};
+
+const NonTriviallyDestructibleClass const_non_trivially_destructible_multi_array[2][2] = {
+ {NonTriviallyDestructibleClass(1), NonTriviallyDestructibleClass(2)},
+ {NonTriviallyDestructibleClass(3), NonTriviallyDestructibleClass(4)}};
+// CHECK-MESSAGES: [[@LINE-3]]:37: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+typedef TriviallyDestructibleClass TDCArray[1];
+TDCArray tdc_array[1];
+
+typedef NonTriviallyDestructibleClass NTDCArray[1];
+NTDCArray ntdc_array[1];
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+typedef TriviallyDestructibleClass& tdc_ref;
+tdc_ref typedef_ref_tdc =
+ *new TriviallyDestructibleClass;
+
+typedef NonTriviallyDestructibleClass& ntdc_ref;
+ntdc_ref typedef_ref_ntdc =
+ *new NonTriviallyDestructibleClass;
+
+void func() {
+ static NonTriviallyDestructibleClass func_ntdc;
+ // CHECK-MESSAGES: [[@LINE-1]]:40: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+ static int func_j = 0;
+ static TriviallyDestructibleClass func_tdc;
+
+ // Static references with function scope are allowed if they don't have
+ // lifetime-extension.
+ static const NonTriviallyDestructibleClass &const_ref_func_ntdc =
+ *new NonTriviallyDestructibleClass;
+
+ static const NonTriviallyDestructibleClass
+ &extended_lifetime_const_ref_func_ntdc = {};
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+ // 'thread_local' variables in functions are allowed without restrictions.
+ thread_local NonTriviallyDestructibleClass tl_func_ntdc(1);
+}
+
+thread_local NonTriviallyDestructibleClass tl_ntdc(1);
+// CHECK-MESSAGES: [[@LINE-1]]:44: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+static const NonTriviallyDestructibleClass &const_ref_ntdc =
+ *new NonTriviallyDestructibleClass;
+
+class ContainsStaticVars {
+ static NonTriviallyDestructibleClass class_ntdc;
+ static NonTriviallyDestructibleTemplateClass<int> class_ntdt;
+ int local_var;
+ static int class_counter;
+ static TriviallyDestructibleClass class_trivially_destructible;
+};
+
+NonTriviallyDestructibleClass ContainsStaticVars::class_ntdc;
+// CHECK-MESSAGES: [[@LINE-1]]:51: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+NonTriviallyDestructibleTemplateClass<int> ContainsStaticVars::class_ntdt;
+// CHECK-MESSAGES: [[@LINE-1]]:64: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+int ContainsStaticVars::class_counter = 42;
+TriviallyDestructibleClass ContainsStaticVars::class_trivially_destructible;
+
+// Test exemptions configured through options
+
+class LazyRE2 {
+ public:
+ LazyRE2();
+};
+
+LazyRE2 lre2;
+
+template <class Type>
+class ThreadLocal {
+ public:
+ ThreadLocal() {}
+ // We need a destructor to make the class non trivially destructible.
+ ~ThreadLocal() {}
+};
+
+ThreadLocal<int> global_thread_local;
+
+void foo() {
+ static ThreadLocal<int> static_thread_local;
+}
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
@@ -213,6 +213,7 @@
`cppcoreguidelines-virtual-class-destructor <cppcoreguidelines/virtual-class-destructor.html>`_, "Yes"
`darwin-avoid-spinlock <darwin/avoid-spinlock.html>`_,
`darwin-dispatch-once-nonstatic <darwin/dispatch-once-nonstatic.html>`_, "Yes"
+ `fuchsia-global-variables<fuchsia/global-variables.html>`_,
`fuchsia-default-arguments-calls <fuchsia/default-arguments-calls.html>`_,
`fuchsia-default-arguments-declarations <fuchsia/default-arguments-declarations.html>`_, "Yes"
`fuchsia-multiple-inheritance <fuchsia/multiple-inheritance.html>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/fuchsia/global-variables.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/fuchsia/global-variables.rst
@@ -0,0 +1,48 @@
+.. title:: clang-tidy - fuchsia-global-variables
+
+fuchsia-global-variables
+=============================
+
+Warns of static and global variables with non-trivial destructors.
+
+For example:
+
+.. code-block:: c++
+
+ // No warning
+ class PODClass {
+ public:
+ int Var;
+ float F;
+ char C;
+ char Cs[10];
+ };
+
+ // No warning
+ PODClass pod_class;
+
+ class NonPODClass {
+ public:
+ NonPODClass() : Var(0) {}
+ NonPODClass(int Var) : Var(Var) {}
+ ~NonPODClass() {
+ cout << "Running destructor";
+ }
+ int Var;
+ };
+
+ // Warning
+ NonPODClass non_pod_class;
+
+`std::array` produces a warning only if it contains a member that is not
+trivially destructible.
+
+`thread_local` variables in functions are permitted without restrictions.
+
+Options
+-------
+
+.. option:: Ignore
+
+ Semicolon-separated list of names of types for the check to ignore. Default
+ is empty.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -159,6 +159,11 @@
Warns when an rvalue reference function parameter is never moved within
the function body.
+- New :doc:`fuchsia-global-variables
+ <clang-tidy/checks/fuchsia/global-variables>` check.
+
+ Checks static and global variables that are not trivially destructible.
+
- New :doc:`llvmlibc-inline-function-decl
<clang-tidy/checks/llvmlibc/inline-function-decl>` check.
Index: clang-tools-extra/clang-tidy/fuchsia/GlobalVariablesCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/fuchsia/GlobalVariablesCheck.h
@@ -0,0 +1,35 @@
+//===--- GlobalVariablesCheck.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_FUCHSIA_GLOBAL_VARIABLES_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_GLOBAL_VARIABLES_CHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::fuchsia {
+
+/// GlobalVariables with non-trivial destructors are disallowed
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/fuchsia/global-variables.html
+class GlobalVariablesCheck : public ClangTidyCheck {
+public:
+ GlobalVariablesCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void checkType(const ast_matchers::MatchFinder::MatchResult &Result,
+ const VarDecl *decl, const QualType type);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+ const std::vector<StringRef> IgnoreVars;
+};
+
+} // namespace clang::tidy::fuchsia
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_GLOBAL_VARIABLES_CHECK_H
Index: clang-tools-extra/clang-tidy/fuchsia/GlobalVariablesCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/fuchsia/GlobalVariablesCheck.cpp
@@ -0,0 +1,89 @@
+//===--- GlobalVariablesCheck.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 "GlobalVariablesCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using llvm::StringRef;
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::fuchsia {
+
+GlobalVariablesCheck::GlobalVariablesCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ IgnoreVars(utils::options::parseStringList(Options.get("Ignore", ""))) {}
+
+void GlobalVariablesCheck::registerMatchers(MatchFinder *Finder) {
+ const auto is_global_or_static_candidate =
+ allOf(hasGlobalStorage(), isDefinition(), unless(isConstexpr()),
+ unless(hasAttr(clang::attr::NoDestroy)), unless(isConstinit()),
+ // Exception: references without lifetime-extension.
+ unless(allOf(hasType(references(qualType())),
+ unless(hasInitializer(exprWithCleanups(
+ has(materializeTemporaryExpr())))))));
+
+ Finder->addMatcher(
+ varDecl(is_global_or_static_candidate,
+ unless(hasType(cxxRecordDecl(hasAnyName(IgnoreVars)))))
+ .bind("global_var"),
+ this);
+}
+
+void GlobalVariablesCheck::checkType(const MatchFinder::MatchResult &Result,
+ const VarDecl *decl, QualType type) {
+ if (type.getNonReferenceType().isDestructedType() == QualType::DK_none) {
+ return;
+ }
+
+ // Ignore variables which are declared in macros even if they are static or
+ // global variables of non-POD types.
+ if (decl->getLocation().isMacroID()) {
+ return;
+ }
+
+ // 'thread_local' variables in functions are allowed without restrictions.
+ auto *decl_context = decl->getDeclContext();
+ if (!isa<NamespaceDecl>(decl_context) &&
+ !isa<TranslationUnitDecl>(decl_context)) {
+ if (decl->getTLSKind() == VarDecl::TLS_Dynamic ||
+ decl->getTLSKind() == VarDecl::TLS_Static) {
+ return;
+ }
+ }
+
+ // 'thread_local' variables with no dynamic initialization are allowed
+ // everywhere.
+ if (decl->getTLSKind() == VarDecl::TLS_Static) {
+ return;
+ }
+
+ // Objective-C ARC-managed objects are allowed everywhere.
+ if (type->isObjCLifetimeType()) {
+ return;
+ }
+
+ diag(decl->getLocation(),
+ "non trivially destructible static and global variables are forbidden");
+}
+
+void GlobalVariablesCheck::check(const MatchFinder::MatchResult &Result) {
+ if (const auto *decl = Result.Nodes.getNodeAs<VarDecl>("global_var")) {
+ QualType type = decl->getType().getCanonicalType();
+ checkType(Result, decl, type);
+ }
+}
+
+void GlobalVariablesCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "Ignore",
+ utils::options::serializeStringList(IgnoreVars));
+}
+} // namespace clang::tidy::fuchsia
Index: clang-tools-extra/clang-tidy/fuchsia/FuchsiaTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/fuchsia/FuchsiaTidyModule.cpp
+++ clang-tools-extra/clang-tidy/fuchsia/FuchsiaTidyModule.cpp
@@ -12,6 +12,7 @@
#include "../google/UnnamedNamespaceInHeaderCheck.h"
#include "DefaultArgumentsCallsCheck.h"
#include "DefaultArgumentsDeclarationsCheck.h"
+#include "GlobalVariablesCheck.h"
#include "MultipleInheritanceCheck.h"
#include "OverloadedOperatorCheck.h"
#include "StaticallyConstructedObjectsCheck.h"
@@ -31,6 +32,8 @@
"fuchsia-default-arguments-calls");
CheckFactories.registerCheck<DefaultArgumentsDeclarationsCheck>(
"fuchsia-default-arguments-declarations");
+ CheckFactories.registerCheck<GlobalVariablesCheck>(
+ "fuchsia-global-variables");
CheckFactories.registerCheck<google::build::UnnamedNamespaceInHeaderCheck>(
"fuchsia-header-anon-namespaces");
CheckFactories.registerCheck<MultipleInheritanceCheck>(
Index: clang-tools-extra/clang-tidy/fuchsia/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/fuchsia/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/fuchsia/CMakeLists.txt
@@ -8,6 +8,7 @@
DefaultArgumentsDeclarationsCheck.cpp
FuchsiaTidyModule.cpp
MultipleInheritanceCheck.cpp
+ GlobalVariablesCheck.cpp
OverloadedOperatorCheck.cpp
StaticallyConstructedObjectsCheck.cpp
TrailingReturnCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
