[PATCH/coding style] clarify pointers and operators

[PATCH/coding style] clarify pointers and operators

[Martin Sebor]

Martin suggested we update the Coding Conventions to describe
the expected style for function declarations with a pointer
return types, and for overloaded operators.  Below is the patch.

As an aside, regarding the space convention in casts: a crude
grep search yields about 10,000 instances of the "(type)x" kinds
of casts in GCC sources and 40,000 of the preferred "(type) x"
style with the space.  That's a consistency of only 80%.  Is
it worth documenting a preference for a convention that's so
inconsistently followed?

Martin

Index: htdocs/codingconventions.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/codingconventions.html,v
retrieving revision 1.85
diff -u -r1.85 codingconventions.html
--- htdocs/codingconventions.html	30 Sep 2018 14:38:46 -0000	1.85
+++ htdocs/codingconventions.html	26 Nov 2018 17:59:21 -0000
@@ -803,12 +803,17 @@
    <td><code>- x</code></td>
  </tr><tr>
    <td class="left">cast</td>
-  <td class="right"><code>(foo) x</code></td>
-  <td><code>(foo)x</code></td>
+  <td class="right"><code>(type) x</code></td>
+  <td><code>(type)x</code></td>
  </tr><tr>
-  <td class="left">pointer dereference</td>
-  <td class="right"><code>*x</code></td>
-  <td><code>* x</code></td>
+  <td class="left">pointer cast</td>
+  <td class="right"><code>(type *) x</code></td>
+  <td><code>(type*)x</code></td>
+</tr>
+</tr><tr>
+  <td class="left">pointer return type</td>
+  <td class="right"><code>type *f (void)</code></td>
+  <td><code>type* f (void)</code></td>
  </tr>
  </table>

@@ -992,6 +997,21 @@
  <a href="codingrationale.html#overoper">Rationale and Discussion</a>
  </p>

+<p>
+Note: in declarations of operator functions or in invocations of
+such functions that involve the keyword <code>operator</code>
+the full name of the operator should be considered as including
+the keyword with no spaces in between the keyowrd and the operator
+token.  Thus, the expected format of a declaration of an operator
+is<pre>
+    T &amp;operator== (const T &amp; const T &amp);
+</pre>
+and not for example<pre>
+    T &amp;operator == (const T &amp; const T &amp);
+</pre>
+(with the space between <code>operator</code> and <code>==</code>).
+</p>
+

  <h4 id="Default">Default Arguments</h4>

Re: [PATCH/coding style] clarify pointers and operators

[Jeff Law]

On 11/26/18 10:59 AM, Martin Sebor wrote:
> Martin suggested we update the Coding Conventions to describe
> the expected style for function declarations with a pointer
> return types, and for overloaded operators.  Below is the patch.
THanks.  That's fine.

> 
> As an aside, regarding the space convention in casts: a crude
> grep search yields about 10,000 instances of the "(type)x" kinds
> of casts in GCC sources and 40,000 of the preferred "(type) x"
> style with the space.  That's a consistency of only 80%.  Is
> it worth documenting a preference for a convention that's so
> inconsistently followed?
Please do.  It's a fairly recent change -- I suspect some old code was
never fixed and some folks (perhaps myself) have that extraneous
whitespace in their muscle memory and still need to eliminate it.

jeff

Re: [PATCH/coding style] clarify pointers and operators

[Martin Liška]

