[RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 2111 bytes --]

Hi!

;-P Jakub, thanks for furnishing me a fit occasion here:

On 2019-12-05T16:15:15+0100, Jakub Jelinek <jakub@redhat.com> wrote:
> [...] much more indented though, but you could
> use a temporary, like:
> 		      tree nullarg = null_pointer_node;

I object to cluttering the code by introducing temporary variables/names
just for the sake of a few characters of screen width.  Even if located
close lexically, when reading the following code you still have to trace
back from the 'nullarg' usage to its 'null_pointer_node' definition in
order to figure out what a 'nullarg' might be:

> 		      if (present)
> 			ptr
> 			  = gfc_build_conditional_assign_expr (block, present,
> 							       ptr, nullarg);

> Another option would be shorten the name of the function, say
> s/conditional/cond/.

Likewise I object to "crippling" identifier names like that just for the
sake of a few characters of screen width.  (Here of course, "cond", or
the existing "expr" might be fine abbreviations, but my point is about
the general case.)

> There were some discussions about lifting the 80 column restriction and bump
> it to something like +-130, but nothing happened yet.

Indeed.  :-)

In the relevant session at the GNU Tools Cauldron 2019, Michael Meissner
stated that even he is not using a 80 x 24 terminal anymore, and that
should tell us something.  ;-)

So, I formally propose that we lift this characters per line restriction
from IBM punch card (80) to mainframe line printer (132).

Nonwithstanding that, we should try to not overstrain that; deep
indentation often is a sign that code should be split out into a separate
function, for example.  My point is just to avoid things like the two
examples cited above.

Also, I'm not proposing any mass-reformatting of the existing code, or
re-writing all "expr" into "expression".

Tasks:

  - Discussion.
  - Get agreement/make a decision (by means still to be determined).
  - Put suitable Emacs/Vim configuration files into the source tree?
  - Update coding style guidelines.


Grüße
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Jakub Jelinek]

On Thu, Dec 05, 2019 at 04:46:45PM +0100, Thomas Schwinge wrote:
> Hi!
> 
> ;-P Jakub, thanks for furnishing me a fit occasion here:
> 
> On 2019-12-05T16:15:15+0100, Jakub Jelinek <jakub@redhat.com> wrote:
> > [...] much more indented though, but you could
> > use a temporary, like:
> > 		      tree nullarg = null_pointer_node;
> 
> I object to cluttering the code by introducing temporary variables/names
> just for the sake of a few characters of screen width.  Even if located
> close lexically, when reading the following code you still have to trace
> back from the 'nullarg' usage to its 'null_pointer_node' definition in
> order to figure out what a 'nullarg' might be:
> 
> > 		      if (present)
> > 			ptr
> > 			  = gfc_build_conditional_assign_expr (block, present,
> > 							       ptr, nullarg);
> 
> > Another option would be shorten the name of the function, say
> > s/conditional/cond/.
> 
> Likewise I object to "crippling" identifier names like that just for the
> sake of a few characters of screen width.  (Here of course, "cond", or
> the existing "expr" might be fine abbreviations, but my point is about
> the general case.)

The point about temporaries is general, and I believe they actually make
code much more readable.  Mostly about coding style like:
	t = fold_build2_loc (loc, code, fold_build2_loc (loc, code2,
							 something1,
							 something2),
			     fold_build2_loc (loc, code3, something3,
					      something4));
vs.
	tree op1 = fold_build2_loc (loc, code2, something1, something2);
	tree op2 = fold_build2_loc (loc, code3, something3, something4);
	t = fold_build2_loc (loc, code, op1, op2);
The above case is extreme in both being indented quite a lot (general rule
is to consider outlining something into a function then) and using
way too long function names.  If you look at the earlier suggestion where
the code is indented reasonably, using the temporary there makes the code more
readable and shorter.

> 
> > There were some discussions about lifting the 80 column restriction and bump
> > it to something like +-130, but nothing happened yet.
> 
> Indeed.  :-)
> 
> In the relevant session at the GNU Tools Cauldron 2019, Michael Meissner
> stated that even he is not using a 80 x 24 terminal anymore, and that
> should tell us something.  ;-)
> 
> So, I formally propose that we lift this characters per line restriction
> from IBM punch card (80) to mainframe line printer (132).

Such a proposal would need to be accompanied with a wwwdocs
codingconventions.html patch and contrib/check_GNU_style.{sh,py} patch I
guess ;)

	Jakub

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Joseph Myers]

On Thu, 5 Dec 2019, Thomas Schwinge wrote:

> In the relevant session at the GNU Tools Cauldron 2019, Michael Meissner
> stated that even he is not using a 80 x 24 terminal anymore, and that
> should tell us something.  ;-)
> 
> So, I formally propose that we lift this characters per line restriction
> from IBM punch card (80) to mainframe line printer (132).

