On Wed, Jan 22, 2025 at 10:03:40PM -0500, Jason Merrill wrote: > On 1/6/25 7:21 AM, Nathaniel Shead wrote: > > Something like this should probably be backported to GCC 14 too, since > > my change in r14-9232-g3685fae23bb008 inadvertantly caused ICEs that > > this fixes. But without the previous patch this patch will cause ABI > > changes, and I'm not sure how easily it would be to divorce those > > changes from the fix here. > > > > I suppose probably the true issue is that r14-9232 inadvertantly changed > > ABI for lambdas in base classes, and we should just revert the parser.cc > > changes for 14.3? (And accept that it'll regress those modules tests.) > > I suppose so. >
OK, I'll send around a potential patch at some point. > > -- >8 -- > > > > This is a step closer to implementing the suggested changes for > > https://github.com/itanium-cxx-abi/cxx-abi/pull/85. Most lambdas > > defined within a class should have an extra scope of that class so that > > uses across different TUs are properly merged by the linker. This also > > needs to happen during template instantiation. > > > > 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 since r14-9232. > > This is also the root cause of PR c++/118245. > > > > One notable case not handled either here or in the ABI is what is > > supposed to happen with lambdas declared in alias templates; see > > lambda-ctx4.C. I believe that by the C++ standard, such lambdas should > > also dedup across TUs, but this isn't currently implemented (for > > class-scope or not). I wasn't able to work out how to fix the mangling > > logic for this case easily so I've just excluded alias templates from > > the class-scope mangling rules in template instantiation. > > I'm skeptical of the instantiate_template change for other member templates > as well, since the set of instantiations of such member templates might vary > between translation units, so the lambda should be in the scope of that > member template instantiation rather than just the class. > > And I don't see any other member templates in the testcases? Like, for > > struct S { > template <int I> > decltype([]{ return I; }) f() { return {}; }; > }; > > void f(decltype(S().f<24>())*) {} > void f(decltype(S().f<42>())*) {} > > how are these lambdas mangled? Before this patch, the two lambdas have > arbitrary discriminators in S but they are treated as TU-local so it doesn't > matter. After this patch, they get the same number and are no longer > TU-local, and assembly fails. > > As with the member alias template example, I think it would be coherent to > say the lambdas are TU-local because they're between a template parameter > scope and a class/function/initializer, as a clarification to > https://eel.is/c++draft/basic#link-15.2 . Or we need to mangle them in the > scope of the member template instantiation even though they aren't within > the body of the function. > Thanks for looking into this, I hadn't considered this at all (I'd gotten caught up with the alias case...). I think I agree that just calling such lambdas TU-local is probably the most practical approach right now, especially given we're in stage 4. I'll send out a new version of the patch series (without the alias template mangling from #3 of this series) that treats them as such. Nathaniel > Interestingly, clang mangles them in the scope of S::f, without any template > arguments. That seems to have the same problem as just using S. And they > aren't treated as TU-local, so the arbitrary discriminators are an ABI > problem. > > Jason >
