On Fri, Oct 11, 2024 at 10:37:11AM -0400, Jason Merrill wrote: > On 9/5/24 11:02 AM, Nathaniel Shead wrote: > > Bootstrapped and regtested (so far just dg.exp) on x86_64-pc-linux-gnu, > > OK for trunk if full regtest passes? Or would it be better to try to > > implement all the rules mentioned in the linked pull request for one > > commit; I admit I haven't looked very closely yet at how else we > > diverge? > > I see a few things in that pull request: > > 1. Mangling of multiple levels of template arguments, r14-4544. > 2. Mangling of lambdas in general class scope, this patch. > 3. Mangling explicit lambda template parameters, r13-3527. > 4. Mangling lambdas in expressions: looks like we use 'tl' instead of 'L'. > 5. A note to check that a lambda in the signature of a namespace-scope > function doesn't match a textually identical declaration in another TU; we > make the function internal, but I don't see a testcase. > > Fixing #4 as a quick followup would be good. >
Thanks for investigating, I'll take a look when I get a chance. > > This is a step closer to implementing the suggested changes for > > https://github.com/itanium-cxx-abi/cxx-abi/pull/85. > > > > The main purpose of the patch is to solve testcase PR c++/116568, caused > > by lambda expressions within the templates not correctly having the > > extra mangling scope attached. > > > > While I was working on this I found some other cases where the mangling > > of lambdas was incorrect and causing issues, notably the testcase > > lambda-ctx3.C which currently emits the same mangling for the base class > > and member lambdas, causing mysterious assembler errors. Fixing this > > ended up also improving the situation for PR c++/107741 as well, though > > it doesn't seem easily possible to fix the A::x case at this time so > > I've left that as an XFAIL. > > It looks pretty straightforward to split grokfield into start and finish > functions? But that certainly doesn't need to be part of this patch, which > is OK as is. > Thanks; I haven't actually pushed this yet because I've found it actually can cause breakages; consider e.g. struct S { template <int I> using T = decltype([]{ return I; }); }; S::T<0> a; S::T<1> b; int main() { a(); b(); } Currently this mangles as _ZNK1SUlvE0_clEv and _ZNK1SUlvE1_clEv, but with my patch they now both get mangled as _ZNK1SUlvE_clEv: because they're separate instantiations they each restart the numbering, it appears. I think the fix should be that template aliases should also get added as a mangling context so that they depend on the name of the member and any template params. This looks like it'll require a fair bit of adjustment to the mangling logic however. Does that seem reasonable to you? For comparison, Clang mangles as _ZNK1S3$_0clEv and _ZNK1S3$_1clEv currently and gives the lambdas internal linkage, which I think is incorrect. Nathaniel