I thought these line lengths were based on readability studies suggesting 
lengths that lines shorter than 80 columns were more readable?

Longer lines mean less space for multiple terminal / editor windows 
side-by-side to look at different pieces of code.  I don't think that's an 
improvement.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Paul Koning]

> On Dec 5, 2019, at 11:17 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> 
> On Thu, 5 Dec 2019, Thomas Schwinge wrote:
> 
>> In the relevant session at the GNU Tools Cauldron 2019, Michael Meissner
>> stated that even he is not using a 80 x 24 terminal anymore, and that
>> should tell us something.  ;-)
>> 
>> So, I formally propose that we lift this characters per line restriction
>> from IBM punch card (80) to mainframe line printer (132).
> 
> I thought these line lengths were based on readability studies suggesting 
> lengths that lines shorter than 80 columns were more readable?

That's certainly a general rule.  There is a reason why books aren't wide, and why newspapers have columns.  The eye can't deal well with long lines.  So while 132 column lines are certainly possible with modern computers, it doesn't mean they are desirable.

	paul

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Jeff Law]

On 12/5/19 9:24 AM, Paul Koning wrote:
> 
> 
>> On Dec 5, 2019, at 11:17 AM, Joseph Myers <joseph@codesourcery.com>
>> wrote:
>> 
>> On Thu, 5 Dec 2019, Thomas Schwinge wrote:
>> 
>>> In the relevant session at the GNU Tools Cauldron 2019, Michael
>>> Meissner stated that even he is not using a 80 x 24 terminal
>>> anymore, and that should tell us something.  ;-)
>>> 
>>> So, I formally propose that we lift this characters per line
>>> restriction from IBM punch card (80) to mainframe line printer
>>> (132).
>> 
>> I thought these line lengths were based on readability studies
>> suggesting lengths that lines shorter than 80 columns were more
>> readable?
> 
> That's certainly a general rule.  There is a reason why books aren't
> wide, and why newspapers have columns.  The eye can't deal well with
> long lines.  So while 132 column lines are certainly possible with
> modern computers, it doesn't mean they are desirable.
I'd like to see the restriction relaxed.  THe 80 column limit really
presents readability problems and excessive expression wrapping to
accommodate the limit.  132 seems like a very reasonable compromise.

My biggest worry with moving to 132 columns is that it will discourage
refactoring when indention levels cause excessive wrapping.

Jeff

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Michael Matz]

Hello,

(oh a flame bait :) )

On Thu, 5 Dec 2019, Thomas Schwinge wrote:

> So, I formally propose that we lift this characters per line restriction
> from IBM punch card (80) to mainframe line printer (132).
> 
> Tasks:
> 
>   - Discussion.

I object to cluttering code in excuse for using sensible function names or 
temporaries that otherwise can help clearing up code.  Using 132-char 
lines is cluttering code:
- long lines are harder to read/grasp: vertical eye movement is easier 
  than horizontal, and source code should be optimized for 
  reading, not writing
- long lines make it impossible to have two files next to each other at a 
  comfortable font size
- long lines are incompatible with existing netiquette re emails, for 
  instance

So, at least for me, that my terminals are 80 wide (but not x24) has 
multiple reasons, and the _least_ of it is because that's what punch cards 
had.


Ciao,
Michael.

Re: [RFC] Characters per line: from punch card (80) to line printer (132)

[Florian Weimer]

* Paul Koning:

> That's certainly a general rule.  There is a reason why books aren't
> wide, and why newspapers have columns.  The eye can't deal well with
> long lines.  So while 132 column lines are certainly possible with
> modern computers, it doesn't mean they are desirable.

If the line starts at column 40 or so, I don't think readability suffers
too much if it goes to column 100.  If it starts at column 2, then it
might be problematic.

Frequent long lines reduce the usefulness of side-by-side diff viewers.

And there are those of us who use screens in portrait mode, following
the rule that a good function is not longer than a single screen.  1080
pixels give you 8 pixels per character, which isn't that much.

Thanks,
Florian

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Jonathan Wakely]

On Thu, 5 Dec 2019 at 16:44, Michael Matz <matz@suse.de> wrote:
>
> Hello,
>
> (oh a flame bait :) )
>
> On Thu, 5 Dec 2019, Thomas Schwinge wrote:
>
> > So, I formally propose that we lift this characters per line restriction
> > from IBM punch card (80) to mainframe line printer (132).
> >
> > Tasks:
> >
> >   - Discussion.
>
> I object to cluttering code in excuse for using sensible function names or
> temporaries that otherwise can help clearing up code.  Using 132-char
> lines is cluttering code:
> - long lines are harder to read/grasp: vertical eye movement is easier
>   than horizontal, and source code should be optimized for
>   reading, not writing
> - long lines make it impossible to have two files next to each other at a
>   comfortable font size
> - long lines are incompatible with existing netiquette re emails, for
>   instance
>
> So, at least for me, that my terminals are 80 wide (but not x24) has
> multiple reasons, and the _least_ of it is because that's what punch cards
> had.

