[libcxx-commits] [PATCH] D124346: [libc++] Complete the implementation of N4190
Eric Fiselier via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Jun 11 08:05:41 PDT 2022
EricWF added inline comments.
Comment at: libcxx/include/__functional/unary_function.h:51
+template <class _Derived, class _Arg, class _Result>
> ldionne wrote:
> > EricWF wrote:
> > > I'm not convinced this optimization is worth the complexity it adds to the library. I suspect nobody is really writing code that would benefit from this, and if they are, I don't think they really care about the extra byte.
> > >
> > > Also this generates different ABI's between C++14 and C++17, which is a not OK. You should be able to link code compiled in different dialects.
> > > Also this generates different ABI's between C++14 and C++17, which is a not OK.
> > I don't see it -- can you explain why we generate different ABIs? The goal of jumping through these hoops was specifically to generate the same ABI, but maybe we missed something (you know how tricky these changes are, so it's great to have additional opinions). This is how I understand the code right now (let's ignore `_LIBCPP_ABI_NO_BINDER_BASES` for the moment):
> > - `C++ <= 14`: we inherit from `unary_function<Arg, Result>`
> > - `C++ > 14`: we inherit from `__unary_function_keep_layout_base<Arg, Result>`
> > In both cases, the layout of the base classes is the same, and they have the same properties with respect to the empty base optimization applying. Did we miss something?
> Consider when `_LIBCPP_ABI_NO_BINDER_BASES` is defined.
> It's observable if you're inheriting from `__unary_function_ebo_base` base or not.
> Because we pass the extra arg to make sure the base class is unique, can't that uniqueness make something EBO away in C++17 that wouldn't in C++14?
Though I grant the cases which trigger this are likely rare. But they're also the same set of cases this EBO optimization is targeting.
Please let me know if I'm misunderstanding something.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits