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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 10 08:53:33 PST 2020


aaron.ballman added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenModule.h:1482
+  void AddGlobalDtor(llvm::Function *Dtor, int Priority = 65535,
+                     bool IsDtorAttrFunc = false);
 
----------------
Xiangling_L wrote:
> 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?
I think the signature should be:
```
void AddGlobalDtor(llvm::Function *Dtor, DestructorAttr *DA = nullptr);
```
(I don't have strong opinions about whether the attribute pointer should be defaulted to null or not.) `IsDtorAttrFunc` is implied by a nonnull pointer and the priority can be gleaned directly from that attribute (or set to the magic number if the attribute pointer is null).


================
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.
----------------
Xiangling_L wrote:
> 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.
Ah, the comment confused me because the user could always write something like:
```
[[clang::stdcall, gnu::destructor]] void func();
```
where the destructor function is not something you can call with the default (cdecl) calling convention. Should the comment say "we can reasonably call with the correct CC" instead to avoid this confusion?


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

https://reviews.llvm.org/D90892



More information about the cfe-commits mailing list