[PATCH] D90892: [AIX][FE] Support constructor/destructor attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:276
   // extern "C" int atexit(void (*f)(void));
-  assert(cast<llvm::Function>(dtorStub)->getFunctionType() ==
-             llvm::FunctionType::get(CGM.VoidTy, false) &&
+  llvm::PointerType *Expected =
+      llvm::PointerType::get(llvm::FunctionType::get(CGM.VoidTy, false),
----------------
This will likely cause unused variable warnings in release builds due to the `assert` macro.


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:303
   // extern "C" int unatexit(void (*f)(void));
-  assert(dtorStub->getFunctionType() ==
-             llvm::FunctionType::get(CGM.VoidTy, false) &&
+  llvm::PointerType *Expected =
+      llvm::PointerType::get(llvm::FunctionType::get(CGM.VoidTy, false),
----------------
Same here.


================
Comment at: clang/lib/CodeGen/CodeGenModule.h:1482
+  void AddGlobalDtor(llvm::Function *Dtor, int Priority = 65535,
+                     bool IsDtorAttrFunc = false);
 
----------------
There's a fixme comment a few lines up about hardcoding priority being gross and this sort of extends the grossness a bit. Perhaps these functions should accept a `DestructorAttr *`/`ConstructorAttr *` that can be null?


================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2537
+  // priority.
+  CodeGenFunction CGF(CGM);
+
----------------
Do you need this? I think you can get `VoidTy` off `CGM` already which seems to be the only use of `CGF`.


================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2584
+      // We're assuming that the destructor function is something we can
+      // reasonably call with the default CC.  Go ahead and cast it to the
+      // right prototype.
----------------
Is this assumption safe though given that there are calling convention attributes that can be written on the function?


================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2648
+        // We're assuming that the destructor function is something we can
+        // reasonably call with the default CC.  Go ahead and cast it to the
+        // right prototype.
----------------
Same question here as above.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90892/new/

https://reviews.llvm.org/D90892