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

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.


================
Comment at: clang/include/clang/AST/Decl.h:1472
 
+  /// Do we need to emit an exit-time destructor for this variable?
+  bool isNoDestroy(const ASTContext &) const;
----------------
This is only valid for static variables, right? It's probably better to document the function name that way, e.g. `isStaticWithNoDestroy`.


================
Comment at: clang/include/clang/Basic/Attr.td:2999
+
+def NoDestroy : InheritableAttr {
+  let Spellings = [Clang<"no_destroy", 0>];
----------------
Ditto for these, I think the names of the variables should clarify that it's only for `static`. Note that I wouldn't rename the attributes themselves! Usage in context is unambiguous, and the docs should be clear. I just think the variable names don't provide the required context.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:3494
+The ``always_destroy`` attribute specifies that a variable with static or thread
+storage duration should have it's exit-time destructor run. This attribute does
+nothing unless clang was invoked with -fno-c++-static-destructors.
----------------
"its"


================
Comment at: clang/include/clang/Basic/AttrDocs.td:3495
+storage duration should have it's exit-time destructor run. This attribute does
+nothing unless clang was invoked with -fno-c++-static-destructors.
+  }];
----------------
"does nothing" is more "is the default".


================
Comment at: clang/include/clang/Basic/AttrDocs.td:3503
+The ``no_destroy`` attribute specifies that a variable with static or thread
+storage duration shouldn't have it's exit-time destructor run. Annotating every
+static or thread duration variable with this attribute is equalivant to invoking
----------------
"its"


================
Comment at: clang/include/clang/Basic/AttrDocs.td:3504
+storage duration shouldn't have it's exit-time destructor run. Annotating every
+static or thread duration variable with this attribute is equalivant to invoking
+clang with -fno-c++-static-destructors.
----------------
"static and thread"


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5931
+    handleSimpleAttributeWithExclusions<NoDestroyAttr, AlwaysDestroyAttr>(S, D, A);
+  }
+}
----------------
This allows a variable with both attributes :)
Looking at the code above it seems like having both attributes is equivalent to no-destroy.


Repository:
  rC Clang

https://reviews.llvm.org/D50994