C++17 introduces a nice feature, with rationale similar to declaring
variables in a for-loop init-statement:

if (auto var = foo(); bar(var))

The variable is only in scope for the block where you need it, just
like a for-loop.

Unfortunately nearly every time I've tried to use this recently, I've
found it's impossible in 80 columns, e.g. this from yesterday:

    if (auto __c = __builtin_memcmp(&*__first1, &*__first2, __len) <=>
0; __c != 0)
      return __c;

When you're forced to uglify every variable with a leading __ you run
out of characters pretty damn quickly.

I can either not use the feature (and have the variable defined in a
larger scope than it needs to be) or add fairly arbitrary line breaks:

            if (auto __c
            = __builtin_memcmp(&*__first1, &*__first2, __len)
            <=> 0; __c != 0)
              return __c;

or try to give the variables shorter (and less meaningful) names.
Adding line breaks or picking shorter names doesn't help readability.
So I end up not using the feature.

I'm loosely in favour of relaxing the rule for libstdc++ code. I don't
really care what the rest of GCC looks like ;-)

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[N.M. Maclaren]

On Dec 5 2019, Michael Matz wrote:
>
>(oh a flame bait :) )

Quite.  I shall try not to make it too much worse, but there's another point
that needs mentioning.

I find long names hard to read, with either short or long lines, especially
when combined with variants like negotiate_twisty_little_passage, and
negotiate_little_twisty_passage.  And extending line lengths will probably
encourage the use of longer names.

As people say, there are conficting requirements, but I side with you in
preferring 80 column lines.  Actually, I tend to use less, but that's
because of the (still current) frequency with which utilities make a mess
of lines exactly 80 columns long in an 80 column field.

Regards,
Nick Maclaren.

Re: [RFC] Characters per line: from punch card (80) to line printer (132)

[Andrew Stubbs]

On 05/12/2019 16:17, Joseph Myers wrote:
> Longer lines mean less space for multiple terminal / editor windows
> side-by-side to look at different pieces of code.  I don't think that's an
> improvement.

Here's a data-point ....

My 1920 pixel-wide screen, in the default font, allows 239 columns; not 
enough for two 130-wide editors.  Especially not with line numbers and 
"gutter" columns.

On the other hand, 80 columns does tend to cause some formatting 
contortions, with long function names and deeper indentations.

I think a nice round 100 would be a good compromise.

Andrew

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Marek Polacek]

On Thu, Dec 05, 2019 at 05:03:43PM +0000, Jonathan Wakely wrote:
> On Thu, 5 Dec 2019 at 16:44, Michael Matz <matz@suse.de> wrote:
> >
> > Hello,
> >
> > (oh a flame bait :) )
> >
> > On Thu, 5 Dec 2019, Thomas Schwinge wrote:
> >
> > > So, I formally propose that we lift this characters per line restriction
> > > from IBM punch card (80) to mainframe line printer (132).
> > >
> > > Tasks:
> > >
> > >   - Discussion.
> >
> > I object to cluttering code in excuse for using sensible function names or
> > temporaries that otherwise can help clearing up code.  Using 132-char
> > lines is cluttering code:
> > - long lines are harder to read/grasp: vertical eye movement is easier
> >   than horizontal, and source code should be optimized for
> >   reading, not writing
> > - long lines make it impossible to have two files next to each other at a
> >   comfortable font size
> > - long lines are incompatible with existing netiquette re emails, for
> >   instance
> >
> > So, at least for me, that my terminals are 80 wide (but not x24) has
> > multiple reasons, and the _least_ of it is because that's what punch cards
> > had.
> 
> C++17 introduces a nice feature, with rationale similar to declaring
> variables in a for-loop init-statement:
> 
> if (auto var = foo(); bar(var))
> 
> The variable is only in scope for the block where you need it, just
> like a for-loop.
> 
> Unfortunately nearly every time I've tried to use this recently, I've
> found it's impossible in 80 columns, e.g. this from yesterday:
> 
>     if (auto __c = __builtin_memcmp(&*__first1, &*__first2, __len) <=>
> 0; __c != 0)
>       return __c;
> 
> When you're forced to uglify every variable with a leading __ you run
> out of characters pretty damn quickly.
> 
> I can either not use the feature (and have the variable defined in a
> larger scope than it needs to be) or add fairly arbitrary line breaks:
> 
>             if (auto __c
>             = __builtin_memcmp(&*__first1, &*__first2, __len)
>             <=> 0; __c != 0)
>               return __c;
> 
> or try to give the variables shorter (and less meaningful) names.
> Adding line breaks or picking shorter names doesn't help readability.
> So I end up not using the feature.
> 
> I'm loosely in favour of relaxing the rule for libstdc++ code. I don't
> really care what the rest of GCC looks like ;-)

