sfertile added inline comments.

================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:520
+
+class XLCXXABI final : public ItaniumCXXABI {
+public:
----------------
Xiangling_L wrote:
> sfertile wrote:
> > Here would be a good place to add a comment to indicate that XL has several 
> > C++ ABIs, but this represents the one used in 'xlClang++'.
> You mean we have legacy XLC and XLClang++ ABI? But for static init, they have 
> same implementation. So it's not a must to point it out. 
> 
> And also AFAIK, `static init` is the only thing we will differ from Generic 
> Itanium ABI in the frontend, so basically it's the only thing we will add in 
> this ABI.
> 
> I am okay with either way with a little concern that legacy XLC user may 
> wonder is there any difference of static init implementation between XLC and 
> XLClang++ ABI if we add the comment.
Sorry, I had a matching comment on the 'XL' enum, but I must have deleted it 
accidentally before submitting. I said I agreed with using just 'XL' since 
there is only one XL C++ ABI implemented in Clang we don't have to worry about 
differentiating between the 'legacy' XL and the C++11 XL ABIs. If you did want 
to clarify then adding a comment here would be the only thing I suggest.


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

https://reviews.llvm.org/D74015



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

Reply via email to