From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by sourceware.org (Postfix) with ESMTP id 413E03851C2D for ; Sat, 16 May 2020 02:45:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 413E03851C2D Received: from mail-il1-f198.google.com (mail-il1-f198.google.com [209.85.166.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-316-aOEkWlcGPN6-ukaddEnmnw-1; Fri, 15 May 2020 22:45:44 -0400 X-MC-Unique: aOEkWlcGPN6-ukaddEnmnw-1 Received: by mail-il1-f198.google.com with SMTP id d8so3999251ilo.1 for ; Fri, 15 May 2020 19:45:44 -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; bh=GE25ldOlAidFrKZb48GK7IDKekU8wsNHUf1D8tHdu+8=; b=p0Pp8pOzVzOySb/7H0UAZN+o2WQuZtKgQupO56qwzFxFwZGaK7XiFLjmFbuuvO/45u GruUvT9BkJX3mGRQ6NdnaHLDqjAbyO2kbeZ8EpreW4qeWbzaOd5K9/oUWGZyWxfNQz9k 6WCIsHhgAL0wMMekX8tGiXSG16Vz3Cz2PoqKHczKaQ9o4XSM8b4rrsMjT20Gf5hPFJA9 Kr+vyoYbFj6g8tSZ5d6XVAlTdciJejcoa0KsxPcQyyMzqBsFyhoMMJAnicuuyl0c9vJm Zb09sChIA1Qfmij/Ski24QyF0DsPtjwcQWNMBAUySKAM0Z02IpiWKPgzbpgmn5WUDv4V O1Gg== X-Gm-Message-State: AOAM5321cOZlqgruug/WG7quNHl2igHmWITxbzbqFhGy+QolEkljTVcH /ouEdDIJkoa/SYHG7BT+BncTKHBiROFqvez/UGv6Fqv5RNKugIvxYSxNA4RwRaHHd9vQgdkjxJQ KWYGXDOtH/seqN5UwIcRRep6c29++JgqVYw== X-Received: by 2002:a92:3603:: with SMTP id d3mr6486536ila.264.1589597143862; Fri, 15 May 2020 19:45:43 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx9ky3K1w35Ybzb88KV0nsZ48KHTn+cqo3Vve5J0VNWyCHlsbd1PZsa6XgDcmy7Tnt9Di6H0VGQpEjdtXz89r4= X-Received: by 2002:a92:3603:: with SMTP id d3mr6486511ila.264.1589597143494; Fri, 15 May 2020 19:45:43 -0700 (PDT) MIME-Version: 1.0 References: <20200514210559.4449-1-jason@redhat.com> <5296a9f2-ad5d-ab12-0385-bc7742a2877a@gmail.com> In-Reply-To: <5296a9f2-ad5d-ab12-0385-bc7742a2877a@gmail.com> From: Jason Merrill Date: Fri, 15 May 2020 22:45:32 -0400 Message-ID: Subject: Re: To: Martin Sebor Cc: Richard Biener , Richard Biener via Gcc-patches , Richard Sandiford X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-13.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, HTML_MESSAGE, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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: Sat, 16 May 2020 02:45:49 -0000 On Fri, May 15, 2020 at 9:47 PM Martin Sebor wrote: > On 5/15/20 8:08 AM, Richard Sandiford wrote: > >>>> We've moved more and more to stronly-typed data structures > >>>> so I'd not like to see 'auto' everywhere - it should be still > >>>> obvious what kind of objects we're working with where they > >>>> matter. IMHO they do not matter for example for iterators. > >>>> I don't care about the iterator type but about the type of > >>>> the object and the container. > >>> Also agreed. :-) How about this as a starting point: > >>> > >>> --------------------------------------------------------------- > >>> Use auto for: > >>> > >>> - the result of casts or other expressions that give the type > >>> explicitly. E.g.: > >>> > >>> if (auto *table = dyn_cast (insn)) > >>> > >>> instead of: > >>> > >>> if (rtx_jump_table_data *table = dyn_cast > (insn)) > >>> > >>> - iterator types. E.g.: > >>> > >>> auto it = foo.begin (); > >>> > >>> instead of: > >>> > >>> foo_type::iterator it = foo.begin (); > >>> > >>> - expressions that provide an alternative view of something, > >>> when the expression is bound to a read-only temporary. E.g.: > >>> > >>> auto val1 = wi::to_wide (...); > >>> auto val2 = wi::uhwi (12, 16); > >>> > >>> instead of: > >>> > >>> wide_int val1 = wi::to_wide (...); > >>> wide_int val2 = wi::uhwi (12, 16); > >>> > >>> (Using "wide_int" is less efficient than using the natural type of > >>> the expression.) > >>> > >>> - the type of a lambda expression. E.g.: > >>> > >>> auto f = [] (int x) { return x + 1; }; > >> Those are all good examples. Mind putting that into a patch > >> for the coding conventions? > > How's this? I added "new" expressions as another example of the > > first category. > > > > I'm sure I've missed other good uses, but we can always add to the > > list later if necessary. > > > > Thanks, > > Richard > > > > > > 0001-Describe-coding-conventions-surrounding-auto.patch > > > > From 10b27e367de0fa9d5bf91544385401cdcbdb8c00 Mon Sep 17 00:00:00 2001 > > From: Richard Sandiford > > Date: Fri, 15 May 2020 14:58:46 +0100 > > Subject: [PATCH] Describe coding conventions surrounding "auto" > > > > --- > > htdocs/codingconventions.html | 53 +++++++++++++++++++++++++++++++++++ > > htdocs/codingrationale.html | 17 +++++++++++ > > 2 files changed, 70 insertions(+) > > > > diff --git a/htdocs/codingconventions.html > b/htdocs/codingconventions.html > > index f4732ef6..ae49fb91 100644 > > --- a/htdocs/codingconventions.html > > +++ b/htdocs/codingconventions.html > > @@ -51,6 +51,7 @@ the conventions separately from any other changes to > the code.

> >
  • Language Use > >
      > >
    • Variable Definitions
      • > > +
    • Use of auto
      • > >
    • Struct Definitions
      • > >
    • Class Definitions
      • > >
    • Constructors and > Destructors
      • > > @@ -884,6 +885,58 @@ Variables may be simultaneously defined and tested > in control expressions. > > Rationale and Discussion > >

      > > > > +

      Use of auto

      > > + > > +

      auto should be used in the following circumstances: > > +

        > > +

      • when the expression gives the C++ type explicitly. For > example

        > > + > > +
        > > +
        if (auto *table = dyn_cast <rtx_jump_table_data
> *> (insn))                 // OK
> > +  ...
> > +if (rtx_jump_table_data *table = dyn_cast <rtx_jump_table_data *>
> (insn))  // Avoid
> > +  ...
> > +auto *map = new hash_map <tree, size_t>;
>         // OK
> > +hash_map <tree, size_t> *map = new hash_map <tree, size_t>;
> // Avoid
        > > + > > +

        This rule does not apply to abbreviated type names embedded in > > + an identifier, such as the result of tree_to_shwi.

        > > +
        • > > +
      • > > +

        when the expression simply provides an alternative view of an > object > > + and is bound to a read-only temporary. For example:

        > > + > > +
        > > +
        auto wioff = wi::to_wide (off);         // OK
> > +wide_int wioff = wi::to_wide (off);     // Avoid if wioff is read-only
> > +auto minus1 = std::shwi (-1, prec);     // OK
> > +wide_int minus1 = std::shwi (-1, prec); // Avoid if minus1 is
> read-only
        > > + > > +

        In principle this rule applies to other views of an object too, > > + such as a reversed view of a list, or a sequential view of a > > + hash_set. It does not apply to general > temporaries.

        > > +
        • > > +
      • > > +

        the type of an iterator. For example:

        > > + > > +
        > > +
        auto it = std::find (names.begin (), names.end (),
> needle);        // OK
> > +vector <name_map>::iterator it = std::find (names.begin (),
> > +                                            names.end (), needle); //
> Avoid
        > > +
        • > > +
      • > > +

        the type of a lambda expression. For example:

        > > + > > +
        > > +
        auto f = [] (int x) { return x + 1; }; //
> OK
        > > +
        • > > +

      > > + > > +

      auto should not be used in other contexts.

      > > This seems like a severe (and unnecessary) restriction... > > > + > > +

      > > +Rationale and Discussion > > +

      > > > >

      Struct Definitions

      > > > > diff --git a/htdocs/codingrationale.html b/htdocs/codingrationale.html > > index 0b44f1da..a919023c 100644 > > --- a/htdocs/codingrationale.html > > +++ b/htdocs/codingrationale.html > > @@ -50,6 +50,23 @@ if (info *q = get_any_available_info ()) { > > } > > > > > > +

      Use of auto

      > > + > > +

      The reason for preferring auto in expressions like: > > +

      auto wioff = wi::to_wide (off);
      > > +is that using the natural type of the expression is more efficient than > > +converting it to types like wide_int.

      > > + > > +

      The reason for excluding other uses of auto is that > > +in most other cases the type carries useful information. For example: > > +

      for (const std::pair <const char *, tree>
> &elt : indirect_pool)
> > +  ...
      > > +makes it obvious that elt is a pair and gives the types of > > +elt.first and elt.second. In contrast: > > +
      for (const auto &elt : indirect_pool)
> > +  ...
      > > +gives no immediate indication what elt is or what can > > +be done with it.

      > > ...there are countless constructs in C++ 98 as well in C where there > is no such indication yet we don't (and very well can't) try to avoid > using them. Examples include macros, members of structures defined > far away from the point of their use, results of ordinary function > calls, results of overloaded functions or templates, default function > arguments, default template parameters, etc. > > By way of a random example from genrecog.c: > > int_set::iterator end > = std::set_union (trans1->labels.begin (), trans1->labels.end (), > combined->begin (), combined->end (), > next->begin ()); > > There is no immediate indication precisely what type int_set::iterator > is. All we can tell is that that it's some sort of an iterator, and > that should be good enough. It lets us (even forces us to) write code > that satisfies the requirements of the abstraction (whatever it happens > to be), and avoid tying it closely to the implementation. That's > a good thing. > > Unless there is a sound technical reason for avoiding it (e.g., > unacceptable inefficiency or known safety problems) I'd say leave > it to everyone's judgment what convenience features to use. If > something turns out to be a problem we'll deal with it then. > I agree with this. If using 'auto' makes the code harder to read, that's a good comment for code review, but it's hard to write a general rule for this sort of thing. Jason