[PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

[Andrey Ali Khan Bolshakov via Phabricator via cfe-commits]

bolshakov-a added inline comments.


================
Comment at: clang/lib/AST/ItaniumMangle.cpp:4397
+    // argument.
+    // As proposed in https://github.com/itanium-cxx-abi/cxx-abi/issues/111.
+    auto *SNTTPE = cast<SubstNonTypeTemplateParmExpr>(E);
----------------
efriedma wrote:
> bolshakov-a wrote:
> > erichkeane wrote:
> > > erichkeane wrote:
> > > > aaron.ballman wrote:
> > > > > We should get this nailed down. It was proposed in Nov 2020 and the issue is still open. CC @rjmccall 
> > > > This definitely needs to happen.  @rjmccall or @eli.friedman ^^ Any idea what the actual mangling should be?
> > > This is still an open, and we need @rjmccall @eli.friedman or @asl to help out here.
> > Ping @efriedma, @rjmccall, @asl.
> I'm not really familiar with the mangling implications for this particular construct, nor am I actively involved with the Itanium ABI specification, so I'm not sure how I can help you directly.
> 
> That said, as a general opinion, I don't think it's worth waiting for updates to the Itanuim ABI  document to be merged; such updates are happening slowly at the moment, and having a consistent mangling is clearly an improvement even if it's not specified.  My suggested plan of action:
> 
> - Make sure you're satisfied the proposed mangling doesn't have any holes you're concerned about (i.e. it produces a unique mangling for all the relevant cases).  If you're not sure, I can try to spend some time understanding this, but it doesn't sound like you have any concerns about this.
> - Put a note on the issue in the Itanium ABI repo that you're planning to go ahead with using this mangling in clang.  Also send an email directly to @rjmccall and @rsmith in case they miss the notifications.
> - Go ahead with this.
> Put a note on the issue in the Itanium ABI repo that you're planning to go ahead with using this mangling in clang. Also send an email directly to @rjmccall and @rsmith in case they miss the notifications.

I'm sorry for noting one more time that Richard already pushed these changes in clang upstream, but they had been just reverted.

Maybe, I should make a PR into Itanium API repository, but I probably need some time to dig into the theory and all the discussions. But yes, even NTTP argument mangling rules are not still merged: https://github.com/itanium-cxx-abi/cxx-abi/pull/140


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

https://reviews.llvm.org/D140996