[tor-bugs] #32921 [Core Tor/Tor]: Code and script changes to run clang-format without breaking checkSpaces or coccinelle

[Tor Bug Tracker & Wiki]

#32921: Code and script changes to run clang-format without breaking checkSpaces or
coccinelle
----------------------------+------------------------------------
 Reporter:  nickm           |          Owner:  nickm
     Type:  enhancement     |         Status:  needs_review
 Priority:  Medium          |      Milestone:  Tor: 0.4.3.x-final
Component:  Core Tor/Tor    |        Version:
 Severity:  Normal          |     Resolution:
 Keywords:  style, 043-can  |  Actual Points:  1.5
Parent ID:  #29226          |         Points:
 Reviewer:  catalyst        |        Sponsor:
----------------------------+------------------------------------

Comment (by teor):

 Replying to [comment:20 catalyst]:
 > Remaining things that I think are excessive changes by clang-format:
 >
 > * clang-format is too eager to move line breaks around so that a long
 argument list wraps with the longest-possible lines.  This can make code
 more confusing if arguments have a useful semantic grouping.  I'm not sure
 how to turn this off.  I think it might do a similar thing to long
 expressions in the controlling expressions of conditional statements?
 Maybe we can work around this by adding redundant parentheses to make
 clang-format more reluctant to break lines at certain places.  What do
 people think about that?

 I don't think using whitespace as implicit documentation is ideal.

 Instead of giving whitespace an implicit meaning, we should explicitly
 document that meaning. If we just use whitespace:
 * it's easy for other developers to miss subtle formatting, and
 * any changes to the code can easily destroy the grouping.

 So I suggest that we add comments to these kinds of argument lists.

 If necessary, we can add whitespace or punctuation to the comments, to
 achieve a particular argument alignment.

 > * extra indentation for array initializers, e.g. clang produces
 >   {{{
 > struct foo a[] = {
 >     V(x),
 >     V(y),
 > };
 > }}}
 >   while existing code tends to have
 >   {{{
 > struct foo a[] = {
 >   V(x),
 >   V(y),
 > };
 > }}}
 >   I think we can fix this with `ContinuationIndentWidth: 2`, but I'm not
 sure.  (This would change to 4 once we change to a 4-column indent.)

 I don't see a problem here. I don't mind either way.

 > * clang-format doesn't retain spaces inside curly braces in one-line
 initializer items, e.g. clang produces
 >   {{{
 > struct foo a[] = {
 >   {0, 0, 0},
 > };
 > }}}
 >   while existing code tends to have
 >   {{{
 > struct foo a[] = {
 >   { 0, 0, 0 },
 > };
 > }}}
 >   I find the style with the internal spaces more readable.  This is not
 a strong perference.

 I find the internal spaces slightly more readable, particularly with
 single digits. I don't think it matters as much with variable names.

 But I don't mind either way.

 > * clang-format doesn't have a way to maintain tabular alignment in
 initializer lists.  We have a lot of these.  The branch doesn't turn off
 clang-format in all of them.  Do we want to maintain tabular alignment?
 (I think it makes large tables more readable, but I'd like to hear other
 opinions.)

 I think tabular alignment is useful, particularly for large tables.

 We use *.inc files for some large tables. We could do the same thing with
 all the tables we want to have specific alignment?

 (The config code is in the process of migrating to *.inc files for its
 config variable tables.)

 > * I don't like the way `AlignTrailingComments: false` looks.  I like
 alignment for stuff like Doxygen in-line comments for structures.  It's
 not a strong preference, though.  (Among other things, I think Doxygen
 might do its own alignment in its HTML output.)

 Can you point to a before and after example?

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/32921#comment:21>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online