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

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L marked 6 inline comments as done.
Xiangling_L added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenModule.h:1482
+  void AddGlobalDtor(llvm::Function *Dtor, int Priority = 65535,
+                     bool IsDtorAttrFunc = false);
 
----------------
aaron.ballman wrote:
> 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?
Yeah, I can understand that putting a magic number as 65535 here is gross, but a `bool` with default argument also falls into that way? Or you are indicating it's better to not use default argument?


================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2537
+  // priority.
+  CodeGenFunction CGF(CGM);
+
----------------
aaron.ballman wrote:
> Do you need this? I think you can get `VoidTy` off `CGM` already which seems to be the only use of `CGF`.
Thanks, you are right. I am gonna remove it.


================
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.
----------------
aaron.ballman wrote:
> Is this assumption safe though given that there are calling convention attributes that can be written on the function?
Actually I copied this comment from where linux implemented the dtor attribute functions. I think it makes sense to make this assumption. Because when they are used as destructor functions, they actually don't have any caller from source.


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

https://reviews.llvm.org/D90892