On Mon, 8 Apr 2019 at 19:12, Ville Voutilainen <ville.voutilai...@gmail.com> wrote: > > On Mon, 8 Apr 2019 at 19:02, Jonathan Wakely <jwak...@redhat.com> wrote: > > The attached patch implements the same thing with totally separate > > __gen_vtable_r and __gen_vtable_r_impl class templates, instead of > > adding all the visit<R> functionality into the existing code (and then > > needing to tease it apart again with if-constexpr). > > > > The visit<R> implementation doesn't need to care about the > > __variant_cookie or __variant_idx_cookie cases, which simplifies > > things. > > > > This also adjusts some whitespace, for correct indentation and for > > readability. And removes a redundant && from a type. > > > > What do you think? > > I hate the duplication of __gen_vtable with a burning passion, because > *that* is the part that causes me heartburn, > not the compile-time ifs in the other bits. But if this is what you > want to ship, I can live with it.
A bit of elaboration: in all of this, the most dreadful parts to understand were _Multi_array and __gen_vtable. The latter is already a recursive template, with this patch we have two of them. I knew that I could split the hierarchy for <R> and non-<R>, but I instinctively went with a pass-through template parameter that I then handled in __visit_invoke, so as to specifically avoid splitting the recursive template into two recursive templates. That recursive template and how it recurses is not something we are going to change often, or really reason about often. But it's something that I need to rack my brain over every time, whereas looking at an if-constexpr is simple as pie to me. I suppose there are different levels of familiarity/comfort here.