Not using such a nice feature just because of formatting sounds really
shameful.  Would the compromise of 100 chars make things any better here?

Marek

Re: [RFC] Characters per line: from punch card (80) to line printer (132)

[Eric Gallager]

On 12/5/19, Andrew Stubbs <ams@codesourcery.com> wrote:
> On 05/12/2019 16:17, Joseph Myers wrote:
>> Longer lines mean less space for multiple terminal / editor windows
>> side-by-side to look at different pieces of code.  I don't think that's
>> an
>> improvement.
>
> Here's a data-point ....
>
> My 1920 pixel-wide screen, in the default font, allows 239 columns; not
> enough for two 130-wide editors.  Especially not with line numbers and
> "gutter" columns.
>
> On the other hand, 80 columns does tend to cause some formatting
> contortions, with long function names and deeper indentations.
>
> I think a nice round 100 would be a good compromise.

Here's mine:

My 1280 pixel-wide screen allows 179 columns, which comes to 89.5
columns when divided by 2. I think rounding up to 90 columns would be
a good compromise, although if that's too small, 100 is good as well.

>
> Andrew
>

Re: [RFC] Characters per line: from punch card (80) to line printer (132)

[Robin Curtis]

My IBM Selectric golfball electronic printer only does 90 characters on A4 in portrait mode………(at 10 cps) 

(as for my all electric TELEX Teleprinter machine !) 

Is this debate for real ?!  - or is this a Christmas spoof ? 

External observer…..keep up the great work. :)

(while I punch out a few more 80 column cards). 



> On 5 Dec 2019, at 17:55, Andrew Stubbs <ams@codesourcery.com> wrote:
> 
> On 05/12/2019 16:17, Joseph Myers wrote:
>> Longer lines mean less space for multiple terminal / editor windows
>> side-by-side to look at different pieces of code.  I don't think that's an
>> improvement.
> 
> Here's a data-point ....
> 
> My 1920 pixel-wide screen, in the default font, allows 239 columns; not enough for two 130-wide editors.  Especially not with line numbers and "gutter" columns.
> 
> On the other hand, 80 columns does tend to cause some formatting contortions, with long function names and deeper indentations.
> 
> I think a nice round 100 would be a good compromise.
> 
> Andrew

Re: [RFC] Characters per line: from punch card (80) to line printer (132)

[Martin Sebor]

On 12/5/19 8:46 AM, Thomas Schwinge wrote:
> Hi!
> 
> ;-P Jakub, thanks for furnishing me a fit occasion here:
> 
> On 2019-12-05T16:15:15+0100, Jakub Jelinek <jakub@redhat.com> wrote:
>> [...] much more indented though, but you could
>> use a temporary, like:
>> 		      tree nullarg = null_pointer_node;
> 
> I object to cluttering the code by introducing temporary variables/names
> just for the sake of a few characters of screen width.  Even if located
> close lexically, when reading the following code you still have to trace
> back from the 'nullarg' usage to its 'null_pointer_node' definition in
> order to figure out what a 'nullarg' might be:
> 
>> 		      if (present)
>> 			ptr
>> 			  = gfc_build_conditional_assign_expr (block, present,
>> 							       ptr, nullarg);

The snippet of code above looks like it might be the symptom
of another common problem: deeply nested conditionals, case
statements, or loops in very large functions.  Those usually
make it much harder to follow code than local variables or
expressions that are broken up to fit the width limit.
Shorter functions typically also means fewer local variables.

One thing I find improves readability in functions with many
local variables is declaring const those that don't change
after initialization.  That also enforces the initialization-
on-declaration coding style, and can result in more efficient
code.

Another solution that might help in this context is default
function arguments: if the last argument may be null, making
it the default in the function declaration avoids having to
pass it explicitly.

> 
>> Another option would be shorten the name of the function, say
>> s/conditional/cond/.

As long as it doesn't compromise readability this sounds like
a good suggestion for a change to the function above.  _cond_
is no less clear or descriptive than _conditional_, similarly
to _expr vs _expression.

> 
> Likewise I object to "crippling" identifier names like that just for the
> sake of a few characters of screen width.  (Here of course, "cond", or
> the existing "expr" might be fine abbreviations, but my point is about
> the general case.)
> 
>> There were some discussions about lifting the 80 column restriction and bump
>> it to something like +-130, but nothing happened yet.
> 
> Indeed.  :-)
> 
> In the relevant session at the GNU Tools Cauldron 2019, Michael Meissner
> stated that even he is not using a 80 x 24 terminal anymore, and that
> should tell us something.  ;-)
> 
> So, I formally propose that we lift this characters per line restriction
> from IBM punch card (80) to mainframe line printer (132).

