Re:

[Martin Sebor <msebor@gmail.com>]

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.sandiford@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 &lt;<b>rtx_jump_table_data *</b>&gt; (insn))                 // OK
> +  ...
> +if (rtx_jump_table_data *table = dyn_cast &lt;rtx_jump_table_data *&gt; (insn))  // Avoid
> +  ...
> +<b>auto *</b>map = new <b>hash_map &lt;tree, size_t&gt;</b>;                    // OK
> +hash_map &lt;tree, size_t&gt; *map = new hash_map &lt;tree, size_t&gt;; // 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 &lt;name_map&gt;::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 &lt;const char *, tree&gt; &amp;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 &amp;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.

Martin