[squid-dev] RFC: clang-format

[Alex Rousskov]

On 4/11/20 6:26 PM, Francesco Chemolli wrote:
>   I have made an attempt at running clang-format on the squid sources,
> after applying a configuration which follows as closely as possible
> the project's coding guidelines.
> 
> You can find the output of the exercise at
> https://github.com/kinkie/squid/tree/clang-format ; the configuration
> is at https://github.com/kinkie/squid/blob/clang-format/.clang-format
> .
> 
> Most of the changes are shifting comments and realigning macro blocks.
> What do you think? Is it worthwhile to make this or same variant of
> this an official guideline?


Hi Francesco,

    Most likely, you have spent more time researching this issue than
anybody reading your email; your recommendations and especially
justifications behind those recommendations could be the most valuable!
Do _you_ think clang-format advantages are worth the large amount of
conflicts the switch will create across branches? If yes, what are those
advantages, and why are they worth the cost? Can clang-format do much
better than mimicking our current (very limited) Code Style? Etc. Please
try to convert this thread from an opinion poll into a true RFC that it
is supposed to be...


I only looked at a small portion of the diff, but, FWIW, I saw:

* many small improvements (e.g., [1]),
* some small regressions (e.g., [2,3]),
* more complex #include ordering rules (e.g., [4]),
* numerous bad comment movements (e.g., [5]),
* numerous pointless(?) last empty line removals (e.g., [6]).

I suspect some of the problems mentioned above can be reduced via
clang-format configuration adjustments, but I did not study this issue.


Thank you,

Alex.

[1] better tertiary operator spacing near
https://github.com/squid-cache/squid/compare/master...kinkie:clang-format#diff-94b81725fceba313922066c3ce6071a7R19

[2] broken xgetnameinfo() formatting near
https://github.com/squid-cache/squid/compare/master...kinkie:clang-format#diff-31086c4678ad7442b4024e422f6e4351R318

[3] extra ip6_parsenumeric() function name indent near
https://github.com/squid-cache/squid/compare/master...kinkie:clang-format#diff-31086c4678ad7442b4024e422f6e4351R337

[4]
https://github.com/squid-cache/squid/compare/master...kinkie:clang-format#diff-2b432472575e0b46de13556740f65a1aR10

[5] manual comment alignment for related code lines is a Bad Idea, but
even if we do automate all such alignments, Squid code is not written to
help clang identify _related_ lines correctly as illustrated by the
jobs_ member declaration changes near
https://github.com/squid-cache/squid/compare/master...kinkie:clang-format#diff-57b6a8db44fb379395f9be80a4c44c5aR82

[6] Virtually any modified source file can be used as an example, but
here is one (compat/inet_pton.h) where nothing else has changed:
https://github.com/squid-cache/squid/compare/master...kinkie:clang-format#diff-563683e32b22291b9d5622652ec544d9L33

Alex.