I'm not a fan of rigid rules, especially those that are subject
to personal style preferences.  At the same time I wouldn't like
to see lines become as long as this as the norm.  As others,
I have windows om my desktop arranged in a way to maximize screen
real estate: three columns of editor/debugger and a couple of
terminals on of top of the other.  They only fit because they're
all 80 characters wide.  But more important:

> deep
> indentation often is a sign that code should be split out into a separate
> function, for example.

Exactly.

Martin

   My point is just to avoid things like the two
> examples cited above.
> 
> Also, I'm not proposing any mass-reformatting of the existing code, or
> re-writing all "expr" into "expression".
> 
> Tasks:
> 
>    - Discussion.
>    - Get agreement/make a decision (by means still to be determined).
>    - Put suitable Emacs/Vim configuration files into the source tree?
>    - Update coding style guidelines.
> 
> 
> Grüße
>   Thomas
> 

Re: [RFC] Characters per line: from punch card (80) to line printer (132)

[James Secan]

All,

As a certified Old Guy (coding in FORTRAN since 1967) my default mode is to stop at 72 characters.  It would be nice to push the max out a bit, but I’ll most likely keep my lines (and my function and variable names!) short.  As always, brevity is the soul of wit.

And yes, please keep up the good work.

Jim
3222 NE 89th St
Seattle, WA 98115
(206) 430-0109

> On Dec 5, 2019, at 10:21 AM, Robin Curtis <Curtis@geoscience.co.uk> wrote:
> 
> Is this debate for real ?!  - or is this a Christmas spoof ? 
> 
> External observer…..keep up the great work. :)
> 
> (while I punch out a few more 80 column cards). 

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Segher Boessenkool]

Hi!

On Thu, Dec 05, 2019 at 05:03:43PM +0000, Jonathan Wakely wrote:
> C++17 introduces a nice feature, with rationale similar to declaring
> variables in a for-loop init-statement:
> 
> if (auto var = foo(); bar(var))

Similar to GNU C statement expressions, which are *also* only a good
idea to use in limited cases.

> The variable is only in scope for the block where you need it, just
> like a for-loop.
> 
> Unfortunately nearly every time I've tried to use this recently, I've
> found it's impossible in 80 columns, e.g. this from yesterday:
> 
>     if (auto __c = __builtin_memcmp(&*__first1, &*__first2, __len) <=>
> 0; __c != 0)
>       return __c;
> 
> When you're forced to uglify every variable with a leading __ you run
> out of characters pretty damn quickly.

If using this "nice feature" forces you to uglify your code, then maybe
it is not such a nice feature, and you should not use it.

If you have issues with scoping your functions are WAY too long already.


Segher

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Segher Boessenkool]

On Thu, Dec 05, 2019 at 04:44:21PM +0000, Michael Matz wrote:
> (oh a flame bait :) )

I will blissfully ignore that warning.

> On Thu, 5 Dec 2019, Thomas Schwinge wrote:
> I object to cluttering code in excuse for using sensible function names or 
> temporaries that otherwise can help clearing up code.  Using 132-char 
> lines is cluttering code:
> - long lines are harder to read/grasp: vertical eye movement is easier 
>   than horizontal, and source code should be optimized for 
>   reading, not writing
> - long lines make it impossible to have two files next to each other at a 
>   comfortable font size
> - long lines are incompatible with existing netiquette re emails, for 
>   instance
> 
> So, at least for me, that my terminals are 80 wide (but not x24) has 
> multiple reasons, and the _least_ of it is because that's what punch cards 
> had.

I agree with all of this.

If you have a hard time writing nicely readable code in 80 columns, you
will have a much harder time still using more columns.  80 is somewhat
too long already (~60 is better), but we have indents in source code,
so that's alright.  And you should not indent that far, so this all
works out splendidly.


Segher

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Segher Boessenkool]

On Thu, Dec 05, 2019 at 05:04:12PM +0100, Jakub Jelinek wrote:
> On Thu, Dec 05, 2019 at 04:46:45PM +0100, Thomas Schwinge wrote:
> > On 2019-12-05T16:15:15+0100, Jakub Jelinek <jakub@redhat.com> wrote:
> > > [...] much more indented though, but you could
> > > use a temporary, like:
> > > 		      tree nullarg = null_pointer_node;
> > 
> > I object to cluttering the code by introducing temporary variables/names
> > just for the sake of a few characters of screen width.  Even if located
> > close lexically, when reading the following code you still have to trace
> > back from the 'nullarg' usage to its 'null_pointer_node' definition in
> > order to figure out what a 'nullarg' might be:
> > 
> > > 		      if (present)
> > > 			ptr
> > > 			  = gfc_build_conditional_assign_expr (block, present,
> > > 							       ptr, nullarg);
> > 
> > > Another option would be shorten the name of the function, say
> > > s/conditional/cond/.
> > 
> > Likewise I object to "crippling" identifier names like that just for the
> > sake of a few characters of screen width.  (Here of course, "cond", or
> > the existing "expr" might be fine abbreviations, but my point is about
> > the general case.)
> 
> The point about temporaries is general, and I believe they actually make
> code much more readable.  Mostly about coding style like:
> 	t = fold_build2_loc (loc, code, fold_build2_loc (loc, code2,
> 							 something1,
> 							 something2),
> 			     fold_build2_loc (loc, code3, something3,
> 					      something4));
> vs.
> 	tree op1 = fold_build2_loc (loc, code2, something1, something2);
> 	tree op2 = fold_build2_loc (loc, code3, something3, something4);
> 	t = fold_build2_loc (loc, code, op1, op2);

