Caslyn updated this revision to Diff 535546.
Caslyn marked 12 inline comments as done.
Caslyn added a comment.
Changes per 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,183 @@
+// 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));
+
+class TriviallyDestructibleClass {
+ public:
+ // This is a trivially destructible class.
+ int I;
+ float F;
+ char C;
+ char Cs[10];
+};
+
+class NonTriviallyDestructibleClass {
+ public:
+ // We need a destructor to make the class non trivially destructible.
+ ~NonTriviallyDestructibleClass() { Var = 0; }
+ int Var;
+};
+
+template <class T>
+class NonTriviallyDestructibleTemplateClass {
+ public:
+ // We need a destructor to make the class non trivially destructible.
+ ~NonTriviallyDestructibleTemplateClass() { Var = 0; }
+ int Var;
+ T Var2;
+};
+
+int GlobalI;
+_Atomic size_t GlobalAtomic;
+
+TriviallyDestructibleClass Tdc;
+
+NonTriviallyDestructibleClass Ntdc;
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+[[clang::no_destroy]] NonTriviallyDestructibleClass NtdcNoDestory;
+
+TriviallyDestructibleClass TdcArray[2] = { TriviallyDestructibleClass(), TriviallyDestructibleClass()};
+
+NonTriviallyDestructibleClass NtdcArray[2] = {
+ NonTriviallyDestructibleClass(), NonTriviallyDestructibleClass()};
+// CHECK-MESSAGES: [[@LINE-2]]:31: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+const TriviallyDestructibleClass TdcConstArray[2] = {
+ TriviallyDestructibleClass(), TriviallyDestructibleClass()};
+
+const NonTriviallyDestructibleClass NtdcConstArray[2] = {
+ NonTriviallyDestructibleClass(), NonTriviallyDestructibleClass()};
+// CHECK-MESSAGES: [[@LINE-2]]:37: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+TriviallyDestructibleClass TdcMultiArray[2][2] = {
+ {TriviallyDestructibleClass(), TriviallyDestructibleClass()},
+ {TriviallyDestructibleClass(), TriviallyDestructibleClass()}};
+
+NonTriviallyDestructibleClass NtdcMultiArray[2][2] = {
+ {NonTriviallyDestructibleClass(), NonTriviallyDestructibleClass()},
+ {NonTriviallyDestructibleClass(), NonTriviallyDestructibleClass()}};
+// CHECK-MESSAGES: [[@LINE-3]]:31: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+const TriviallyDestructibleClass TdcMultiConstArray[2][2] = {
+ {TriviallyDestructibleClass(), TriviallyDestructibleClass()},
+ {TriviallyDestructibleClass(), TriviallyDestructibleClass()}};
+
+const NonTriviallyDestructibleClass NtdcMultiConstArray[2][2] = {
+ {NonTriviallyDestructibleClass(), NonTriviallyDestructibleClass()},
+ {NonTriviallyDestructibleClass(), NonTriviallyDestructibleClass()}};
+// CHECK-MESSAGES: [[@LINE-3]]:37: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+typedef TriviallyDestructibleClass TDCArray[1];
+TDCArray TdcTypedefArray[1];
+
+typedef NonTriviallyDestructibleClass NTDCArray[1];
+NTDCArray NTdcTypedefArray[1];
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+const TriviallyDestructibleClass& getTdcRef() {
+ TriviallyDestructibleClass Tdc;
+ return Tdc;
+}
+
+const NonTriviallyDestructibleClass& getNtdcRef() {
+ NonTriviallyDestructibleClass Ntdc;
+ return Ntdc;
+}
+
+typedef const TriviallyDestructibleClass& TdcConstRef;
+typedef const NonTriviallyDestructibleClass& NtdcConstRef;
+
+void testLifetimeExtension() {
+ static const TriviallyDestructibleClass &Tdc = getTdcRef();
+ static TdcConstRef TdcTypedef = getTdcRef();
+
+ static const NonTriviallyDestructibleClass& Ntdc = getNtdcRef();
+ // CHECK-MESSAGES: [[@LINE-1]]:47: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+ static NtdcConstRef NtdcTypedef = getNtdcRef();
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+ static const NonTriviallyDestructibleClass &ExtendedLifetimeNtdc = {};
+ // CHECK-MESSAGES: [[@LINE-1]]:47: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+ /*
+ // TODO: fix false-positive match
+ // Static references with function scope are allowed if they don't have
+ // lifetime-extension.
+ static const NonTriviallyDestructibleClass &ConstRefFuncNtdc =
+ *new NonTriviallyDestructibleClass;
+ */
+}
+
+/*
+// TODO: fix false-positive match
+static const NonTriviallyDestructibleClass &ConstRefNtdc =
+ *new NonTriviallyDestructibleClass;
+*/
+
+template<typename T>
+const T& f() {
+static T Value;
+// CHECK-MESSAGES: [[@LINE-1]]:10: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+return Value;
+}
+
+void func() {
+ static TriviallyDestructibleClass FuncTdc = f<TriviallyDestructibleClass>();
+
+ static NonTriviallyDestructibleClass FuncNtdc = f<NonTriviallyDestructibleClass>();
+ // CHECK-MESSAGES: [[@LINE-1]]:40: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+ static NonTriviallyDestructibleClass Ntdc;
+ // CHECK-MESSAGES: [[@LINE-1]]:40: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+ static int FuncJ = 0;
+ static TriviallyDestructibleClass Tdc;
+
+ // 'thread_local' variables in functions are allowed without restrictions.
+ thread_local NonTriviallyDestructibleClass TlNtdc;
+}
+
+thread_local NonTriviallyDestructibleClass TlNtdc;
+// CHECK-MESSAGES: [[@LINE-1]]:44: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+class ContainsStaticVars {
+ static NonTriviallyDestructibleClass ClassNtdc;
+ static NonTriviallyDestructibleTemplateClass<int> ClassNtdt;
+ int LocalVar;
+ static int ClassCounter;
+ static TriviallyDestructibleClass ClassTriviallyDestructible;
+};
+
+NonTriviallyDestructibleClass ContainsStaticVars::ClassNtdc;
+// CHECK-MESSAGES: [[@LINE-1]]:51: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+NonTriviallyDestructibleTemplateClass<int> ContainsStaticVars::ClassNtdt;
+// CHECK-MESSAGES: [[@LINE-1]]:64: warning: non trivially destructible static and global variables are forbidden [fuchsia-global-variables]
+
+int ContainsStaticVars::ClassCounter = 42;
+TriviallyDestructibleClass ContainsStaticVars::ClassTriviallyDestructible;
+
+// Test exemptions configured through options
+
+class LazyRE2 {
+ public:
+ LazyRE2();
+};
+
+LazyRE2 Lre2;
+
+template <class Type>
+class ThreadLocal {
+ public:
+ // We need a destructor to make the class non trivially destructible.
+ ~ThreadLocal() {}
+};
+
+ThreadLocal<int> GlobalThreadLocal;
+
+void foo() {
+ static ThreadLocal<int> StaticThreadLocal;
+}
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,45 @@
+.. 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;
+
+`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,96 @@
+//===--- 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"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using llvm::StringRef;
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::fuchsia {
+namespace {
+AST_MATCHER(CXXRecordDecl, hasNonTrivialDestructor) {
+ return Node.hasNonTrivialDestructor();
+}
+} // namespace
+
+GlobalVariablesCheck::GlobalVariablesCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ IgnoreVars(utils::options::parseStringList(Options.get("Ignore", ""))) {}
+
+void GlobalVariablesCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ varDecl(
+ allOf(hasGlobalStorage(), isDefinition()),
+ anyOf(hasType(hasCanonicalType(references(qualType()))),
+ hasType(arrayType()),
+ hasType(cxxRecordDecl(hasNonTrivialDestructor()))),
+ unless(isConstexpr()), unless(isConstinit()),
+ unless(hasAttr(clang::attr::NoDestroy)),
+ unless(hasType(qualType(hasCanonicalType(hasDeclaration(
+ cxxRecordDecl(matchers::matchesAnyListedName(IgnoreVars))))))))
+ .bind("non_trivial_dtor_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 with non-trivial-destructors.
+ if (Decl->getLocation().isMacroID()) {
+ return;
+ }
+
+ // 'thread_local' variables in functions are allowed without restrictions.
+ auto *DeclContext = Decl->getDeclContext();
+ if (!isa<NamespaceDecl>(DeclContext) &&
+ !isa<TranslationUnitDecl>(DeclContext)) {
+ 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>("non_trivial_dtor_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