On 11/26/18 6:59 PM, Martin Sebor wrote:
> Martin suggested we update the Coding Conventions to describe
> the expected style for function declarations with a pointer
> return types, and for overloaded operators.  Below is the patch.
> 
> As an aside, regarding the space convention in casts: a crude
> grep search yields about 10,000 instances of the "(type)x" kinds
> of casts in GCC sources and 40,000 of the preferred "(type) x"
> style with the space.  That's a consistency of only 80%.  Is
> it worth documenting a preference for a convention that's so
> inconsistently followed?
> 
> Martin
> 
> Index: htdocs/codingconventions.html
> ===================================================================
> RCS file: /cvs/gcc/wwwdocs/htdocs/codingconventions.html,v
> retrieving revision 1.85
> diff -u -r1.85 codingconventions.html
> --- htdocs/codingconventions.html    30 Sep 2018 14:38:46 -0000    1.85
> +++ htdocs/codingconventions.html    26 Nov 2018 17:59:21 -0000
> @@ -803,12 +803,17 @@
>    <td><code>- x</code></td>
>  </tr><tr>
>    <td class="left">cast</td>
> -  <td class="right"><code>(foo) x</code></td>
> -  <td><code>(foo)x</code></td>
> +  <td class="right"><code>(type) x</code></td>
> +  <td><code>(type)x</code></td>
>  </tr><tr>
> -  <td class="left">pointer dereference</td>
> -  <td class="right"><code>*x</code></td>
> -  <td><code>* x</code></td>
> +  <td class="left">pointer cast</td>
> +  <td class="right"><code>(type *) x</code></td>
> +  <td><code>(type*)x</code></td>
> +</tr>
> +</tr><tr>
> +  <td class="left">pointer return type</td>
> +  <td class="right"><code>type *f (void)</code></td>
> +  <td><code>type* f (void)</code></td>
>  </tr>
>  </table>
> 
> @@ -992,6 +997,21 @@
>  <a href="codingrationale.html#overoper">Rationale and Discussion</a>
>  </p>
> 
> +<p>
> +Note: in declarations of operator functions or in invocations of
> +such functions that involve the keyword <code>operator</code>
> +the full name of the operator should be considered as including
> +the keyword with no spaces in between the keyowrd and the operator
> +token.  Thus, the expected format of a declaration of an operator
> +is<pre>
> +    T &amp;operator== (const T &amp; const T &amp);
> +</pre>
> +and not for example<pre>
> +    T &amp;operator == (const T &amp; const T &amp);
> +</pre>
> +(with the space between <code>operator</code> and <code>==</code>).
> +</p>
> +
> 
>  <h4 id="Default">Default Arguments</h4>
> 

Thank you Martin for the patch, I like it.

Martin

Re: [PATCH/coding style] clarify pointers and operators

[Jakub Jelinek]

On Mon, Nov 26, 2018 at 10:59:47AM -0700, Martin Sebor wrote:
> --- htdocs/codingconventions.html	30 Sep 2018 14:38:46 -0000	1.85
> +++ htdocs/codingconventions.html	26 Nov 2018 17:59:21 -0000
> @@ -803,12 +803,17 @@
>    <td><code>- x</code></td>
>  </tr><tr>
>    <td class="left">cast</td>
> -  <td class="right"><code>(foo) x</code></td>
> -  <td><code>(foo)x</code></td>
> +  <td class="right"><code>(type) x</code></td>
> +  <td><code>(type)x</code></td>
>  </tr><tr>
> -  <td class="left">pointer dereference</td>
> -  <td class="right"><code>*x</code></td>
> -  <td><code>* x</code></td>

Why have you removed the pointer dereference case, rather than just added
the new cases and replaced foo with type for cast?  We want people to write
*x rather than * x.

	Jakub

Re: [PATCH/coding style] clarify pointers and operators

[Martin Sebor]

