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

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

Hello Alan,

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

It proposes a new style variable, in addition to the already existing 16 (!).  
Syntactic contexts are constant as long as the variable keeps the same value, 
but yes, they differ depending on the particular chosen value, and that's 
exactly the point of this new setting.  This is no different than how `c-
syntactic-indentation-in-macros' works.  Finally, design and architecture 
don't necessary need to be held up until there are actual bugs to solve.
 
> I don't find it elegant.

Elegance in this matter, at least in my book, is reaching objectives with the 
minimal amounts of code at the right place(s) while preserving or enhancing 
the design (that's perhaps a reductive definition, but it is enough for the 
current discussion).  I wouldn't have bothered submitting this patch to the 
engine if it didn't enhance the current architecture.  And it does so because 
it allows a logical change in behavior with a single switch at the right place 
rather than tweaking many small pieces after the fact to achieve it.

> The style variables are intended for widely used features in CC Mode.

I'm generally skeptical on most popularity contexts since they are usually 
biased, either statistically or conceptually.  But I'll contribute an 
additional data point: I certainly did not have to use any of them in 20+ 
years until recently, except for `c-basic-offset' and `c-offsets-alist' of 
course.  If you're stating a policy, then I don't think it is written anywhere 
in the doc or in code comments.  And I think the current situation may just be 
proof of its non-existence.  If it's a new one, than I hope I can convince you 
it shouldn't apply in this case.

> To introduce another one just for a particular indentation would disrupt
> the logical consistency of CC Mode.

Would "disrupt the logical consistency of CC Mode"?  Nothing less?  Could you 
elaborate on what is the consistency that this change threatens exactly?  I 
can only hope that you won't be exactly repeating the points I already 
responded to above, since they are unconvincing at best and currently appear 
to be great exaggerations.  On the other hand more explanations would be more 
than welcome.

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

There are changes and changes.  I would not be surprised that most don't call 
for a new style variable.  This is simply not the case here.

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

I know the manual in and out, and the code realizing the indentation very 
well.  I've also written a bunch of non-trivial lineup functions (comparable 
to some bundled in `cc-align.el') and I'm already using the excellent 
combining facilities provided by offset lists.

And with that knowledge, I have to disagree.  Writing lineup functions is not 
easy, it can be tedious and messy, even if in the end the code doesn't need to 
be very long.  Because of a certain lack of coherency in how syntactic 
contexts are built (and lack of sufficient documentation for that process), it 
is indeed easy to forget cases, which only appear later when indenting some 
new or yet unexplored code.  Sure, you can write lineups very quickly for toy 
examples, but this is not what I'm aiming at (and you're now aware of the 
larger context).

I don't claim to have years of experience with the internals of CC mode and 
lineups, contrary to you.  Yet, in addition to what I already exposed, I've 
carefully analyzed a large part of CC Mode code in the past days (I've not 
counted, but based on the number of files and their size, very roughly between 
30 and 50% of the code base), that's also why I'm confidently making these 
statements.  I might be wrong.  On the other hand, I'm also beginning to 
wonder if your ~20-year maintainership of CC Mode is simply biasing you on the 
matter.

Indeed, don't you see any problem with:

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

It doesn't have to be like that.  And it is not going to take only a single 
`first' list.  And we're only talking about a single issue in the journey to 
implement BSD KNF, there are a lot more requiring even more complexities in 
offsets and new lineup functions.  I know that since I have working code for 
the whole thing.

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

