From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 40090 invoked by alias); 8 Apr 2019 16:36:18 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 39900 invoked by uid 89); 8 Apr 2019 16:36:18 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.1 required=5.0 tests=AWL,BAYES_00,LIKELY_SPAM_BODY,SPF_HELO_PASS autolearn=no version=3.3.1 spammy=ship, hate, knew, rack X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 08 Apr 2019 16:36:16 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 418FD30198BD; Mon, 8 Apr 2019 16:36:15 +0000 (UTC) Received: from localhost (unknown [10.33.36.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id E8FCA5D9C9; Mon, 8 Apr 2019 16:36:14 +0000 (UTC) Date: Mon, 08 Apr 2019 16:36:00 -0000 From: Jonathan Wakely To: Ville Voutilainen Cc: libstdc++ , gcc-patches List Subject: Re: [PATCH] Implement std::visit for C++2a (P0655R1) Message-ID: <20190408163614.GW943@redhat.com> References: <20190405180616.GA15228@redhat.com> <20190405202927.GR943@redhat.com> <20190408160227.GU943@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.11.3 (2019-02-01) X-SW-Source: 2019-04/txt/msg00299.txt.bz2 On 08/04/19 19:20 +0300, Ville Voutilainen wrote: >On Mon, 8 Apr 2019 at 19:12, Ville Voutilainen > wrote: >> >> On Mon, 8 Apr 2019 at 19:02, Jonathan Wakely 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 functionality into the existing code (and then >> > needing to tease it apart again with if-constexpr). >> > >> > The visit 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. Completely agreed, that's why I found extending __gen_vtable_impl to handle a new feature made it even harder to understand. >The latter is already a recursive template, with this patch we have >two of them. I knew that I could split the hierarchy >for and non-, 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. The if-constexpr itself is easy to understand, but how we get from __do_visit to a specific specialization of a recursive class template and a specific if-constexpr condition is the hard part for me to follow. But now that I've wrapped my head around the existing code, I'll spend some more time getting acquainted with your proposed changes and see if they grow on me overnight.