On 11/27/18 5:57 AM, Jakub Jelinek wrote:
> On Mon, Nov 26, 2018 at 10:59:47AM -0700, Martin Sebor wrote:
>> --- htdocs/codingconventions.html	30 Sep 2018 14:38:46 -0000	1.85
>> +++ htdocs/codingconventions.html	26 Nov 2018 17:59:21 -0000
>> @@ -803,12 +803,17 @@
>>     <td><code>- x</code></td>
>>   </tr><tr>
>>     <td class="left">cast</td>
>> -  <td class="right"><code>(foo) x</code></td>
>> -  <td><code>(foo)x</code></td>
>> +  <td class="right"><code>(type) x</code></td>
>> +  <td><code>(type)x</code></td>
>>   </tr><tr>
>> -  <td class="left">pointer dereference</td>
>> -  <td class="right"><code>*x</code></td>
>> -  <td><code>* x</code></td>
> 
> Why have you removed the pointer dereference case, rather than just added
> the new cases and replaced foo with type for cast?  We want people to write
> *x rather than * x.

Whoops, that was unintentional.  I just restored it.

Martin

Re: [PATCH/coding style] clarify pointers and operators

[Segher Boessenkool]

On Mon, Nov 26, 2018 at 01:41:07PM -0700, Jeff Law wrote:
> On 11/26/18 10:59 AM, Martin Sebor wrote:
> > As an aside, regarding the space convention in casts: a crude
> > grep search yields about 10,000 instances of the "(type)x" kinds
> > of casts in GCC sources and 40,000 of the preferred "(type) x"
> > style with the space.  That's a consistency of only 80%.  Is
> > it worth documenting a preference for a convention that's so
> > inconsistently followed?
> Please do.  It's a fairly recent change -- I suspect some old code was
> never fixed and some folks (perhaps myself) have that extraneous
> whitespace in their muscle memory and still need to eliminate it.

Huh?  Spaces after casts are required, and make things much more
readable.  This isn't recent.

A lot of old code writes spaces after single-character unary operators,
too, which is ugly and less readable.  It's not recent that this is
explicitly documented as wrong, either.


Segher

Re: [PATCH/coding style] clarify pointers and operators

[Jeff Law]

On 11/27/18 11:46 AM, Segher Boessenkool wrote:
> On Mon, Nov 26, 2018 at 01:41:07PM -0700, Jeff Law wrote:
>> On 11/26/18 10:59 AM, Martin Sebor wrote:
>>> As an aside, regarding the space convention in casts: a crude
>>> grep search yields about 10,000 instances of the "(type)x" kinds
>>> of casts in GCC sources and 40,000 of the preferred "(type) x"
>>> style with the space.  That's a consistency of only 80%.  Is
>>> it worth documenting a preference for a convention that's so
>>> inconsistently followed?
>> Please do.  It's a fairly recent change -- I suspect some old code was
>> never fixed and some folks (perhaps myself) have that extraneous
>> whitespace in their muscle memory and still need to eliminate it.
> 
> Huh?  Spaces after casts are required, and make things much more
> readable.  This isn't recent.
> 
> A lot of old code writes spaces after single-character unary operators,
> too, which is ugly and less readable.  It's not recent that this is
> explicitly documented as wrong, either.
Sorry.  Got confused with something else.

jeff

Re: [PATCH/coding style] clarify pointers and operators

[Sandra Loosemore]

