This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Add script to maintain list of fast clang-tidy checks
ClosedPublic

Authored by sammccall on Nov 22 2022, 5:51 AM.

Details

Summary

The plan is to intersect this list with the checks selected per config.
This is not yet done, but the initial list is checked in as a baseline.

https://github.com/clangd/clangd/issues/1337

Diff Detail

Event Timeline

sammccall created this revision.Nov 22 2022, 5:51 AM
Herald added a project: Restricted Project. · View Herald Transcript
sammccall requested review of this revision.Nov 22 2022, 5:51 AM
kadircet added inline comments.Nov 23 2022, 1:30 AM
clang-tools-extra/clangd/TidyFastChecks.py
32

file to use for benchmark?

42

what does -P do? shouldn't the latter be -DFAST?

71

comment seems to be incomplete here

71

it might be useful to include name of the file benchmarks were performed on in the output.

84

i don't see the point in including delta in the output if we're also making the decision here. is it mostly for debugging purposes? e.g. when updating the list we get to see the difference?

sammccall marked 3 inline comments as done.Nov 23 2022, 1:54 AM
sammccall added inline comments.
clang-tools-extra/clangd/TidyFastChecks.py
42

-P avoids emitting # 0 /path/to/Sema.cpp lines. Added comments

Fixed -DFAST, I must have somehow mangled this after running it!

84

Yes, that's exactly the reason. Can make it a comment instead if you like, but that makes ad-hoc analysis slightly harder.

sammccall updated this revision to Diff 477424.Nov 23 2022, 2:18 AM
sammccall marked an inline comment as done.

review comments

kadircet accepted this revision.Nov 23 2022, 2:23 AM
kadircet added inline comments.
clang-tools-extra/clangd/TidyFastChecks.inc
3

can you also re-run the script before checking in (or update here, since re-running all might take a while, but would be a good way to test history preserving logic)

clang-tools-extra/clangd/TidyFastChecks.py
77

s/perncentage/percentage

84

if that's the case no need. just wanted to make sure about it.

This revision is now accepted and ready to land.Nov 23 2022, 2:23 AM
sammccall updated this revision to Diff 477443.Nov 23 2022, 3:40 AM

Script reads/writes from file rather than redirecting output.
Update .inc file to show "no-op" changes.

This revision was landed with ongoing or failed builds.Nov 28 2022, 5:29 AM
This revision was automatically updated to reflect the committed changes.