Remove deprecated Clang-Format style options and setting BreakAfterReturnType to ExceptShortType

[anupsdf]

what

Remove deprecated clang-format20 style options and improve readability.

One more change here is to set BreakAfterReturnType: ExceptShortType which puts the return type in the same line as the definition with break after return type based on PenaltyReturnTypeOnItsOwnLine setting.
Example:

class A {
  int f() { return 0; };
};
int f();
int f() { return 1; }
int LongName::
    AnotherLongName();

why

1. Keeping .clang-format config file up to date by removing deprecated options.
2. Changes look more readable with reduced lines of code with return type on the same line. If a line gets too long, PenaltyReturnTypeOnItsOwnLine setting of 60 would break the line after return type.

NOTE: Counter proposal in #5061

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[drebelsky]

Just some general notes

[drebelsky]

Are there not fewer changes with BreakAfterReturnType: AllDefinitions?

[anupsdf]

I thought ExceptShortType was more readable than AllDefinitions. Though AllDefinitions is closer to what we had before. I will leave it for the team to decide on which one they prefer.

[dmkozh]

I would vote for never breaking after the function return type. I don't think it adds any readability, but it definitely does make search for the functions harder. I sometimes search for something like int foo and the line break makes this impossible to do without involving regexes.

[anupsdf]

There is a None option which sounds like would never break but its marked as deprecated in https://releases.llvm.org/20.1.0/tools/clang/docs/ClangFormatStyleOptions.html

We should do a team poll to vote on the available options: Automatic, ExceptShortType, All, TopLevel, AllDefinitions, TopLevelDefinitions

[anupsdf]

Here is a difference between ExceptShortType and Automatic,

==========
BreakAfterReturnType: ExceptShortType
==========

void Peer::maybeExecuteInBackground(
    std::string const& jobName, std::function<void(std::shared_ptr<Peer>)> f)
{


==========
BreakAfterReturnType: Automatic
==========

void
Peer::maybeExecuteInBackground(std::string const& jobName,
                               std::function<void(std::shared_ptr<Peer>)> f)
{

[drebelsky]

I believe CurrentLine corresponds to what we already have

ConstructorInitializerAllOnOneLineOrOnePerLine (Boolean) clang-format 3.7 ¶
This option is deprecated. See CurrentLine of PackConstructorInitializers

PCIS_CurrentLine (in configuration: CurrentLine) Put all constructor initializers on the current line if they fit. Otherwise, put each one on its own line.
PCIS_NextLine (in configuration: NextLine) Same as PCIS_CurrentLine except that if all constructor initializers do not fit on the current line, try to fit them on the next line.

[anupsdf]

NextLine seemed a bit more readable but we can check with team's preference.

[anupsdf]

Here is a difference between NextLine and CurrentLine,

==========
PackConstructorInitializers: NextLine
==========

CatchupConfiguration::CatchupConfiguration(LedgerNumHashPair ledgerHashPair,
                                           uint32_t count, Mode mode)
    : mCount{count}, mLedgerHashPair{ledgerHashPair}, mMode{mode}
{
}

==========
PackConstructorInitializers: CurrentLine
==========
CatchupConfiguration::CatchupConfiguration(LedgerNumHashPair ledgerHashPair,
                                           uint32_t count, Mode mode)
    : mCount{count}
    , mLedgerHashPair{ledgerHashPair}
    , mMode{mode}
{
}