[PATCH] D152953: [clang-tidy] Introduce fuchsia-global-variables check

[Caslyn Tonelli via Phabricator via cfe-commits]

Caslyn updated this revision to Diff 535845.
Caslyn added a comment.

correct reference in test


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,181 @@
+// 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 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() {
+  return *n

[PATCH] D152953: [clang-tidy] Introduce fuchsia-global-variables check

[Caslyn Tonelli via Phabricator via cfe-commits]

Caslyn added a comment.

Hi Piotr - I'm sorry for the delay in getting back to you. Thank you again for 
your review comments. I did my best trying to get the right combination of 
matchers that limit the candidates and allow the exceptions that we want. I 
wasn't successful in figuring out a way to exempt static references to 
non-trivial destructor classes that don't have the lifetime extension (see 
comment).




Comment at: clang-tools-extra/clang-tidy/fuchsia/GlobalVariablesCheck.cpp:35
+  Finder->addMatcher(
+  varDecl(is_global_or_static_candidate,
+  unless(hasType(cxxRecordDecl(hasAnyName(IgnoreVars)

PiotrZSL wrote:
> only classes can be non trivial to destroy, so we should exclude on this 
> level all types that are not CXXRecordDecl.
In the latest revision I narrowed the matching candidates to: 

```
 anyOf(hasType(hasCanonicalType(references(qualType(,
hasType(arrayType()),
hasType(cxxRecordDecl(hasNonTrivialDestructor(,
```

I found I needed to capture arrays and references in the tests and included 
those in the limited candidates.

However, after a lot of testing and experimenting with 
`materializeTemporaryExpr` and your suggestions, I still couldn't figure out a 
way to allow references without lifetime extensions. For ex, this test gives a 
false positive with the latest patch:

```
  // Static references with function scope are allowed if they don't have
  // lifetime-extension.
  static const NonTriviallyDestructibleClass &ConstRefFuncNtdc = *new 
NonTriviallyDestructibleClass;
```

I've been starting to question if this is a general enough use case to include 
as an exception. Do you think it would be a mistake if this check does not 
allow static references to non-trivial destructors, regardless of a lifetime 
extension?  



Comment at: clang-tools-extra/clang-tidy/fuchsia/GlobalVariablesCheck.cpp:36
+  varDecl(is_global_or_static_candidate,
+  unless(hasType(cxxRecordDecl(hasAnyName(IgnoreVars)
+  .bind("global_var"),

PiotrZSL wrote:
> PiotrZSL wrote:
> > could be better, like in other checks.
> 
I went ahead and combined the two suggestions around the `IgnoreVars` matching .



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/fuchsia/global-variables.cpp:103
+ntdc_ref typedef_ref_ntdc =
+*new NonTriviallyDestructibleClass;
+

PiotrZSL wrote:
> that will act just like alias
> ``NonTriviallyDestructibleClass XYZ;
> typedef_ref_ntdc  = XYZ;``
> this ``new`` here is confusing... both examples should be made simpler.
I got rid of these scenarios and tested typedef to a reference per your 
suggestion in the latest patch - hopefully I captured it correctly:

> typedef for const reference of non trivial type that is used to exend 
> lfietime of variable (calling function that returns object with non trivial 
> destructor).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152953/new/

https://reviews.llvm.org/D152953

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152953: [clang-tidy] Introduce fuchsia-global-variables check

[Caslyn Tonelli via Phabricator via cfe-commits]

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 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 TriviallyDestructi

[PATCH] D152953: [clang-tidy] Introduce fuchsia-global-variables check

[Caslyn Tonelli via Phabricator via cfe-commits]

Caslyn added a comment.

Thanks for the review @PiotrZSL, I’m new to this space and appreciate the 
comments. I have a few questions around some of them and would greatly 
appreciate any guidance you can give.

The intent of this patch is to upstream a generic version of the google style 
check (apologies for the private link) for use on Fuchsia and other projects. 
I’m happy to make this check a misc check that handles initialization and 
destruction if you wouldn’t mind advising a little bit. To handle the 
initialization aspect, would you suggest I combine logic from the existing 
fuchsia-statically-constructed-objects check?

> Please also look into: https://github.com/llvm/llvm-project/issues/62334

I see there’s some potential clean up for the fuchsia module. I can take a stab 
at that ticket after this patch.




Comment at: clang-tools-extra/clang-tidy/fuchsia/GlobalVariablesCheck.cpp:31-32
+unless(allOf(hasType(references(qualType())),
+ unless(hasInitializer(exprWithCleanups(
+ has(materializeTemporaryExpr(;
+

PiotrZSL wrote:
> there can be some implicitCasts here, and it wont match, also 
> CXXBindTemporaryExpr can show up, this isn't a good way...
Is there a better way to implement this intent while handling those corner 
cases?



Comment at: clang-tools-extra/clang-tidy/fuchsia/GlobalVariablesCheck.cpp:38
+  varDecl(is_global_or_static_candidate,
+  unless(hasType(cxxRecordDecl(hasAnyName(IgnoreVars,
+  // Special handling for std::array is treated below

PiotrZSL wrote:
> what about typedefs ?
There is the typedef scenario tested on L#90 the global-variables.cpp test and 
I've added an additional test for typedef to references - are there any other 
test cases I can add to make sure this check handles them correctly?




Comment at: clang-tools-extra/clang-tidy/fuchsia/GlobalVariablesCheck.cpp:96
+ Result.Nodes.getNodeAs("global_var_std_array")) {
+// In the case of std::array, check the type of the first template 
argument.
+// Zero-size arrays are always allowed.

PiotrZSL wrote:
> if you put non trivial type into std::array, then std::array instance will 
> become non-trivial itself, no need to look into arguments.
Done - removed special handling for `std::array`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152953/new/

https://reviews.llvm.org/D152953

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152953: [clang-tidy] Introduce fuchsia-global-variables check

[Caslyn Tonelli via Phabricator via cfe-commits]

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
+struct array {
+  ~array() {};
+  T Element;
+};
+template
+struct array {};
+}  // 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 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), Non

[PATCH] D152953: [clang-tidy] Introduce fuchsia-global-variables check

[Caslyn Tonelli via Phabricator via cfe-commits]

Caslyn created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, abrachet, phosek, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
Caslyn updated this revision to Diff 531489.
Caslyn added a comment.
Caslyn updated this revision to Diff 531932.
Caslyn updated this revision to Diff 532713.
Caslyn added reviewers: ymandel, Prabhuk.
Caslyn published this revision for review.
Herald added projects: LLVM, clang-tools-extra.
Herald added subscribers: cfe-commits, llvm-commits.

Fixes, cleanups.


Caslyn added a comment.

Include exception for variables declared in macros; cleanups.


Caslyn added a comment.

Add constinit exception, remove G3 exceptions


Introduce a clang-tidy check to the fuchsia module to warn of any static
or global variables that do not have trivial destructors.

The implementation is a pared down version of the Google3
check 

 with configurable options to add types for the check to ignore
(e.g. LazyRE2).


Repository:
  rG LLVM Github Monorepo

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,160 @@
+// 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
+struct array {
+  ~array() {};
+  T Element;
+};
+template
+struct array {};
+}  // 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 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: static and global variables are forbidden unless they are trivially destructible [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: static and global variables are forbidden unless they are trivially destructible [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: static and global variables are forbidden unless they are trivially destructible [fuchsia-global-variables]
+
+TriviallyDestructibleClass trivially_destruct