Re: An abridged "Writing C" for the gcc web pages

[Carlos O'Donell]

On 05/03/2016 06:39 PM, Bernd Schmidt wrote:
> On 05/03/2016 09:59 PM, Richard Sandiford wrote:
>> 
>> And sometimes there are multiple acceptable ways of writing the
>> same code. Which wins is an aesthetic choice, which tools tend to
>> be bad at handling. E.g. if a parameter list is too long for a
>> line, there's often a choice of how to balance the parameters
>> across multiple lines, with some being more readable than others.
>> I wouldn't want the one chosen by a formatting tool to be the only
>> acceptable one.
> 
> I have all the same reservations about using tools for this. We don't
> call it "style" for nothing, it is not purely mechanical and until we
> have general-purpose AI I don't believe computers can do a
> satisfactory job. I've seen attempts to do so in the past, and they
> have failed - you invariably end up with humans contorting their code
> to have it pass the checker, which is entirely counterproductive.

This would be silly. I'd never impose the tool as a commit blocker.
We have strayed from the initial goal: Help new developers submit
well formatted patches, and learn by example using a tool.

-- 
Cheers,
Carlos.

Re: An abridged "Writing C" for the gcc web pages

[Bernd Schmidt]

On 05/03/2016 09:59 PM, Richard Sandiford wrote:


And sometimes there are multiple acceptable ways of writing the same code.
Which wins is an aesthetic choice, which tools tend to be bad at handling.
E.g. if a parameter list is too long for a line, there's often a choice
of how to balance the parameters across multiple lines, with some being
more readable than others.  I wouldn't want the one chosen by a formatting
tool to be the only acceptable one.


I have all the same reservations about using tools for this. We don't 
call it "style" for nothing, it is not purely mechanical and until we 
have general-purpose AI I don't believe computers can do a satisfactory 
job. I've seen attempts to do so in the past, and they have failed - you 
invariably end up with humans contorting their code to have it pass the 
checker, which is entirely counterproductive.



Bernd

Re: An abridged "Writing C" for the gcc web pages

[Maciej W. Rozycki]

On Tue, 3 May 2016, Richard Sandiford wrote:

> And sometimes there are multiple acceptable ways of writing the same code.
> Which wins is an aesthetic choice, which tools tend to be bad at handling.
> E.g. if a parameter list is too long for a line, there's often a choice
> of how to balance the parameters across multiple lines, with some being
> more readable than others.  I wouldn't want the one chosen by a formatting
> tool to be the only acceptable one.

 I'd call that a bug in the tool.  It shouldn't be switching between valid 
formatting styles unless explicitly asked to, even if the formatting style 
seen is not one the tool would itself produce from badly formatted code.

> And sometimes people split "if" conditions over multiple lines even when
> they would fit on one line, since the split version seems more readable.
> Again I wouldn't want a tool to be the final judge of which is right.

 Like the above -- as long as it's correct it should be left alone even if 
it's "suboptimal".

 FWIW,

  Maciej

Re: An abridged "Writing C" for the gcc web pages

[Carlos O'Donell]

On 05/03/2016 03:59 PM, Richard Sandiford wrote:
> Jeff Law  writes:
>> On 05/02/2016 02:40 PM, Carlos O'Donell wrote:
>>>
>>> However, in the end, I think that yet-another-document is not the
>>> solution we want. What we actually need is a program that just formats
>>> your source according to the GNU Coding Style and that way you can always
>>> tell your users "Run indent" and be done with it. The output of such a
>>> program should always be considered correct, and if it's not, we should
>>> fix it immediately.
>>>
>>> Why can't we use indent?
>> Sadly, "indent" simply breaks c++ code.
>>
>> I think the solution here is clang-format with a suitable clang-format 
>> configuration file.  One has been started (gcc/contrib/clang-format), 
>> but it's not yet complete.
> 
> How far are you intending to go with that though?

As far as it takes to make the process of submitting patches less
painful for new contributors and sensible for maintainers?

If we end up with one tool per branch then we did it wrong. In glibc
I'm happy to retroactively say the newest formatting tool is always
right for all branches barring expert intervention.

> And if the tool isn't the final authority, and it does still remain
> a human choice, then...

We *are* the community, so yes, there is a final line in the sand where
a sensible expert maintainer may tell you "yeah but" and ask you to change
the code to make it more legible. However, 99% of the changes are now just
going to be automatic.
 
>>> At the end of the day I never want to see another comment about code
>>> formatting because the user used X and X always formats code correctly.
>> Amen.
> 
> ...this kind of comment is still going to occur.  And clear documentation
> should help when it does.

Yes, it will occur, but the frequency should be much reduced. I also agree
that clear documentation helps, so I don't disparage Bernd's work here,
I think it's great. I just think that the most important next step here
is a submission code formatting tool.

> Also, coding standards and the scope of Bernd's document are more
> than formatting.  E.g. take your comment in a recent thread about not
> adding "()" when referring to a function in comments.  Even clang-format
> is unlikely to be smart enough to know whether a particular () is
> acceptable or not.  (It would be acceptable in a quoted code snippet
> but not in a normal sentence.)  Also, the tool wouldn't know when
> argument names need to be capitalised in comments, etc.  Sometimes
> argument names are English words and comments contain a mixture of
> both uses.  Bernd's document talked about those kinds of details too.

Agreed. That's minor.

> Sorry for the rant :-)  Maybe I'm just jaded by past experience with
> automatic formatting tools.  E.g. we internally use(d?) check_GNU_style.sh
> to check patches before submission and it seemed to produce far more
> false positives than true positives.  (I'm not sure it ever produced
> what I'd consider a true positive for me, although I did sometimes do
> what it said even if I disagreed, as the path of least resistance.)
> That's not to say the script is bad, just that it shouldn't always be
> taken literally.

That's a shame and a clear indicator the script is wrong or the technology
wasn't around for us to do the kinds of things we wanted.

> Obviously clang-format is smarter than check_GNU_style.sh but I think
> the same principle applies.

If we get one false positive out of the new checker we've done something
wrong. It should strive to give output that everyone agrees is 100%
correct barring optinal aesthetic choices. This way the tool is run, a
reviewer looks at the patch, and final comments are made on the style.

That's what I'd like to see for glibc. $0.02.

-- 
Cheers,
Carlos.

Re: An abridged "Writing C" for the gcc web pages

[Richard Sandiford]

Jeff Law  writes:
> On 05/02/2016 02:40 PM, Carlos O'Donell wrote:
>>
>> However, in the end, I think that yet-another-document is not the
>> solution we want. What we actually need is a program that just formats
>> your source according to the GNU Coding Style and that way you can always
>> tell your users "Run indent" and be done with it. The output of such a
>> program should always be considered correct, and if it's not, we should
>> fix it immediately.
>>
>> Why can't we use indent?
> Sadly, "indent" simply breaks c++ code.
>
> I think the solution here is clang-format with a suitable clang-format 
> configuration file.  One has been started (gcc/contrib/clang-format), 
> but it's not yet complete.

How far are you intending to go with that though?  I'd hate to see
a tool become the final authority on coding standards.  For one thing
it means that everyone will need exactly the same version of clang-format
installed (a la autoconf) in order for all the submissions to agree.
If we ever did upgrade to a newer version, the formatting rules for the
codebase are likely to change en masse to whatever the new clang-format
says is correct.  Presumably we might need to keep more than one version
of the formatter around, depending on which branch we're working on.

And sometimes there are multiple acceptable ways of writing the same code.
Which wins is an aesthetic choice, which tools tend to be bad at handling.
E.g. if a parameter list is too long for a line, there's often a choice
of how to balance the parameters across multiple lines, with some being
more readable than others.  I wouldn't want the one chosen by a formatting
tool to be the only acceptable one.

And sometimes people split "if" conditions over multiple lines even when
they would fit on one line, since the split version seems more readable.
Again I wouldn't want a tool to be the final judge of which is right.

Also, reformatting a statement over more lines isn't always the best way
of dealing with long lines.  Sometimes it's better to split it into separate
statements instead.  That too is a choice that tools are bad at making.
(E.g. if you introduce a temporary variable, what should it be called?)

There are exceptions to rules too.  E.g. I don't think it's required
to end a comment with a full stop if the comment ends with code.

  /* This is to handle statements like:
   i++;  */

is IMO correct, rather than:

  /* This is to handle statements like:
   i++;.  */

or:

  /* This is to handle statements like:
   i++;
 .  */

I doubt any tool is going to be good enough to make that kind of
distinction.  (E.g. you can't rely on the semicolon if quoting
Fortran code, or a .def style macro invocation.)

And if the tool isn't the final authority, and it does still remain
a human choice, then...

>> At the end of the day I never want to see another comment about code
>> formatting because the user used X and X always formats code correctly.
> Amen.

...this kind of comment is still going to occur.  And clear documentation
should help when it does.

Also, coding standards and the scope of Bernd's document are more
than formatting.  E.g. take your comment in a recent thread about not
adding "()" when referring to a function in comments.  Even clang-format
is unlikely to be smart enough to know whether a particular () is
acceptable or not.  (It would be acceptable in a quoted code snippet
but not in a normal sentence.)  Also, the tool wouldn't know when
argument names need to be capitalised in comments, etc.  Sometimes
argument names are English words and comments contain a mixture of
both uses.  Bernd's document talked about those kinds of details too.

Sorry for the rant :-)  Maybe I'm just jaded by past experience with
automatic formatting tools.  E.g. we internally use(d?) check_GNU_style.sh
to check patches before submission and it seemed to produce far more
false positives than true positives.  (I'm not sure it ever produced
what I'd consider a true positive for me, although I did sometimes do
what it said even if I disagreed, as the path of least resistance.)
That's not to say the script is bad, just that it shouldn't always be
taken literally.

Obviously clang-format is smarter than check_GNU_style.sh but I think
the same principle applies.

Thanks,
Richard

Re: An abridged "Writing C" for the gcc web pages

[Jeff Law]

On 05/02/2016 02:40 PM, Carlos O'Donell wrote:


However, in the end, I think that yet-another-document is not the
solution we want. What we actually need is a program that just formats
your source according to the GNU Coding Style and that way you can always
tell your users "Run indent" and be done with it. The output of such a
program should always be considered correct, and if it's not, we should
fix it immediately.

Why can't we use indent?

Sadly, "indent" simply breaks c++ code.

I think the solution here is clang-format with a suitable clang-format 
configuration file.  One has been started (gcc/contrib/clang-format), 
but it's not yet complete.




At the end of the day I never want to see another comment about code
formatting because the user used X and X always formats code correctly.

Amen.

jeff

Re: An abridged "Writing C" for the gcc web pages

[Carlos O'Donell]

On 04/22/2016 12:21 PM, Bernd Schmidt wrote:
> (Apologies if you get this twice, the mailing list didn't like the
> html attachment in the first attempt).
> 
> We frequently get malformatted patches, and it's been brought to my
> attention that some people don't even make the effort to read the GNU
> coding standards before trying to contribute code. TL;DR seems to be
> the excuse, and while I find that attitude inappropriate, we could
> probably improve the situation by spelling out the most basic rules
> in an abridged document on our webpages. Below is a draft I came up
> with. Thoughts?

Despite the comments downthread, I think that abridged versions of
longish standards documents are always a good idea. They need to be
maintained, usually reactively, and that's fine. It gives a new
contributor something to read that's short enough to provide the salient
points for an initial patch/review cycle. The point is to get better
incrementally. Nobody is ever perfect the first time.

I especially like that Jason caught on that your document is actually
*more* than just the GNU Coding Standard, and that where the standard
is underspecified the projects have their own conventions.

For examples just look at:
https://sourceware.org/glibc/wiki/Style_and_Conventions

However, in the end, I think that yet-another-document is not the
solution we want. What we actually need is a program that just formats
your source according to the GNU Coding Style and that way you can always
tell your users "Run indent" and be done with it. The output of such a
program should always be considered correct, and if it's not, we should
fix it immediately.

Why can't we use indent?

If we can't use indent, what do we need to solve this problem?

At the end of the day I never want to see another comment about code
formatting because the user used X and X always formats code correctly.

-- 
Cheers,
Carlos.

Re: An abridged "Writing C" for the gcc web pages

[Richard Sandiford]

Bernd Schmidt  writes:
> (Apologies if you get this twice, the mailing list didn't like the html 
> attachment in the first attempt).
>
> We frequently get malformatted patches, and it's been brought to my 
> attention that some people don't even make the effort to read the GNU 
> coding standards before trying to contribute code. TL;DR seems to be the 
> excuse, and while I find that attitude inappropriate, we could probably 
> improve the situation by spelling out the most basic rules in an 
> abridged document on our webpages. Below is a draft I came up with. 
> Thoughts?

The patch got some slightly negative reactions, so to balance things out:
it looks like a good intro to me FWIW.  A couple of nits:

> +There should be a space before open-parentheses and after commas.
> +We also use spaces around binary operators.

I realise it's trying to be short, but the first rule doesn't account
for things like "((int) a + b) * 3" and "-(int) 3".  Maybe "There should
be a space between a function name and the opening parenthesis?".
Doesn't cover macros of course.

> +Also note that multi-line comments should always be formatted as in
> +the previous example.  There should not be extra stars at the
> +beginning of new lines, and the comment text should being immediately
> +after the opening /*.

s/being/begin/.

Thanks,
Richard

Re: An abridged "Writing C" for the gcc web pages

[Bernd Schmidt]

On 04/22/2016 09:45 PM, Sandra Loosemore wrote:

On 04/22/2016 10:42 AM, paul_kon...@dell.com wrote:



Would you expect people to conform to the abridged version or the
full standard?  If the full standard, then publishing an abridged
version is not a good idea, it will just cause confusion.  Let the
full standard be the rule, make people read it, and if they didn't
bother that's their problem.


I agree; let's not have two documents that can conflict or get out of
sync with each other, unless you can figure out how to extract the
abridged document automatically from the full version.


Hmm. As for being out-of-date, I'd say the official document is guilty: 
it talks about how we can use C89 now that it is prevalent enough. Now 
that I've looked at it again after many years, the document really does 
seem poorly organized and contains lots of irrelevant information (a 
huge table of long option names?)


I think if we limit our local document to just the basics (and maybe 
call it a Getting Started guide rather than an abridged form of the GNU 
coding standards), there's little danger of it going out of date, and I 
still think having it would improve our documentation.



Bernd

Re: An abridged "Writing C" for the gcc web pages

[Sandra Loosemore]

On 04/22/2016 10:42 AM, paul_kon...@dell.com wrote:



On Apr 22, 2016, at 12:21 PM, Bernd Schmidt
 wrote:

(Apologies if you get this twice, the mailing list didn't like the
html attachment in the first attempt).

We frequently get malformatted patches, and it's been brought to my
attention that some people don't even make the effort to read the
GNU coding standards before trying to contribute code. TL;DR seems
to be the excuse, and while I find that attitude inappropriate, we
could probably improve the situation by spelling out the most basic
rules in an abridged document on our webpages. Below is a draft I
came up with. Thoughts?


Would you expect people to conform to the abridged version or the
full standard?  If the full standard, then publishing an abridged
version is not a good idea, it will just cause confusion.  Let the
full standard be the rule, make people read it, and if they didn't
bother that's their problem.


I agree; let's not have two documents that can conflict or get out of 
sync with each other, unless you can figure out how to extract the 
abridged document automatically from the full version.


I think it's fine to have something on the web pages explaining that all 
contributions must follow the GNU coding standards (with a link) since 
code that follows the same formatting conventions throughout is easier 
to read, and that (in particular) patches must match the style of the 
surrounding code.


-Sandra

Re: An abridged "Writing C" for the gcc web pages

[Mikhail Maltsev]

On 04/22/2016 07:21 PM, Bernd Schmidt wrote:
> (Apologies if you get this twice, the mailing list didn't like the html
> attachment in the first attempt).
> 
> We frequently get malformatted patches, and it's been brought to my attention
> that some people don't even make the effort to read the GNU coding standards
> before trying to contribute code. TL;DR seems to be the excuse, and while I 
> find
> that attitude inappropriate, we could probably improve the situation by 
> spelling
> out the most basic rules in an abridged document on our webpages. Below is a
> draft I came up with. Thoughts?
> 
Probably contrib/clang-format and https://gcc.gnu.org/wiki/FormattingCodeForGCC
are also worth mentioning.

-- 
Regards,
Mikhail Maltsev

Re: An abridged "Writing C" for the gcc web pages

[Jason Merrill]

On 04/22/2016 12:42 PM, paul_kon...@dell.com wrote:



On Apr 22, 2016, at 12:21 PM, Bernd Schmidt  wrote:

(Apologies if you get this twice, the mailing list didn't like the html 
attachment in the first attempt).

We frequently get malformatted patches, and it's been brought to my attention 
that some people don't even make the effort to read the GNU coding standards 
before trying to contribute code. TL;DR seems to be the excuse, and while I 
find that attitude inappropriate, we could probably improve the situation by 
spelling out the most basic rules in an abridged document on our webpages. 
Below is a draft I came up with. Thoughts?


Would you expect people to conform to the abridged version or the full 
standard?  If the full standard, then publishing an abridged version is not a 
good idea, it will just cause confusion.  Let the full standard be the rule, 
make people read it, and if they didn't bother that's their problem.


And this isn't strictly an abridged version, as it contains information 
that is not part of the GNU standard.



+The format should be text/plain so that mail clients such as
+thunderbird can display and quote it, without forcing potential
+reviewers to take extra steps to save it and open it elsewhere before
+being able to look at it.


I note that this patch itself is sent as text/x-patch, which thunderbird 
handles fine.  And apparently so does gmail, if you use the .diff 
extension; the .patch extension that I have tended to use doesn't work 
properly, so I guess I'll switch.



+All leading whitespace should be replaced with tab characters as
+much as possible, but tab characters should not be used in any other
+circumstances.


Some headers also use tabs to separate the parameter-list from the 
function name.


Jason

Re: An abridged "Writing C" for the gcc web pages

[Paul_Koning]

> On Apr 22, 2016, at 12:21 PM, Bernd Schmidt  wrote:
> 
> (Apologies if you get this twice, the mailing list didn't like the html 
> attachment in the first attempt).
> 
> We frequently get malformatted patches, and it's been brought to my attention 
> that some people don't even make the effort to read the GNU coding standards 
> before trying to contribute code. TL;DR seems to be the excuse, and while I 
> find that attitude inappropriate, we could probably improve the situation by 
> spelling out the most basic rules in an abridged document on our webpages. 
> Below is a draft I came up with. Thoughts?

Would you expect people to conform to the abridged version or the full 
standard?  If the full standard, then publishing an abridged version is not a 
good idea, it will just cause confusion.  Let the full standard be the rule, 
make people read it, and if they didn't bother that's their problem.

paul