erichkeane wrote: > @erichkeane Thank you for the feedback! I'll add a release note. > > > However, this ALSO needs to have an entry in the -fclang-abi list, this is > > obviously quite breaking of a change ABI wise. > > I've thought about this, but I'm not yet convinced that a new > `-fclang-abi-compat` entry is necessary. The change only affects > function-local classes, and while such types can technically escape their > function scope, they seems cannot escape the translation unit in a way that > would create a ABI compatibility issue. Am I missing something? As a point of > comparison, a similar mangling fix in [PR > #195340](https://github.com/llvm/llvm-project/pull/195340) did not introduce > a new ABI compatibility entry. That said, I'm happy to add one, just want to > make sure it is actually necessary. > > > Does our lambda numbering still work correctly? it isn't clear from your > > tests. > > I'm not intending to change lambda numbering, but I checked it for sure. This > example: > > ``` > void foo() { > { > auto f = [] { std::cout << 1 << '\n'; }; > f(); > } > > > > { > auto f = [] { std::cout << 2 << '\n'; }; > f(); > } > } > ``` > > produces a bit unusual mangling, but the numbering itself seems correct and > isn't affected by the patch: > > ``` > _ZZ3foovENK3$_0clEv > _ZZ3foovENK3$_1clEv > ``` > > But I'd expect smth like this anyway > > ``` > _ZZ3foovENKUlvE_clEv > _ZZ3foovENKUlvE0_clEv > ```
Hmm... we perhaps SHOULD have on that other patch... ``` Am I missing something? ``` Perhaps? Every time I personally determine "we probably don't need one because there is no WAY this can escape the TU!" I've been wrong :D https://github.com/llvm/llvm-project/pull/206740 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
