On Fri, May 15, 2020 at 9:47 PM Martin Sebor <mse...@gmail.com> 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 <rtx_jump_table_data *> (insn))
> >>>
> >>> instead of:
> >>>
> >>> if (rtx_jump_table_data *table = dyn_cast <rtx_jump_table_data *>
> (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<richard.sandif...@arm.com>
> > 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.</p>
> > <li><a href="#Cxx_Language">Language Use</a>
> > <ul>
> > <li><a href="#Variable">Variable Definitions</a></li>
> > + <li><a href="#Auto">Use of <code>auto</code></a></li>
> > <li><a href="#Struct_Use">Struct Definitions</a></li>
> > <li><a href="#Class_Use">Class Definitions</a></li>
> > <li><a href="#Constructors">Constructors and
> Destructors</a></li>
> > @@ -884,6 +885,58 @@ Variables may be simultaneously defined and tested
> in control expressions.
> > <a href="codingrationale.html#variables">Rationale and Discussion</a>
> > </p>
> >
> > +<h4 id="Auto">Use of <code>auto</code></h4>
> > +
> > +<p><code>auto</code> should be used in the following circumstances:
> > +<ul>
> > + <li><p>when the expression gives the C++ type explicitly. For
> example</p>
> > +
> > + <blockquote>
> > +<pre>if (<b>auto *</b>table = dyn_cast <<b>rtx_jump_table_data
> *</b>> (insn)) // OK
> > + ...
> > +if (rtx_jump_table_data *table = dyn_cast <rtx_jump_table_data *>
> (insn)) // Avoid
> > + ...
> > +<b>auto *</b>map = new <b>hash_map <tree, size_t></b>;
> // OK
> > +hash_map <tree, size_t> *map = new hash_map <tree, size_t>;
> // Avoid</pre></blockquote>
> > +
> > + <p>This rule does not apply to abbreviated type names embedded in
> > + an identifier, such as the result of <code>tree_to_shwi</code>.</p>
> > + </li>
> > + <li>
> > + <p>when the expression simply provides an alternative view of an
> object
> > + and is bound to a read-only temporary. For example:</p>
> > +
> > + <blockquote>
> > +<pre><b>auto</b> wioff = <b>wi::to_wide (off);</b> // OK
> > +wide_int wioff = wi::to_wide (off); // Avoid if wioff is read-only
> > +<b>auto</b> minus1 = <b>std::shwi (-1, prec);</b> // OK
> > +wide_int minus1 = std::shwi (-1, prec); // Avoid if minus1 is
> read-only</pre></blockquote>
> > +
> > + <p>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
> > + <code>hash_set</code>. It does not apply to general
> temporaries.</p>
> > + </li>
> > + <li>
> > + <p>the type of an iterator. For example:</p>
> > +
> > + <blockquote>
> > +<pre><b>auto</b> it = <b>std::find (names.begin (), names.end (),
> needle)</b>; // OK
> > +vector <name_map>::iterator it = std::find (names.begin (),
> > + names.end (), needle); //
> Avoid</pre></blockquote>
> > + </li>
> > + <li>
> > + <p>the type of a lambda expression. For example:</p>
> > +
> > + <blockquote>
> > +<pre><b>auto</b> f = <b>[] (int x) { return x + 1; }</b>; //
> OK</pre></blockquote>
> > + </li>
> > +</ul></p>
> > +
> > +<p><code>auto</code> should <b>not</b> be used in other contexts.</p>
>
> This seems like a severe (and unnecessary) restriction...
>
> > +
> > +<p>
> > +<a href="codingrationale.html#auto">Rationale and Discussion</a>
> > +</p>
> >
> > <h4 id="Struct_Use">Struct Definitions</h4>
> >
> > 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 ()) {
> > }
> > </code></pre></blockquote>
> >
> > +<h4 id="auto">Use of <code>auto</code></h4>
> > +
> > +<p>The reason for preferring <code>auto</code> in expressions like:
> > +<blockquote><pre>auto wioff = wi::to_wide (off);</pre></blockquote>
> > +is that using the natural type of the expression is more efficient than
> > +converting it to types like <code>wide_int</code>.</p>
> > +
> > +<p>The reason for excluding other uses of <code>auto</code> is that
> > +in most other cases the type carries useful information. For example:
> > +<blockquote><pre>for (const std::pair <const char *, tree>
> &elt : indirect_pool)
> > + ...</pre></blockquote>
> > +makes it obvious that <code>elt</code> is a pair and gives the types of
> > +<code>elt.first</code> and <code>elt.second</code>. In contrast:
> > +<blockquote><pre>for (const auto &elt : indirect_pool)
> > + ...</pre></blockquote>
> > +gives no immediate indication what <code>elt</code> is or what can
> > +be done with it.</p>
>
> ...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
