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

[Alan Mackenzie <acm@muc.de>]

Hello, Olivier.

On Sat, May 06, 2023 at 23:41:29 +0200, Olivier Certner wrote:
> 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.

In your message with the patch, there was a lot of "what", but very
little, if any, "why".  Thanks for filling in the gap for me!

[ .... ]

> 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.

[ .... ]

> 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) {

[ .... ]

> 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:

[ .... ]

> 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?  

Not really.  It proposes introducing a new style variable, for one
thing.  But it also proposes making the syntactic expressions produced
for some constructs non-constant, and that where there isn't a bug to
solve.

> 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.

I don't find it elegant.  The style variables are intended for widely
used features in CC Mode.  To introduce another one just for a
particular indentation would disrupt the logical consistency of CC Mode.
There have been many fixes made to indentation over the years, and if
even just a few of these involved adding new style variables, the style
system would be more confusing and less usable that it is.

> 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 lineup functions (or special indent hooks), or I 
> modify the engine.  I don't contest that writing a lineup function is 
> possible.

It is also easy, particularly in the current case.  CC Mode is designed
to make it easy.  You could do this with a single lineup function which
would return either the desired offset, or nil when the function is
inapplicable.  The c-offsets-alist entries for the three (or four?)
pertinent syntactic symbols would then be modified to lists beginning
with the symbol `first' and followed by the new function then the
default.  The new function would likely be well under 20 lines (not
counting comments).  See the page "c-offsets-alist" in the CC Mode
manual.

> 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?

The treatment is already uniform - it just doesn't currently allow the
indentation you want in your new style.

> 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.

No, there have been no other bug reports about this in the last 20 years
or so, at least.  Yours is the first one.

> 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.

The treatment of `for' in CC Mode is not a special case in CC Mode.  If
anything, it's a special case in the language definitions, where
executable code is enclosed in parentheses.

> 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.

I think there are rather _several_ different situations rather than many.

> 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).

I don't foresee the troubles that you do.  By the time the lineup
function comes to be run, things like extra parenthesised expressions
have already been analysed by the indentation engine.  The anchor
positions allow one readily to find the opening parenthesis and the
preceding `for' token, or determine that they don't exist in the current
case.

> 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.

I think a single 20 line function could do the trick, as outlined above.
The extra run time (if any) taken would be too small to measure.

> So what are you, as the maintainer, afraid of in my proposed solution
> exactly?

That it would fragment CC Mode, reducing its cohesiveness and
introducing special cases where none are needed.  It would entail
changes to the test suite, which would complicate it, and substantial
changes to the documentation.

In short, I don't think the approach you've presented is the right one
to get the indentation that you need.

> Thanks and regards.


> -- 
> Olivier Certner

-- 
Alan Mackenzie (Nuremberg, Germany).