From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x329.google.com (mail-wm1-x329.google.com [IPv6:2a00:1450:4864:20::329]) by sourceware.org (Postfix) with ESMTPS id 556F83858D29 for ; Tue, 20 Jul 2021 09:50:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 556F83858D29 Received: by mail-wm1-x329.google.com with SMTP id m11-20020a05600c3b0bb0290228f19cb433so1148510wms.0 for ; Tue, 20 Jul 2021 02:50:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=a9wSmaY2nT11kwKuN1yhaRLVzOuiZWp28ZOSAWdzWIQ=; b=Pf1VkRZQK6iiatO91IF8Qo5U2jyTLxm6XNobKLmXk2zx3XRaIcdmZslQjzYxZ4nqAo mb5kgDgw1WfDlnfl+eL+CDSIpheyuPWixd7QROQpzf6cECRiwr4z9ZI7nTWN5+cw/Je6 4f0nGFOmMz2ePuHuh+ieN7cpBf52Q5yqAn5WVavlUpq08XG33mAsFoBoRy8b9g4ZW9iC KlOFNjhJZHWvrXY5B8OoAQQlEhwes3lftXvkAv6QL88lXEZjqfuJY0erguxKiIAQ1L5V TxTxkHmU3XSPl3QfyRvbufgIfdRnE05i/78euOtdXff6js0mubAzzKJeC/peCOHa0gIo vTog== X-Gm-Message-State: AOAM533Em3gSSwV6+H1yLZ1gnxzsL514UFNMB0qoemR2jNPMFQbH6Y7r P6EWJ7pS5B28ggwhvqIaVUxe5zUx9SgflpR4/4U= X-Google-Smtp-Source: ABdhPJzujxfd93AuPTwJRHkltb4F0jbqeRSsQT99MzbxYybivRudkTtPx9ay5mujoQsXoKynESXwamlGaxmYRGj2XCk= X-Received: by 2002:a1c:c91a:: with SMTP id f26mr30713491wmb.162.1626774609381; Tue, 20 Jul 2021 02:50:09 -0700 (PDT) MIME-Version: 1.0 References: <0a8b77ba-1d54-1eff-b54d-d2cb1e769e09@linux.ibm.com> <211abd8e-6a9b-1a22-dcfc-ede0c49f4223@gmail.com> <8d70f4f8-8855-750e-9b64-e623b46dcada@linux.ibm.com> In-Reply-To: <8d70f4f8-8855-750e-9b64-e623b46dcada@linux.ibm.com> From: Jonathan Wakely Date: Tue, 20 Jul 2021 10:49:58 +0100 Message-ID: Subject: Re: [RFC/PATCH] Use range-based for loops for traversing loops To: "Kewen.Lin" Cc: Martin Sebor , Richard Biener , Richard Sandiford , Jakub Jelinek , Trevor Saunders , Segher Boessenkool , GCC Patches Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-1.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Jul 2021 09:50:11 -0000 On Tue, 20 Jul 2021 at 09:58, Kewen.Lin wrote: > > on 2021/7/19 =E4=B8=8B=E5=8D=8811:59, Martin Sebor wrote: > > On 7/19/21 12:20 AM, Kewen.Lin wrote: > >> Hi, > >> > >> This patch follows Martin's suggestion here[1], to support > >> range-based for loops for traversing loops, analogously to > >> the patch for vec[2]. > >> > >> Bootstrapped and regtested on powerpc64le-linux-gnu P9, > >> x86_64-redhat-linux and aarch64-linux-gnu, also > >> bootstrapped on ppc64le P9 with bootstrap-O3 config. > >> > >> Any comments are appreciated. > > > > Thanks for this nice cleanup! Just a few suggestions: > > > > I would recommend against introducing new macros unless they > > offer a significant advantage over alternatives (for the two > > macros the patch adds I don't think they do). > > > > If improving const-correctness is one of our a goals > > the loops_list iterator type would need to a corresponding > > const_iterator type, and const overloads of the begin() > > and end() member functions. > > > > Rather than introducing more instances of the loop_p typedef > > I'd suggest to use loop *. It has at least two advantages: > > it's clearer (it's obvious it refers to a pointer), and lends > > itself more readily to making code const-correct by declaring > > the control variable const: for (const class loop *loop: ...) > > while avoiding the mistake of using const loop_p loop to > > declare a pointer to a const loop. > > > > Thanks for the suggestions, Martin! Will update them in V2. > > With some experiments, I noticed that even provided const_iterator > like: > > iterator > begin () > { > return iterator (*this, 0); > } > > + const_iterator > + begin () const > + { > + return const_iterator (*this, 0); > + } > > for (const class loop *loop: ...) will still use iterator instead > of const_iterator pair. We have to make the code look like: > > const auto& const_loops =3D loops_list (...); > for (const class loop *loop: const_loops) > > or > template constexpr const T &as_const(T &t) noexcept { retur= n t; } > for (const class loop *loop: as_const(loops_list...)) > > Does it look good to add below as_const along with loops_list in cfgloop.= h? > > +/* Provide the functionality of std::as_const to support range-based for > + to use const iterator. (We can't use std::as_const itself because it= 's > + a C++17 feature.) */ > +template > +constexpr const T & > +as_const (T &t) noexcept The noexcept is not needed because GCC is built -fno-exceptions. For consistency with all the other code that doesn't use noexcept, it should probably not be there. > +{ > + return t; > +} > + That's one option. Another option (which could coexist with as_const) is to add cbegin() and cend() members, which are not overloaded for const and non-const, and so always return a const_iterator: const_iterator cbegin () const { return const_iterator (*this, 0); } iterator begin () const { return cbegin(); } And similarly for `end () const` and `cend () const`.