ahatanak 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]>,
----------------
rjmccall wrote:
> 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".
I renamed the option to `-fregister-global-dtors-with-atexit`.


================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:2213
+void CodeGenModule::registerDtorsWithAtExit() {
+  for (const auto I : DtorsUsingAtExit) {
+    int Priority = I.getFirst();
----------------
rjmccall wrote:
> 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.
I added a comment in registerGlobalDtorsWithAtExit that explains why destructor 
functions will be run in non-ascending order of their priority, which is what 
is expected.


Repository:
  rC Clang

https://reviews.llvm.org/D45578



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

Reply via email to