On 5/18/20 6:51 PM, Martin Sebor via Gcc-patches wrote:
On 5/18/20 12:02 PM, Richard Sandiford wrote:Martin Sebor <mse...@gmail.com> writes:On 5/16/20 4:43 AM, Richard Sandiford wrote:Sorry for the empty subject line earlier... Jason Merrill <ja...@redhat.com> writes: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: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.patchFrom 10b27e367de0fa9d5bf91544385401cdcbdb8c00 Mon Sep 17 00:00:00 2001From: 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.htmlb/htdocs/codingconventions.htmlindex f4732ef6..ae49fb91 100644 --- a/htdocs/codingconventions.html +++ b/htdocs/codingconventions.html@@ -51,6 +51,7 @@ the conventions separately from any other changes tothe 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 andDestructors</a></li>@@ -884,6 +885,58 @@ Variables may be simultaneously defined and testedin 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. Forexample</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 anobject+ 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 isread-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 generaltemporaries.</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.htmlindex 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::iteratoris. All we can tell is that that it's some sort of an iterator, andthat should be good enough. It lets us (even forces us to) write code that satisfies the requirements of the abstraction (whatever it happensto be), and avoid tying it closely to the implementation. That's a good thing.Do you mean that this example should or shouldn't use "auto"? Iterators are included in the list above, so the idea was that using "auto" would be the recommended style here.I meant it as a general example where the exact type isn't (and isn't supposed to be) apparent to the caller because it's an implementation detail.But like I say, the proposal was that this example should use "auto", and it sounds like you might agree. In that case I don't think the example really helps make the decision about whether the coding standards are useful or not. Do you have other examples that you think would be better written using "auto" that aren't covered by the list, and aren't covered by Jason's point about template-dependent types?I think it applies any time mentioning the exact type isn't necessary, not just because it might introduce a dependency on details that the code doesn't need, but also because it's more verbose than the alternative. The for range loop mentioned upthread is an example. But auto is just one of a number of new core language features new in C++. Unlike some other C++ features(*), I don't think it's easily misused, at least not to such an extent to justify adding a guideline for us to use safely, consistently, or simply in good taste. Martin [*] I could see value in guidelines around rvalue references or deleted and defaulted functions, for instance, since those are easily used in dangerous ways. PS Herb Sutter has a nice article (as usual) where he summarizes commonly raised concerns with auto and his thoughts on them:https://herbsutter.com/2013/08/12/gotw-94-solution-aaa-style-almost-always-auto/Similarly, if an API defines a typedef string_tree_pair for std::pair<const char*, tree>, it's the typedef that's meant to be used in preference to what it expands to.Using the typedef would be fine. string_tree_pair describes the type pretty well: it's still obvious that "elt" is a pair, and what "elt.first" and "elt.second" are. Typedefs that don't describe the type well are bad typedefs. :-) The distinction is more between whether "auto" should be used or whether an explicit type should be used, rather than between different ways ofwriting the explicit type. I agree it'd be worth adding "string_tree_pair"to the rationale too, to make it clearer that this isn't about expanding the typedefs as far they'll go.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.But using "auto" is never going to be an efficiency concern, and probably not a safety concern. So in the case of "auto", using that principle would basically come down to "when to use auto is purely a judgement call". I don't see how we can get consistency with that kind of approach. Or is the argument that we're (or I'm :-)) worrying too much about consistency and we should just go with the flow? If we do treat it as a pure judgement call, the problem then is: who's judgement matters most here? The author's or the reviewer's? Should the reviewer respect the choice of the author even if they don't personally agree with it, given that there are no technical issues at stake?When no technical concerns are at stake contributors should be free to use the language as they feel is appropriate. The fewer hurdles we put in place the more time we will be able to focus on getting the many technical details right, and the more fun it will be to contribute.I agree that's a self-consistent approach. So I think at this point three options have been suggested: (1) Try to add coding conventions around when "auto" should and shouldn't be used. (2) Be broadly accepting of "auto", but reject cases that seem hard to read during code review. (3) Allow "auto" to be used anywhere that a contributor thinks is appropriate. Since the decision isn't usually a technical one, reviewers would be encouraged to respect the author's choice and be discouraged from asking for a different choice. Personally my preference order is (1), (3) and (2). I think (2) is the worst of both worlds: it wouldn't give a consistent codebase, because whether something is seen as hard to read would vary based on the people involved. And it wouldn't give predictability because contributors would only know whether a use of "auto" is acceptable by submitting a patch and seeing what the reaction is. (3) also wouldn't give a consistent codebase, but it would give more predictable reviews.If a consensus emerges that some uses are generally best avoided then it might be appropriate to reflect it in the coding conventions. But I'd hope that wouldn't happen before we've had time to gain experience with it.I think the difference here is between whether we start with a list of acceptable uses and expand it with experience, or whether we start by assuming all uses are acceptable and restrict it with experience. The reason I think the former is better is that we're starting with a codebase that doesn't use "auto" at all. So when the switch is flipped and the code is C++11, we'll have a C++11 codebase that never uses "auto". Given that starting point, it seems more natural to list cases in which "auto" should be used (as a change to the status quo) rather than those in which it shouldn't. But besides Jason's point about template-dependent types, I think the objections have been to the idea of having coding conventions around this in principle, rather than to the actual list. So at this point I'm not sure whether the proposed list would actually stop someone from using "auto" in the way they'd typically want to use it, and if so, what those use cases are. Like I say, the list was only supposed to be a starting point, based on my guess at what would be broadly acceptable. So if the chosen casesare themselves a sticking point, suggestions for additions or modificationsare definitely welcome. And the list can be expanded later if new uses crop up. That's still an improvement on how things were until now, where "auto" couldn't be used at all. Thanks, Richard
Richard, There is one use case that may be of issue but its up to other people to decide in that of auto&& and the issues around perfect forwarding. I'm aware of anything in the code base requiring perfect forwarding,and not in the near future. Would it to be nice to avoid that and the same for T& for templates? Or is there something I'm not aware of that
requires it. I'm assuming people know about the dangers of perfect forwarding with either of these four things T&,T&&,auto&&,auto&. Sorry if I'm injecting my thoughts late into the discussion about auto, Nick --Fundamentally an organism has conscious mental states if and only if there is something that it is like to be that organism--something it is like for the organism. - Thomas Nagel
