From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x436.google.com (mail-wr1-x436.google.com [IPv6:2a00:1450:4864:20::436]) by sourceware.org (Postfix) with ESMTPS id C8CDC3858D29 for ; Tue, 20 Jul 2021 09:51:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C8CDC3858D29 Received: by mail-wr1-x436.google.com with SMTP id a13so25278353wrf.10 for ; Tue, 20 Jul 2021 02:51:02 -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=e18L50PGwJD2/pF4AhG1fBB+fsBDmdRkBeY57PzQ2wg=; b=GfbQJPmNYrgYW9DJTuMHJmdNfwKthnsbU36fRsL6PsOQlOzZ38zL/Jp+I8/mvAhTAt PwUpNX+qEsYTreE7CcFG4Vu+DUQp+/CsuuQajN00XzLuhpn0x+bMU6GvYTCUBQpiRCKR WCmdyvh7fin6cWPFZhYOpuEvJR0riB0wQ47G5VP848OYQRS+wbRSGcj7NUL8aN3Fb/9F Opo7KjC+ENVFzqJ11pE6VIIJI4zik7mmSJ1ITP8+LIGXmh854u+qPDquVhOuZNfeP6kA S2vp/tKE94XkeFXsWIO9XraAjAfvoUcwldbMKoqQRqjPH8ewVwiAzrFLSTw96Zx8vLLC 6oHg== X-Gm-Message-State: AOAM531hHYxRl2MaKY7AEMoK6TDfy/opEGYm5utiUyDGi9C+Tc4QqQWn i7iQllg5ygHwCP2sO76B1JA6blmXKwc/VuLpahA= X-Google-Smtp-Source: ABdhPJxdyMM0Ohw4Nh4LkxDcPg0ywiAefpADt1ewGFDyRlp/UMbbH0eHXx/SVFtw1V3+r5M0Xq5iDlFd3LHVe1ETWs8= X-Received: by 2002:a5d:64ac:: with SMTP id m12mr33769321wrp.89.1626774661896; Tue, 20 Jul 2021 02:51:01 -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: From: Jonathan Wakely Date: Tue, 20 Jul 2021 10:50:50 +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:51:04 -0000 On Tue, 20 Jul 2021 at 10:49, Jonathan Wakely wrote= : > > 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 { ret= urn t; } > > for (const class loop *loop: as_const(loops_list...)) > > > > Does it look good to add below as_const along with loops_list in cfgloo= p.h? > > > > +/* Provide the functionality of std::as_const to support range-based f= or > > + 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`. The range-based for loop would not use cbegin and cend, so you'd still want to use as_const for that purpose.