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); ---------------- bolshakov-a wrote: > 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 @aaron.ballman, @erichkeane, seems like it is already fixed in the ABI document: > Typically, only references to function template parameters occurring within > the dependent signature of the template are mangled this way. In other > contexts, template instantiation replaces references to template parameters > with the actual template arguments, and mangling should mangle such > references exactly as if they were that template argument. https://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangle.template-param See also [the discussion in the issue](https://github.com/itanium-cxx-abi/cxx-abi/issues/111#issuecomment-1567486892). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140996/new/ https://reviews.llvm.org/D140996 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
