[PATCH] D19275: Do not register incompatible C++ destructors with __cxa_atexit

[JF Bastien via cfe-commits]

jfb added inline comments.

================
Comment at: lib/CodeGen/CGDeclCXX.cpp:92
@@ +91,3 @@
+  // disabled via a flag, a different helper function is generated anyway.
+  const CXXRecordDecl *Record = type->getAsCXXRecordDecl();
+  bool CanRegisterDestructor = Record &&
----------------
Can you do that unconditionally? Or do you have to test for `dtorKind == QualType::DK_cxx_destructor` first? It looks like the function already does a `dyncast_or_null` so the previous code was doing a check it didn't need to.

================
Comment at: lib/CodeGen/CGDeclCXX.cpp:95
@@ +94,3 @@
+                               !CGM.getCXXABI().HasThisReturn(GlobalDecl(
+                                   Record->getDestructor(), Dtor_Complete));
+
----------------
Comment on why you can't register dtor when has this return (ABI mismatch).

================
Comment at: lib/CodeGen/CGDeclCXX.cpp:97
@@ +96,3 @@
+
+  if (dtorKind == QualType::DK_cxx_destructor && Record &&
+      (CanRegisterDestructor || !CGM.getCodeGenOpts().CXAAtExit)) {
----------------
Do you need to check `Record` here? Doesn't it also imply `dtorKind == QualType::DK_cxx_destructor`?

================
Comment at: lib/CodeGen/CGDeclCXX.cpp:98
@@ +97,3 @@
+  if (dtorKind == QualType::DK_cxx_destructor && Record &&
+      (CanRegisterDestructor || !CGM.getCodeGenOpts().CXAAtExit)) {
+    assert(!Record->hasTrivialDestructor());
----------------
` || !CGM.getCodeGenOpts().CXAAtExit` seems wrong here. Shouldn't the condition be `CanRegisterDestructor && CGM.getCodeGenOpts().CXAAtExit`? I may be misunderstanding...


http://reviews.llvm.org/D19275