Yes.  And the names, even if they do not say much, *do* say enough to
help comprehending the code.  They help structure it.

> The above case is extreme in both being indented quite a lot (general rule
> is to consider outlining something into a function then)

I hope you mean actual factoring, not just outlining :-)  If you pick good
factors you can give them good names, too.

Good names help reading the code.  And on the other hand, when it is hard
to come up with a good name for a piece of code, it is probably not chosen
as a good factor anyway!

> and using
> way too long function names.  If you look at the earlier suggestion where
> the code is indented reasonably, using the temporary there makes the code more
> readable and shorter.

Yup.


Segher

Re: [RFC] Characters per line: from punch card (80) to line printer (132)

[Segher Boessenkool]

On Thu, Dec 05, 2019 at 11:54:04AM -0700, Martin Sebor wrote:
> >>		      if (present)
> >>			ptr
> >>			  = gfc_build_conditional_assign_expr (block, 
> >>			  present,
> >>							       ptr, nullarg);
> 
> The snippet of code above looks like it might be the symptom
> of another common problem: deeply nested conditionals, case
> statements, or loops in very large functions.  Those usually
> make it much harder to follow code than local variables or
> expressions that are broken up to fit the width limit.
> Shorter functions typically also means fewer local variables.

Yes, and you only get problems that you do not know which var is which,
or you do not know what value it is set to because it was set some 2800
(or 40 or whatever) lines ago, in WAY too long functions.

All those problems do not exist in well-factored code.  The point is not
to have short routines: the point is to not have too much (external)
complexity per routine.  A routine should ideally only do one thing, and
its name should describe what it does.


Segher

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Marek Polacek]

On Thu, Dec 05, 2019 at 02:06:50PM -0600, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Dec 05, 2019 at 05:03:43PM +0000, Jonathan Wakely wrote:
> > C++17 introduces a nice feature, with rationale similar to declaring
> > variables in a for-loop init-statement:
> > 
> > if (auto var = foo(); bar(var))
> 
> Similar to GNU C statement expressions, which are *also* only a good
> idea to use in limited cases.
> 
> > The variable is only in scope for the block where you need it, just
> > like a for-loop.
> > 
> > Unfortunately nearly every time I've tried to use this recently, I've
> > found it's impossible in 80 columns, e.g. this from yesterday:
> > 
> >     if (auto __c = __builtin_memcmp(&*__first1, &*__first2, __len) <=>
> > 0; __c != 0)
> >       return __c;
> > 
> > When you're forced to uglify every variable with a leading __ you run
> > out of characters pretty damn quickly.
> 
> If using this "nice feature" forces you to uglify your code, then maybe
> it is not such a nice feature, and you should not use it.

I disagree, it is a nice feature, without quotes.  It's Good Style not to
leak variables into enclosing scopes.

> If you have issues with scoping your functions are WAY too long already.

I don't think that's the case here.

--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Jason Merrill]

On Thu, Dec 5, 2019 at 11:51 AM Michael Matz <matz@suse.de> wrote:

> Hello,
>
> (oh a flame bait :) )
>
> On Thu, 5 Dec 2019, Thomas Schwinge wrote:
>
> > So, I formally propose that we lift this characters per line restriction
> > from IBM punch card (80) to mainframe line printer (132).
> >
> > Tasks:
> >
> >   - Discussion.
>
> I object to cluttering code in excuse for using sensible function names or
> temporaries that otherwise can help clearing up code.  Using 132-char
> lines is cluttering code:
> - long lines are harder to read/grasp: vertical eye movement is easier
>   than horizontal, and source code should be optimized for
>   reading, not writing
> - long lines make it impossible to have two files next to each other at a
>   comfortable font size
> - long lines are incompatible with existing netiquette re emails, for
>   instance
>
> So, at least for me, that my terminals are 80 wide (but not x24) has
> multiple reasons, and the _least_ of it is because that's what punch cards
> had.
>

Agreed.  I work with two side-by-side terminals, one 80x50 and the other as
wide as fits in the rest of the screen, which currently happens to be
111x50.

Jason

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Jonathan Wakely]

