clang-format version incompatibility

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

clang-format version incompatibility

Manas via cfe-dev
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:


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

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


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

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


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


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clang-format version incompatibility

Manas via cfe-dev
On Wed, Oct 14, 2020 at 10:17 PM Renato Golin via cfe-dev <[hidden email]> wrote:
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...)


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

clang-format-10:
    Rule(F f) -> Rule<
      typename rule_traits<decltype(&F::operator())>::Left,
      typename rule_traits<decltype(&F::operator())>::Right,
      F>;
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.
 


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(...));`.

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clang-format version incompatibility

Manas via cfe-dev
On Wed, 14 Oct 2020 at 23:21, Sam McCall <[hidden email]> wrote:
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?

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev