bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable

[Olivier Certner <ocert.dev@free.fr>]

Hello Alan,

Thanks for looking at this.

> But, first things first, please.  What is the problem that your change
> is intended to fix?  What current indentation is suboptimal?

Have you read my first message?  Let me copy the relevant part: "This is a 
proposed change adding a new style variable to control whether `for' clauses 
should be indented as statements or as an argument list.".  I think this 
directly answers your first question, but let's be explicit to avoid 
misunderstandings: I'm not trying to "fix" anything, this is a new feature.  
As for the second question, I think it is safe to say it's irrelevant and 
you'll soon see why.

I'm sorry (to you and Eli) if I didn't give more context initially, it is in 
part an overlook but also in part a conscious decision as a consequence of me 
thinking that you (both, but especially you Alan) would mostly figure out the 
smaller context on your own.  Apparently, I was wrong.  Still, I don't think I 
understand what is not obvious to you (Alan) in the submitted simple patch.  
So I'll try by giving a lot more context and details.  If I don't succeed to 
make things crystal clear this way, I'll have to ask you to be more specific 
about what is still not.

So, now that I'm at it, let me first explain the larger context (the one that 
you couldn't guess).  I've been working on a CC mode style respecting 
guidelines ("style") and existing practice for FreeBSD's kernel.  After more 
work I've found that this is applicable to all BSDs, with small exceptions, 
including 4.4BSD. The style is called Kernel Normal Form (KNF).  In all BSDs 
(well, not the case for NetBSD, but let's ignore that here), what is inside 
parenthesis must, for the excess parts that are split into their own lines (in 
other words, for all lines other than the first, the one that contains the 
opening '('), be always indented by 4 columns.

[Small digression: The desire to have a style for FreeBSD is why I opened 
Bug#63072 in the first place, but you don't need to read it (for now).  In 
retrospect, I opened it too soon, since I noticed some discrepancies between 
the results of my first attempts and the indentation of some files, so this 
bug is currently stalled in terms of message activity.  However, I've been 
working on it and I've almost reached my goals, so expect more CC mode tickets 
in the near future, for most of which Bug#63072 will be a kind of umbrella and 
the ultimate point.  Also, the preliminary discussion with Eli in that bug is 
now most probably obsolete.  This bug (63286) is a step in this work.  While 
I'm here, could you also please have a look at Bug#63285, it describes a 
genuine problem, is self-contained (doesn't need more context) and I've 
provided a fix?]

As for the smaller context and this particular report, let's consider a very 
simple `for' statement as a representative example:

for (i = 0; i < n; ++i) {
...
}

where the block statements are elided because irrelevant to the discussion. 
Here are different splitting of the initial `for' line, and the desired second 
line's indentation in each case:

for 
    (i = 0; i < n; ++i) {

for (
    i = 0; i < n; ++i) {

for (i =
    0; i < n; ++i) {

for (i = 0;
    i < n; ++i) {

for (i = 0; i
    < n; ++i) {

for (i = 0; i < n;
    ++i) {

In plain English, the second line is just always aligned 4 columns away from 
the start of the `for' keyword.  Given the rule I stated above, all these 
results should be unsurprising to you.

By contrast, here is the same list but showing the current indentation with 
the "gnu" style modified with `c-basic-offset' set to 4 to eliminate trivial 
style differences:

for 
    (i = 0; i < n; ++i) {

for (
     i = 0; i < n; ++i) {

for (i =
         0; i < n; ++i) {

for (i = 0;
     i < n; ++i) {

for (i = 0; i
         < n; ++i) {

for (i = 0; i < n;
     ++i) {

Even more than the actual results, what is important here is the syntactic 
context CC Mode devises for each second line.  CC Mode considers the three 
`for' clauses (the `i = 0', `i < n' and `++i'; technically, in the C standard, 
only the first is called a clause, the two others are just normal expressions, 
but again let's ignore for the sake of simplicity) as statements, if they 
stand on a line of their own.  The second and third clauses are anchored to 
the first (indirectly for the third; it is in fact anchored to the second 
which itself is to the first) so are aligned with it (given that `statement' 
is associated to 0 in `c-offsets-alist').  As a consequence, please look at 
how `i < n' or `++i' line up with `i = 0'.  More precisely, these statements 
are anchored to the first non-whitespace character of the first clause.  The 
first clause itself, if starting on a new line, is anchored to the column 
following the opening `(' (in reality, it is anchored to the whole `for', but 
then `c-lineup-arglist-intro-after-paren' in `arglist-intro' simulates that by 
returning an absolute offset corresponding to the column after '(').  By 
contrast, if a clause is split, its second part is indented as a `statement-
cont' (as one would expect, and coherently with all statements that are 
split).  As an example, please look at how the line starting with 0 is shifted 
by 4 columns with respect to `i'.

The two main changes that are required to handle BSD KNF style for `for' 
statements, with "gnu" as a starting point, are:
1. `for' clauses should not be aligned to the opening parenthesis, but rather 
to the start of the whole `for' statement (the `f' character of the `for' 
keyword).
2. Clause continuations should be lined up exactly as start of clauses.

Both these changes cannot be achieved by tweaking of `c-offsets-alist' with 
simple offsets.  Indeed, for 1., there is no fixed relation between both 
anchors (because of the possibility of any extra whitespaces before and after 
the opening '('), and for 2., the clause continuation offset should be 0, 
meaning that currently `statement-cont' must be 0, which affects also any 
statement continuations not in `for' clauses, for which BSD KNF mandates 4 
columns.

> Have you tried fixing it by less radical means and not managed?

I guess you mean "radical" because this is an engine modification?  
Personally, I think the proposed solution is the very opposite of radical.  It 
doesn't disrupt any existing code at all (it's a single line change, having 
effect only when a new, separate style variable is set to some non-default 
value) and is most likely the most elegant solution.  Let me briefly argue 
why.

Given what I've written above, there are only two ways to have CC Mode follow 
BSD KNF style: Either I use lineups functions (or special indent hooks), or I 
modify the engine.  I don't contest that writing a lineup function is 
possible.  But I find it bad style in this case and for lots of reasons:
1. This is correction after the fact for what is already a special case in the 
engine code.  What could be simpler than just avoiding it and getting the 
(logical and expected) fallback, that is, uniform treatment of parenthesized 
parts?
2. While this existing special case has its merits, it is peculiar and I don't 
remember seeing any other editor indenting `for' clauses like this.  I've not 
searched bug reports, but I would be surprised that no one has already 
reported this behavior as a bug (which, again, personally I don't find it is; 
but it should be tunable).  Or perhaps people just gave up on it.  However, in 
the case of BSD KNF, this is not an option because style and the existing code 
are very clear about the expected outcome and such indentation won't be 
accepted.
3. This special case is not well integrated with other parts of `c-guess-
basic-syntax'.  As just a single example, in the second splitting above, the 
second line is analyzed as `arglist-intro'.  It should arguably be recognized 
as `statement' instead, with some appropriate anchor.  It is amusing to note 
that the current behavior is more coherent with the special case disabled in 
some places.
4. The lineup function must apply to several syntactic elements, since there 
are many different situations that can arise depending on how `for' clauses 
are split.  Just for the simple example above, the listed 6 splittings for the 
simple `for' line lead to 4 different syntactic contexts for the second line.  
Most require more or less ad-hoc code to backtrack and detect that we are 
indeed inside a `for' clause and that the engine's chosen offset must be 
overridden.  Take a more complex example, with parenthesized expressions in 
the clauses, and you have even more cases to worry about (part of them could 
be avoided by another lineup function that is separately useful, which I'll 
submit next week, but still).

In fact, I would probably bet that some ~20-line lineup function could do the 
trick.  I just think it doesn't make sense to write one: It will take more 
time than the proposed fix to get correctly and will be less efficient.  So 
what are you, as the maintainer, afraid of in my proposed solution exactly?

Thanks and regards.


-- 
Olivier Certner