On 11/26/18 10:59 AM, Martin Sebor wrote:
> Martin suggested we update the Coding Conventions to describe
> the expected style for function declarations with a pointer
> return types, and for overloaded operators.  Below is the patch.
> 
> As an aside, regarding the space convention in casts: a crude
> grep search yields about 10,000 instances of the "(type)x" kinds
> of casts in GCC sources and 40,000 of the preferred "(type) x"
> style with the space.  That's a consistency of only 80%.  Is
> it worth documenting a preference for a convention that's so
> inconsistently followed?
> 
> Martin
> 
> Index: htdocs/codingconventions.html
> ===================================================================
> RCS file: /cvs/gcc/wwwdocs/htdocs/codingconventions.html,v
> retrieving revision 1.85
> diff -u -r1.85 codingconventions.html
> --- htdocs/codingconventions.html    30 Sep 2018 14:38:46 -0000    1.85
> +++ htdocs/codingconventions.html    26 Nov 2018 17:59:21 -0000
> @@ -803,12 +803,17 @@
>     <td><code>- x</code></td>
>   </tr><tr>
>     <td class="left">cast</td>
> -  <td class="right"><code>(foo) x</code></td>
> -  <td><code>(foo)x</code></td>
> +  <td class="right"><code>(type) x</code></td>
> +  <td><code>(type)x</code></td>
>   </tr><tr>
> -  <td class="left">pointer dereference</td>
> -  <td class="right"><code>*x</code></td>
> -  <td><code>* x</code></td>
> +  <td class="left">pointer cast</td>
> +  <td class="right"><code>(type *) x</code></td>
> +  <td><code>(type*)x</code></td>
> +</tr>
> +</tr><tr>
> +  <td class="left">pointer return type</td>
> +  <td class="right"><code>type *f (void)</code></td>
> +  <td><code>type* f (void)</code></td>
>   </tr>
>   </table>
> 
> @@ -992,6 +997,21 @@
>   <a href="codingrationale.html#overoper">Rationale and Discussion</a>
>   </p>
> 
> +<p>
> +Note: in declarations of operator functions or in invocations of
> +such functions that involve the keyword <code>operator</code>

This sentence would be more readable with a comma inserted here (after 
<code>operator</code>), I think.

> +the full name of the operator should be considered as including
> +the keyword with no spaces in between the keyowrd and the operator

s/keyowrd/keyword/

> +token.  Thus, the expected format of a declaration of an operator
> +is<pre>
> +    T &amp;operator== (const T &amp; const T &amp);
> +</pre>
> +and not for example<pre>

s/ for example//

> +    T &amp;operator == (const T &amp; const T &amp);
> +</pre>
> +(with the space between <code>operator</code> and <code>==</code>).
> +</p>
> +
> 
>   <h4 id="Default">Default Arguments</h4>
> 
> 

-Sandra

Re: [PATCH/coding style] clarify pointers and operators

[Martin Sebor]

On 11/27/18 10:20 PM, Sandra Loosemore wrote:
> On 11/26/18 10:59 AM, Martin Sebor wrote:
>> Martin suggested we update the Coding Conventions to describe
>> the expected style for function declarations with a pointer
>> return types, and for overloaded operators.  Below is the patch.
>>
>> As an aside, regarding the space convention in casts: a crude
>> grep search yields about 10,000 instances of the "(type)x" kinds
>> of casts in GCC sources and 40,000 of the preferred "(type) x"
>> style with the space.  That's a consistency of only 80%.  Is
>> it worth documenting a preference for a convention that's so
>> inconsistently followed?
>>
>> Martin
>>
>> Index: htdocs/codingconventions.html
>> ===================================================================
>> RCS file: /cvs/gcc/wwwdocs/htdocs/codingconventions.html,v
>> retrieving revision 1.85
>> diff -u -r1.85 codingconventions.html
>> --- htdocs/codingconventions.html    30 Sep 2018 14:38:46 -0000    1.85
>> +++ htdocs/codingconventions.html    26 Nov 2018 17:59:21 -0000
>> @@ -803,12 +803,17 @@
>>     <td><code>- x</code></td>
>>   </tr><tr>
>>     <td class="left">cast</td>
>> -  <td class="right"><code>(foo) x</code></td>
>> -  <td><code>(foo)x</code></td>
>> +  <td class="right"><code>(type) x</code></td>
>> +  <td><code>(type)x</code></td>
>>   </tr><tr>
>> -  <td class="left">pointer dereference</td>
>> -  <td class="right"><code>*x</code></td>
>> -  <td><code>* x</code></td>
>> +  <td class="left">pointer cast</td>
>> +  <td class="right"><code>(type *) x</code></td>
>> +  <td><code>(type*)x</code></td>
>> +</tr>
>> +</tr><tr>
>> +  <td class="left">pointer return type</td>
>> +  <td class="right"><code>type *f (void)</code></td>
>> +  <td><code>type* f (void)</code></td>
>>   </tr>
>>   </table>
>>
>> @@ -992,6 +997,21 @@
>>   <a href="codingrationale.html#overoper">Rationale and Discussion</a>
>>   </p>
>>
>> +<p>
>> +Note: in declarations of operator functions or in invocations of
>> +such functions that involve the keyword <code>operator</code>
> 
> This sentence would be more readable with a comma inserted here (after 
> <code>operator</code>), I think.
> 
>> +the full name of the operator should be considered as including
>> +the keyword with no spaces in between the keyowrd and the operator
> 
> s/keyowrd/keyword/
> 
>> +token.  Thus, the expected format of a declaration of an operator
>> +is<pre>
>> +    T &amp;operator== (const T &amp; const T &amp);
>> +</pre>
>> +and not for example<pre>
> 
> s/ for example//
> 
>> +    T &amp;operator == (const T &amp; const T &amp);
>> +</pre>
>> +(with the space between <code>operator</code> and <code>==</code>).
>> +</p>
>> +