Here we are definitely leaving the realm of points of view or opinions for 
that of plain facts.  And unless we do not have the same words, this statement 
is just completely wrong.

	 ;; CASE 7D: we are inside a conditional test clause. treat
	 ;; these things as statements
	 ((progn
	    (goto-char containing-sexp)
	    (and (c-safe (c-forward-sexp -1) t)
		 (looking-at "\\<for\\>[^_]")))
	  (goto-char (1+ containing-sexp))
	  (c-forward-syntactic-ws indent-point)
	  (if (eq char-before-ip ?\;)
	      (c-add-syntax 'statement (point))
	    (c-add-syntax 'statement-cont (point))
	    ))

See the `looking-at' call matching the `for' keyword?  If this is not special 
casing, I don't know what this is.  Case 7D is a sub-case of case 7, whose aim 
is to match expressions, not statements.  Following subcases (7F to 7G) 
actually parse parenthesized expressions (I'm omitting 7E which concerns ObjC 
only).

So, this case specifically matches a `for' on the path to parsing 
parenthesized expressions, and in a branch which is not supposed to parse 
statements. QED.

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

Then at least there won't be things repeated.  But this gives me even more 
responsibility, speaking up for all those that didn't voice their concern.  
Which, now that I vaguely recall, probably included me over 20 years ago in a 
very different context.

And, again, I know of no C editor behaving the same as Emacs+CC Mode on `for' 
clauses, and certainly not the popular ones (but I also do not know all of 
them, and have not used some for years; please someone point out to those that 
would do).
 
> The treatment of `for' in CC Mode is not a special case in CC Mode.

Sorry, but it simply is.  See above (the part commenting case 7D) and just 
below (next paragraph).

> If anything, it's a special case in the language definitions, where
> executable code is enclosed in parentheses.

This is also plain wrong.  There's no more "executable code" (this sentence is 
not used anywhere in the standard) in `for' clauses than in expressions 
(parenthesized or not).  In fact, all but the first clause are just 
expressions in the standard, no more.  Only the first can be a declaration 
instead.  None of the clauses are statements.  You're mixing things up.  I 
encourage you to read the standard so you don't have to take my word for it.

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

You're playing on words but I think you've got the idea.  The main point is 
that this complexity is unnecessary.  Still, you're underestimating the number 
of corner cases.
 
> 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.

Not only I foresee them, but I've already experienced some of them.

And I already know what you're saying.  It would be perfect if that was all 
there was to it.  Unfortunately, it's not the case.  You're omitting for 
example the "special case" of having nested parenthesized groups with opening 
parentheses all on the same line.
 
> 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.

May be, I'm not sure.  CC Mode is already very slow for full file indentation 
(I know it's not its goal, but still).  But whether that matters or not, what 
is certain is that implementing a special case in an engine to subsequently 
undo it in a hook is simply bloat and a waste of cycles, however small it is.
 
> 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.

I couldn't disagree more.

What you are proposing is a lineup function that is itself essentially a large 
`cond' of special cases just to undo an engine's very localized special case.  
The proposed approach instead just selectively deactivates the latter in the 
first place.

Adding only a few new tests is more than enough, because there is provably no 
change at all if the variable is not explicitly set and because changes are 
confined to `for' clauses only.  How does this "complicate" the suite?  If 
adding a few tests is a problem, then the test suite is the problem.  
Addendum: I cannot find anything in the source tree but a mere 'cc-mode-
tests.el' that is 111 lines long (including comments).  Where is the "suite" 
you're talking about?

Even if you want to be comprehensive, on this precise change there's not much 
more to document than what is already in the patch.  Of course, a lot should 
be added to explain all the special cases of the engine, but this is 
independent from this report.  Again, you're talking about "substantial 
changes in the documentation", but you don't mention what you have in mind.

Finally, as for "fragmentation" or threat to "cohesiveness", I'm sorry to have 
to say that I have yet to read a single precise and relevant argument that can 
really back this point of view.

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

Then I'm asking you to step back and then reconsider, because you're most 
likely wrong.  At the very least, filling the gaps between the vague high-
level principles you stated (and which I generally support) and how the 
proposed change actually breaks them, as well as answering my other questions, 
would be very welcome.  If you're not willing to, then I'm not sure there is 
any point in continuing this discussion.

Regards.

-- 
Olivier Certner