[PATCH] D41546: [clang-tidy] Adding Fuchsia checker for statically constructed objects

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Jan 6 09:04:16 PST 2018


aaron.ballman added inline comments.


================
Comment at: clang-tidy/fuchsia/StaticallyConstructedObjectsCheck.cpp:48
+  if (const auto *D = Result.Nodes.getNodeAs<VarDecl>("decl")) {
+    diag(D->getLocStart(), "explicit global static objects are disallowed: if "
+                           "possible, use a constexpr constructor instead "
----------------
Replace the colon with a semicolon.


================
Comment at: clang-tidy/fuchsia/StaticallyConstructedObjectsCheck.cpp:50
+                           "possible, use a constexpr constructor instead "
+                           "[fuchsia-statically-constructed-objects]");
+  }
----------------
You should drop this part of the string (it should be automatically added by the diagnostics engine).


================
Comment at: docs/ReleaseNotes.rst:63
+
+  Warns if global non-trivial objects with explicit static storage are 
+  constructed, unless the object either has a ``constexpr`` constructor or has 
----------------
global non-trivial -> global, non-trivial


================
Comment at: docs/clang-tidy/checks/fuchsia-statically-constructed-objects.rst:6
+
+Warns if global non-trivial objects with explicit static storage are 
+constructed, unless the object either has a ``constexpr`` constructor or has no 
----------------
global non-trivial -> global, non-trivial


================
Comment at: test/clang-tidy/fuchsia-statically-constructed-objects.cpp:26-27
+ClassWithConstexpr C;
+ClassWithConstexpr E(0);
+ClassWithCtor G(0);
+
----------------
This may be expected behavior, but I find it somewhat strange.  This kind of rule is usually in place because you want to prohibit constructors that trigger during program initialization. What is the rule trying to accomplish by disallowing static globals that are declared static while allowing static globals that are not declared with an explicit `static` keyword?

If the rule is trying to prevent constructors from triggering prior to executing main(), what about constexpr constructors that must be evaluated at runtime? e.g.,
```
class S {
  int Val;
public:
  constexpr S(int i) : Val(100 / i) {}
  int getVal() const { return Val; }
};

static S s1(1); // Constructor call is constexpr; s1 does not need to be dynamically initialized
static S s2(0); // Constructor call is not constexpr; s2 must be dynamically initialized

extern int get_i();
static S s3(get_i()); // Constructor call is not constexpr; s3 must be dynamically initialized
```


https://reviews.llvm.org/D41546





More information about the cfe-commits mailing list