[RFC][clang-tidy][clang-format] RequiresVersion configuration

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

[RFC][clang-tidy][clang-format] RequiresVersion configuration

Fangrui Song via cfe-dev
Dear List,

I'm asking if people think the idea of having a requires version clause
in .clang-format and .clang-tidy configuration files would be a good
idea, akin to cmake_minimum_required(VERSION 3.4.3).
The benefit here wouldn't be immediately noticed but down the line it
will resolve issues when people are using different versions of the
respective tools contributing to large projects.

The idea would be you can set the entire file to require a minimun
version or just a specific option/check.

For example say you want to use the new `IndentCaseBlocks` introduced
recently to clang-format but some developers aren't using trunk clang-
tidy(obviously).

>IndentCaseBlocks: true

Putting that in the config file will cause an error when running clang-
tidy in earlier versions that don't understand.
Proposed ideas could be to have:

// Style 1
>RequireMinimumVersion: 11 //At top of .clang-format file
>...
>IndentCaseBlocks: true
or
//Style 2
>[[RequireMinimumVersion: 11]IndentCaseBlocks: true

The 2 different styles handle different cases. If not having the
`IndentCaseBlocks` option will break your formatting, you may not want
to format at all, in which case use `Style 1`. If however you don't
mind not having the format option, then use `Style 2` and when parsing
the options it will just ignore options that fail the minimum version
check.

There is a similar use case for clang-tidy, say a check has been
updated to add a new option in there that you need setting for your
project, you likely will only want the check to run if you can set the
version, this could be enabled in a similar(ish) way

>Checks: '-*,[[RequireMinimunVersion: 11]]check-with-new-option-added-
in-11'

This could also be used to disable checks based on version(if there was
a regression in a check)

>Checks: '-*,misc*,[[RequireMinimumVersion: 11]]-misc-check-with-
regression-in-11'

Likewise you could use a global option in the .clang-tidy file

>RequireMinimunVersion: 11
>Checks: ...

Obviously the syntax of the clause is open to discussion and this will
cause issues with people using a version of aforementioned tools with
configuration files that support this new syntax, however hopefully it
will be a slow rollout of groups/companies using this syntax.

Handling of the clang-format configuration will likely need to be
implemented when parsing the yaml file. Handling of the clang-tidy
configuration will need to be handled when creating the GlobList.

Thank you for reading,

Nathan James



_______________________________________________
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: [RFC][clang-tidy][clang-format] RequiresVersion configuration

Fangrui Song via cfe-dev
Hello,

I think this would be a good idea since i’m guessing in automation many scripts use some sort of a method to determine the minimum `clang-tidy`, `clang-format` version that is required in order to run.
For example @mozilla we have this something similar but outside of `clang-format` configuration [1].

[1] https://searchfox.org/mozilla-central/source/tools/clang-tidy/config.yaml#22

On 13 Apr 2020, at 13:59, Nathan James via cfe-dev <[hidden email]> wrote:

different


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

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [RFC][clang-tidy][clang-format] RequiresVersion configuration

Fangrui Song via cfe-dev
In reply to this post by Fangrui Song via cfe-dev
On Mon, Apr 13, 2020 at 11:59:55 +0100, Nathan James via cfe-dev wrote:
> I'm asking if people think the idea of having a requires version clause
> in .clang-format and .clang-tidy configuration files would be a good
> idea, akin to cmake_minimum_required(VERSION 3.4.3).

Does this mean that if I say "min clang-format of 9.0", 10.0 will use
9.0-like defaults for settings that did not exist in 9.0? While this
might be nice to catch use of older-than-supported clang-format
versions, detecting new ones and making them not change opinion on whole
swaths of code at the same time would be great.

As it is, we have clang-format enforced in one location where we can pin
the version in use (and we can use it to push the expected formatting
back so that folks don't have to track down version x.y on every
development machine). It's the only way we've found that is scalable
with enforced formatting (personally, I find CI that just says "your
code looks dumb, here's a diff" and can't just apply that diff for me to
be very unhelpful).

> The 2 different styles handle different cases. If not having the
> `IndentCaseBlocks` option will break your formatting, you may not want
> to format at all, in which case use `Style 1`. If however you don't
> mind not having the format option, then use `Style 2` and when parsing
> the options it will just ignore options that fail the minimum version
> check.

I want to not have to track `master` though and know what new option
defaults break my code though. But, I know that's a high ask given how
much work we do in CMake to have that property :) .

Thanks,

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