I've made these changes.

Thanks
Martin

Re: [PATCH/coding style] clarify pointers and operators

[Richard Sandiford]

Thanks for doing this,

Martin Sebor <msebor@gmail.com> writes:
> Martin suggested we update the Coding Conventions to describe
> the expected style for function declarations with a pointer
> return types, and for overloaded operators.  Below is the patch.
>
> As an aside, regarding the space convention in casts: a crude
> grep search yields about 10,000 instances of the "(type)x" kinds
> of casts in GCC sources and 40,000 of the preferred "(type) x"
> style with the space.  That's a consistency of only 80%.  Is
> it worth documenting a preference for a convention that's so
> inconsistently followed?

Just to be sure, does that grep include things like the go frontend
and its GCC interface, which deliberately don't follow GNU conventions?
A crude grep for me gives 92% consistency in gcc/* itself (excluding
subdirectories), although that's still disappointingly low...

Thanks,
Richard

Re: [PATCH/coding style] clarify pointers and operators

[Segher Boessenkool]

On Wed, Dec 05, 2018 at 10:04:56AM +0000, Richard Sandiford wrote:
> Martin Sebor <msebor@gmail.com> writes:
> > Martin suggested we update the Coding Conventions to describe
> > the expected style for function declarations with a pointer
> > return types, and for overloaded operators.  Below is the patch.
> >
> > As an aside, regarding the space convention in casts: a crude
> > grep search yields about 10,000 instances of the "(type)x" kinds
> > of casts in GCC sources and 40,000 of the preferred "(type) x"
> > style with the space.  That's a consistency of only 80%.  Is
> > it worth documenting a preference for a convention that's so
> > inconsistently followed?
> 
> Just to be sure, does that grep include things like the go frontend
> and its GCC interface, which deliberately don't follow GNU conventions?
> A crude grep for me gives 92% consistency in gcc/* itself (excluding
> subdirectories), although that's still disappointingly low...

I get:

$ grep '([a-zA-Z_][a-zA-Z0-9_]*)[a-zA-Z_]' *.[ch]|wc -l
454
$ grep '([a-zA-Z_][a-zA-Z0-9_]*) ' *.[ch]|wc -l
28690

(that's gcc/*.[ch]).  About 1.6%, not so terrible.

$ grep '([a-zA-Z_][a-zA-Z0-9_]*\( \?\*\+\)\?)[a-zA-Z_]' *.[ch]|wc -l
631
$ grep '([a-zA-Z_][a-zA-Z0-9_]*\( \?\*\+\)\?) ' *.[ch]|wc -l
29426

With pointer casts it is worse, but still only about 2.2%.

Files other than *.[ch] will probably have many more false hits?


Segher

Re: [PATCH/coding style] clarify pointers and operators

[Segher Boessenkool]

On Wed, Dec 05, 2018 at 11:37:27AM -0600, Segher Boessenkool wrote:
> On Wed, Dec 05, 2018 at 10:04:56AM +0000, Richard Sandiford wrote:
> > Martin Sebor <msebor@gmail.com> writes:
> > > Martin suggested we update the Coding Conventions to describe
> > > the expected style for function declarations with a pointer
> > > return types, and for overloaded operators.  Below is the patch.
> > >
> > > As an aside, regarding the space convention in casts: a crude
> > > grep search yields about 10,000 instances of the "(type)x" kinds
> > > of casts in GCC sources and 40,000 of the preferred "(type) x"
> > > style with the space.  That's a consistency of only 80%.  Is
> > > it worth documenting a preference for a convention that's so
> > > inconsistently followed?
> > 
> > Just to be sure, does that grep include things like the go frontend
> > and its GCC interface, which deliberately don't follow GNU conventions?
> > A crude grep for me gives 92% consistency in gcc/* itself (excluding
> > subdirectories), although that's still disappointingly low...
> 
> I get:
> 
> $ grep '([a-zA-Z_][a-zA-Z0-9_]*)[a-zA-Z_]' *.[ch]|wc -l
> 454
> $ grep '([a-zA-Z_][a-zA-Z0-9_]*) ' *.[ch]|wc -l
> 28690
> 
> (that's gcc/*.[ch]).  About 1.6%, not so terrible.
> 
> $ grep '([a-zA-Z_][a-zA-Z0-9_]*\( \?\*\+\)\?)[a-zA-Z_]' *.[ch]|wc -l
> 631
> $ grep '([a-zA-Z_][a-zA-Z0-9_]*\( \?\*\+\)\?) ' *.[ch]|wc -l
> 29426
> 
> With pointer casts it is worse, but still only about 2.2%.
> 
> Files other than *.[ch] will probably have many more false hits?

Ugh.

$ grep '([a-zA-Z_][a-zA-Z0-9_]*\( \?\*\+\)\?)[a-zA-Z_]' *.[ch]|wc -l
631
$ grep '([a-zA-Z_][a-zA-Z0-9_]*\( \?\*\+\)\?) [a-zA-Z_]' *.[ch]|wc -l
3875

(REs, like all sharp tools, are dangerous).

14%?  Ouch.  And lamely filtering out some comments makes it only worse.


Segher

Re: [PATCH/coding style] clarify pointers and operators

[Martin Sebor]

On 12/5/18 3:04 AM, Richard Sandiford wrote:
> Thanks for doing this,
> 
> Martin Sebor <msebor@gmail.com> writes:
>> Martin suggested we update the Coding Conventions to describe
>> the expected style for function declarations with a pointer
>> return types, and for overloaded operators.  Below is the patch.
>>
>> As an aside, regarding the space convention in casts: a crude
>> grep search yields about 10,000 instances of the "(type)x" kinds
>> of casts in GCC sources and 40,000 of the preferred "(type) x"
>> style with the space.  That's a consistency of only 80%.  Is
>> it worth documenting a preference for a convention that's so
>> inconsistently followed?
> 
> Just to be sure, does that grep include things like the go frontend
> and its GCC interface, which deliberately don't follow GNU conventions?
> A crude grep for me gives 92% consistency in gcc/* itself (excluding
> subdirectories), although that's still disappointingly low...

The only thing I excluded was tests.  I don't have the find/grep
script around anymore but I think this should be pretty close:

   $ find git-svn/gcc -name "*.[ch]" ! -path "*/testsuite/*" | xargs 
grep ") [a-zA-Z_0-9]" | wc -l
   42391

   $ find git-svn/gcc -name "*.[ch]" ! -path "*/testsuite/*" | xargs 
grep ")[a-zA-Z_0-9]" | wc -l
   9712

As I said, it was only a crude search so I'm sure it includes
stuff that's outside the policy (e.g., comments).

Martin