llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.14k stars 11.11k forks source link

Interest in updated run-clang-tidy, with header error deduplication? #91641

Open quicknir opened 2 months ago

quicknir commented 2 months ago

Recently I needed to run clang-tidy in a parallel matter for my CI. I didn't find anything suitable, so I wrote my own. Was wondering if there would be interest in upstreaming it? The single biggest difference between my script and the existing run-clang-tidy.py is that my script handles de-duplicating errors in header files. I find this feature more or less necessary, as without it a single error often leads to pages and pages of error messages, which seem to all be identical, but obviously you need go through it all to verify it. This tends to lead to a workflow where people fix one error and rerun, rather than fixing multiple errors each pass, making it much slower. I do this using --export-fixes and then reading the YAML file, so that my deduplication of errors isn't ad hoc. I do a mild amount of "ad hoc" parsing of the standard out though to get the convenient human readable message corresponding to each error in the YAML.

Beyond the main header error deduplication, some other smaller things in the script (with a general pattern of seemingly being written in kind of "old" python):

I would actually suggest upstreaming this under the name run-clang-tidy2.py - I know that's very strange and normally would be kind of awful. But since this is an end-user, convenience script, I don't know if there's anything wrong with leaving the existing script for backwards maintainability, and for those who prefer it, while simultaneously introducing a newer approach.

Anyway if there's interest I can share the current code, and from there we can discuss what should be added prior to merging (it's relatively minimal now).

PiotrZSL commented 2 months ago

There is this change: https://github.com/llvm/llvm-project/pull/89490 You can help with code review, and then once it land, we could thing on other improvments.

cjdb commented 2 months ago

Can we integrate the features from your script into the existing script, possibly under a flag?

quicknir commented 2 months ago

Some of the things I'm suggesting here are backwards incompatible changes. Also the script is quite short, between my various changes it's tantamount to a full rewrite 🤷 .

nicovank commented 2 months ago

Working on adding types annotations in https://github.com/llvm/llvm-project/pull/89490. Multi-threading approach: I like asyncio.gather with a semaphore for those Python scripts that just wrap another program like this one, this change is also a part of that PR, and makes a progress indicator more easy.

I am on the side of modernizing and breaking backwards compatibility if needed. e.g. I'm fine making PyYAML a required dependency to always use export-fixes for example. This is a simple Python script, no-one should really depend on it other than running it manually or possibly in CI, folks that really need the previous behavior can make a local copy of the existing version.

Willing to help with integration / code review. Maybe some of those changes can also be incremental.