clang-format version incompatibility

Hi folks,

A recent issue [1] on our Github project has highlighted a problem we’ve been having with clang-format.

Basically, when using different versions of clang-format, with the same configuration files [2], we get different results (between 9 and 10).

Previously, the main issue was new versions supported more stuff (ex. AfterCaseLabel [3]) and old ones would not work, so we had to define a minimal version, but newer versions of clang-format were supposed to yield the same results based on the same configuration file.

Or perhaps, the default has changed and we didn’t have a specific config? If so, the fix is easy, just add the option to the config file. If not, we’ll have to continue hard-coding clang-format-9 in the CMake, which is not practical as more people try to build it.

Examples:

https://github.com/microsoft/verona/blob/master/src/mlir/dialect/Typechecker.cc#L79

clang-format-9:

Rule(F f)
->Rule<
typename rule_traits<decltype(&F::operator())>::Left,
typename rule_traits<decltype(&F::operator())>::Right,

;

clang-format-10:

Rule(F f) → Rule<
typename rule_traits<decltype(&F::operator())>::Left,
typename rule_traits<decltype(&F::operator())>::Right,

;

https://github.com/microsoft/verona/blob/master/src/mlir/dialect/VeronaTypes.cc#L487

clang-format-9:

return {MeetType::get(ctx, readElements),
JoinType::get(ctx, writeElements)};

clang-format-10:

return {
MeetType::get(ctx, readElements), JoinType::get(ctx, writeElements)};

https://github.com/microsoft/verona/blob/master/src/mlir/dialect/VeronaTypes.cc#L513

clang-format-9:
return {JoinType::get(ctx, readElements),
MeetType::get(ctx, writeElements)};

clang-format-10:

return {
JoinType::get(ctx, readElements), MeetType::get(ctx, writeElements)};

thanks!
–renato

[1] https://github.com/microsoft/verona/issues/324

[2] https://github.com/microsoft/verona/blob/master/.clang-format
[3] https://github.com/microsoft/verona/blob/master/CMakeLists.txt#L19

Hi folks,

A recent issue [1] on our Github project has highlighted a problem we’ve been having with clang-format.

Basically, when using different versions of clang-format, with the same configuration files [2], we get different results (between 9 and 10).

AFAIK this is in principle expected, clang-format doesn’t place a high prioriy on having stable formatting results across major versions.
This is a tradeoff, requiring formatting to stay stable implies not fixing bugs.

In some workflows you can mitigate this by formatting/checking changed lines, which at least reduces the surface area.
But this is certainly a pain point if you want to enforce style with CI in a diverse environment.
At work, we enforce the same version of clang-format on all clients and CI.
In a JS-based package I work on, we use the clang-format npm package to pin the version.

BTW, someone recently added a clang-format flag to ignore unknown keys in the config file, which will help in the distant future (when clangd-12 is the minimum version it’s safe to assume…)

https://github.com/microsoft/verona/blob/master/src/mlir/dialect/Typechecker.cc#L79

clang-format-9:

Rule(F f)
->Rule<
typename rule_traits<decltype(&F::operator())>::Left,
typename rule_traits<decltype(&F::operator())>::Right,

;

clang-format-10:

Rule(F f) → Rule<
typename rule_traits<decltype(&F::operator())>::Left,
typename rule_traits<decltype(&F::operator())>::Right,

;

This looks like a bugfix to me. I think deduction guides were basically unsupported and broken in 9.
If I’m right about that, there’s certainly no option to restore the old broken behavior.

https://github.com/microsoft/verona/blob/master/src/mlir/dialect/VeronaTypes.cc#L487

clang-format-9:

return {MeetType::get(ctx, readElements),
JoinType::get(ctx, writeElements)};

clang-format-10:

return {
MeetType::get(ctx, readElements), JoinType::get(ctx, writeElements)};

Less clear to me what’s going on here, but I also suspect a bugfix.
The docs say “Fundamentally, C++11 braced lists are formatted exactly like function calls would be formatted in their place.”.
The clang-format-10 formatting is equivalent to how both clang-format-9 and 10 format return x(MeetType::get(...), JoinType::get(...));.

AFAIK this is in principle expected, clang-format doesn’t place a high prioriy on having stable formatting results across major versions.

This is a tradeoff, requiring formatting to stay stable implies not fixing bugs.

Agreed.

In some workflows you can mitigate this by formatting/checking changed lines, which at least reduces the surface area.

But this is certainly a pain point if you want to enforce style with CI in a diverse environment.
At work, we enforce the same version of clang-format on all clients and CI.

That’s what we do, too. I think we’ll just have to update the version of clang-format and require everyone to upgrade theirs, too.

I’ll add a line to the “contributing” document about that.

In a JS-based package I work on, we use the clang-format npm package to pin the version.

Arch Linux has a “clang-format-static-bin” which puts all statically compiled versions in /opt. But it would be good to have a solution that works on all platforms, including Windows.

This looks like a bugfix to me. I think deduction guides were basically unsupported and broken in 9.

Right, I do prefer the new formatting, but had to revert to using version 9 for the CI.

clang-format-9:

return {MeetType::get(ctx, readElements),
JoinType::get(ctx, writeElements)};

clang-format-10:

return {
MeetType::get(ctx, readElements), JoinType::get(ctx, writeElements)};

Less clear to me what’s going on here, but I also suspect a bugfix.
The docs say “Fundamentally, C++11 braced lists are formatted exactly like function calls would be formatted in their place.”.

If that’s true, then wouldn’t the last curly brace + semicolon need to be in a new line, too?