[PATCH] D50994: Add a new flag and attributes to control static destructor registration

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 21 10:29:20 PDT 2018


aaron.ballman added a comment.

Some minor nits that can be handled post-commit.



================
Comment at: lib/Sema/SemaDeclAttr.cpp:5921
+static void handleDestroyAttr(Sema &S, Decl *D, const ParsedAttr &A) {
+  if (!isa<VarDecl>(D) || !cast<VarDecl>(D)->hasGlobalStorage()) {
+    S.Diag(D->getLocation(), diag::err_destroy_attr_on_non_static_var)
----------------
There is no need to check `VarDecl` here -- that's already done for you by the subject.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5927-5931
+  if (A.getKind() == ParsedAttr::AT_AlwaysDestroy) {
+    handleSimpleAttributeWithExclusions<AlwaysDestroyAttr, NoDestroyAttr>(S, D, A);
+  } else {
+    handleSimpleAttributeWithExclusions<NoDestroyAttr, AlwaysDestroyAttr>(S, D, A);
+  }
----------------
Elide braces.


================
Comment at: test/SemaCXX/no_destroy.cpp:33-34
+int main() {
+  [[clang::no_destroy]] int p; // expected-error{{no_destroy attribute can only be applied to a variable with static or thread storage duration}}
+  [[clang::always_destroy]] int p2; // expected-error{{always_destroy attribute can only be applied to a variable with static or thread storage duration}}
+  [[clang::no_destroy]] static int p3;
----------------
Please also add tests demonstrating that the attribute does not appertain to things that don't look like variables (like a function or struct declaration) and that it does not accept arguments.


Repository:
  rC Clang

https://reviews.llvm.org/D50994





More information about the cfe-commits mailing list