[PATCH] D45578: Add a command line option 'fregister_dtor_with_atexit' to register destructor functions annotated with __attribute__((destructor)) using __cxa_atexit or atexit.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: include/clang/Driver/Options.td:1613
+def fregister_dtor_with_atexit : Flag<["-"], "fregister-dtor-with-atexit">, Group<f_Group>, Flags<[CC1Option]>,
+  HelpText<"Use atexit or __cxa_atexit to register destructors">;
 def fuse_init_array : Flag<["-"], "fuse-init-array">, Group<f_Group>, Flags<[CC1Option]>,
----------------
Probably worth spelling out that this is for *global* destructors, at least in the help text and maybe in the option names.  At the very least, the option name should say "dtors" instead of just "dtor".


================
Comment at: include/clang/Frontend/CodeGenOptions.def:46
 CODEGENOPT(CXAAtExit         , 1, 1) ///< Use __cxa_atexit for calling destructors.
+CODEGENOPT(RegisterDtorWithAtExit, 1, 1) ///< Use atexit or __cxa_atexit to register destructors.
 CODEGENOPT(CXXCtorDtorAliases, 1, 0) ///< Emit complete ctors/dtors as linker
----------------
Same thing: worth saying in the option name that this is just global dtors.


================
Comment at: lib/CodeGen/CodeGenModule.h:1339
 
+  llvm::SmallDenseMap<int, SmallVector<llvm::Function *, 1>> DtorsUsingAtExit;
+
----------------
There's an llvm::TinyPtrVector which would be useful here.


================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:2213
+void CodeGenModule::registerDtorsWithAtExit() {
+  for (const auto I : DtorsUsingAtExit) {
+    int Priority = I.getFirst();
----------------
This is iterating over a DenseMap and therefore making the emission order dependent on details like how we hash ints for DenseMap.

Also, is this a correct way of handling Priority?  You should at least justify that in comments.


Repository:
  rC Clang

https://reviews.llvm.org/D45578