On Thu, 5 Dec 2019 at 20:07, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> Hi!
>
> On Thu, Dec 05, 2019 at 05:03:43PM +0000, Jonathan Wakely wrote:
> > C++17 introduces a nice feature, with rationale similar to declaring
> > variables in a for-loop init-statement:
> >
> > if (auto var = foo(); bar(var))
>
> Similar to GNU C statement expressions, which are *also* only a good
> idea to use in limited cases.
>
> > The variable is only in scope for the block where you need it, just
> > like a for-loop.
> >
> > Unfortunately nearly every time I've tried to use this recently, I've
> > found it's impossible in 80 columns, e.g. this from yesterday:
> >
> >     if (auto __c = __builtin_memcmp(&*__first1, &*__first2, __len) <=>
> > 0; __c != 0)
> >       return __c;
> >
> > When you're forced to uglify every variable with a leading __ you run
> > out of characters pretty damn quickly.
>
> If using this "nice feature" forces you to uglify your code, then maybe
> it is not such a nice feature, and you should not use it.

The uglification has absolutely nothing to do with the 'if'
init-statement feature, all code in libstdc++ headers has to be
uglified, always. Blame the C preprocessor for that, not C++ features.

My point is that 80 characters runs out quicker when 10% of it goes on
visual noise that's only needed because the C preprocessor means we
can't have nice names.

> If you have issues with scoping your functions are WAY too long already.

I don't have issues with scoping, it's just good practice to limit the
scope to the minimum necessary.

The example I'm talking about is:
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/include/bits/stl_algobase.h;h=a2fd306e6d0cca579b510148ba1a7089e2b2f3a2;hb=HEAD#l1499

That function is 46 lines long, including a micro-optimisation to use
memcmpy when appropriate. About 15% of that is on assertions to help
users debug their mistakes. Another five lines are the compile-time
if-constexpr checks to decide when the optimisation is appropriate,
which result in deep nesting, but not because of any complex if-else
branches. It could be nested less deeply to reduce indentation by 4
spaces, but that would result in more template instantiations for
users, which would take more time and memory to compile. When you have
a million users including the header in every C++ program, little
things like that help. So the code is written in an unidiomatic way to
optimise for compilation speed, not readability.

But the end result is that the call to __min_cmp is indented about 25%
of the way into the available space, and that the FIRST line of
executable code in the function. The code is not way too long, and
writing it differently would compile slower. The constraints that
apply to that code are quite different to the internals of GCC, which
most users never see or even compile themselves.

(Yes, maybe libstdc++ should stop indenting everything by 2 columns
when it's inside a namespace, given that almost everything is inside
at least one level of namespace ... that might be a good idea even if
the column limit is extended to 100 or 132, but we'll still have all
the __ prefixes eating up space.)

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Segher Boessenkool]

On Thu, Dec 05, 2019 at 03:38:15PM -0500, Marek Polacek wrote:
> On Thu, Dec 05, 2019 at 02:06:50PM -0600, Segher Boessenkool wrote:
> > > When you're forced to uglify every variable with a leading __ you run
> > > out of characters pretty damn quickly.
> > 
> > If using this "nice feature" forces you to uglify your code, then maybe
> > it is not such a nice feature, and you should not use it.
> 
> I disagree, it is a nice feature, without quotes.  It's Good Style not to
> leak variables into enclosing scopes.

It *is* a quote, from Jonathan's mail.

Why is this Good Style?  (And according to who?)

> > If you have issues with scoping your functions are WAY too long already.
> 
> I don't think that's the case here.

Then it does not hurt to have a local that is visible in slightly longer
scope than necessary.

Simpler code is better code.


Segher

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Segher Boessenkool]

On Thu, Dec 05, 2019 at 08:56:35PM +0000, Jonathan Wakely wrote:
> On Thu, 5 Dec 2019 at 20:07, Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> > On Thu, Dec 05, 2019 at 05:03:43PM +0000, Jonathan Wakely wrote:
> > > C++17 introduces a nice feature, with rationale similar to declaring
> > > variables in a for-loop init-statement:

> > > Unfortunately nearly every time I've tried to use this recently, I've
> > > found it's impossible in 80 columns, e.g. this from yesterday:
> > >
> > >     if (auto __c = __builtin_memcmp(&*__first1, &*__first2, __len) <=>
> > > 0; __c != 0)
> > >       return __c;
> > >
> > > When you're forced to uglify every variable with a leading __ you run
> > > out of characters pretty damn quickly.
> >
> > If using this "nice feature" forces you to uglify your code, then maybe
> > it is not such a nice feature, and you should not use it.
> 
> The uglification has absolutely nothing to do with the 'if'
> init-statement feature, all code in libstdc++ headers has to be
> uglified, always. Blame the C preprocessor for that, not C++ features.
> 
> My point is that 80 characters runs out quicker when 10% of it goes on
> visual noise that's only needed because the C preprocessor means we
> can't have nice names.

(Not sure where the preprocessor comes in, these underscores are just to
satisfy language rules afaics, but maybe you mean something else?)

Or you could write

    auto __c = (__builtin_memcmp(&*__first1, &*__first2, __len) <=> 0);
    if (__c)
      return __c;

which is much easier to read, to my eyes anyway.  And it is exactly the
same for the compiler.

Wrapping everything inside-out just for the artificial goal of having
things defined in slightly tighter scopes feels futile.


Segher

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Jonathan Wakely]

On Thu, 5 Dec 2019 at 22:19, Segher Boessenkool wrote:
>
> On Thu, Dec 05, 2019 at 08:56:35PM +0000, Jonathan Wakely wrote:
> > On Thu, 5 Dec 2019 at 20:07, Segher Boessenkool
> > <segher@kernel.crashing.org> wrote:
> > > On Thu, Dec 05, 2019 at 05:03:43PM +0000, Jonathan Wakely wrote:
> > > > C++17 introduces a nice feature, with rationale similar to declaring
> > > > variables in a for-loop init-statement:
>
> > > > Unfortunately nearly every time I've tried to use this recently, I've
> > > > found it's impossible in 80 columns, e.g. this from yesterday:
> > > >
> > > >     if (auto __c = __builtin_memcmp(&*__first1, &*__first2, __len) <=>
> > > > 0; __c != 0)
> > > >       return __c;
> > > >
> > > > When you're forced to uglify every variable with a leading __ you run
> > > > out of characters pretty damn quickly.
> > >
> > > If using this "nice feature" forces you to uglify your code, then maybe
> > > it is not such a nice feature, and you should not use it.
> >
> > The uglification has absolutely nothing to do with the 'if'
> > init-statement feature, all code in libstdc++ headers has to be
> > uglified, always. Blame the C preprocessor for that, not C++ features.
> >
> > My point is that 80 characters runs out quicker when 10% of it goes on
> > visual noise that's only needed because the C preprocessor means we
> > can't have nice names.
>
> (Not sure where the preprocessor comes in, these underscores are just to
> satisfy language rules afaics, but maybe you mean something else?)

The language rules are only necessary because of the preprocessor.
Users can define macros with names like "pred" and "cmp" before
including any standard library header, and because macros don't
respect lexical scope, that would break any standard library code
using "pred" and "cmp". So the std::lib has to use reserved names.

It's entirely due to the preprocessor that we can't use sane names for
local variables, or for any implementation detail in a header (unlike
in C, our helper types and functions are in a namespace so won't
collide with users' types and functions, but we still have to use
reserved names because the preprocessor doesn't respect namespaces).

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Jonathan Wakely]

On Thu, 5 Dec 2019 at 22:19, Segher Boessenkool wrote:
> Or you could write
>
>     auto __c = (__builtin_memcmp(&*__first1, &*__first2, __len) <=> 0);
>     if (__c)
>       return __c;
>
> which is much easier to read, to my eyes anyway.  And it is exactly the
> same for the compiler.

In this case yes, but not in general.

Given:

auto x = foo();
if (bar(x))
{ }
some_type y;

The destructor of x won't run until after y has been destroyed. That's
not at all identical to:

if (auto x = foo(); bar(x))
{ }
some_type y;

Please don't try to tell me how C++ works :-)

Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)

[Segher Boessenkool]

On Thu, Dec 05, 2019 at 10:37:33PM +0000, Jonathan Wakely wrote:
> On Thu, 5 Dec 2019 at 22:19, Segher Boessenkool wrote:
> > Or you could write
> >
> >     auto __c = (__builtin_memcmp(&*__first1, &*__first2, __len) <=> 0);
> >     if (__c)
> >       return __c;
> >
> > which is much easier to read, to my eyes anyway.  And it is exactly the
> > same for the compiler.
> 
> In this case yes, but not in general.
> 
> Given:
> 
> auto x = foo();
> if (bar(x))
> { }
> some_type y;
> 
> The destructor of x won't run until after y has been destroyed. That's
> not at all identical to:
> 
> if (auto x = foo(); bar(x))
> { }
> some_type y;
> 
> Please don't try to tell me how C++ works :-)

I don't, I wouldn't even *know* that.  But this is just the same as in C,
and I do know how to write good C code.

I don't think doing non-trivial things with constructors and destructors
(or anything else!) implicitly is a good idea at all, but that's an
altogether different subject.


Segher

Re: [RFC] Characters per line: from punch card (80) to line printer (132)

[Andrew Stubbs]

On 05/12/2019 18:21, Robin Curtis wrote:
> My IBM Selectric golfball electronic printer only does 90 characters on A4 in portrait mode………(at 10 cps)
> 
> (as for my all electric TELEX Teleprinter machine !)
> 
> Is this debate for real ?!  - or is this a Christmas spoof ?

I can't speak for the debate, but the pain is real.

Andrew