This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Add a little python script that can run clang-tidy and apply fixes over an entire codebase.
ClosedPublic

Authored by bkramer on Sep 4 2014, 5:11 AM.

Details

Summary

Ever wanted to fix all the header guards in clang? Now it's easy.

Make sure clang-tidy is in $PATH and a compilation database is available.
$ ./run-clang-tidy.py -checks=llvm-header-guard -fix
... get coffee (or more CPU cores) ...
$ svn diff

Some may argue that this is just a glorified xargs -P, but it does a bit more ;)

Diff Detail

Repository
rL LLVM

Event Timeline

bkramer updated this revision to Diff 13249.Sep 4 2014, 5:11 AM
bkramer retitled this revision from to [clang-tidy] Add a little python script that can run clang-tidy and apply fixes over an entire codebase..
bkramer updated this object.
bkramer added a reviewer: alexfh.
bkramer added a subscriber: Unknown Object (MLST).
alexfh added inline comments.Sep 4 2014, 6:57 AM
clang-tidy/tool/run-clang-tidy.py
33 ↗(On Diff #13249)

I'd use a more specific term like "Compilation database".

108 ↗(On Diff #13249)

Why not import it in the beginning?

120 ↗(On Diff #13249)

I'm still a bit concerned about reimplementing xargs in Python. Especially if you have to handle ctrl-c manually.

bkramer added inline comments.Sep 4 2014, 7:18 AM
clang-tidy/tool/run-clang-tidy.py
108 ↗(On Diff #13249)

Mostly because it's heavyweight and we only need this tiny bit of information. Thinking more about it I could also move it to the top.

120 ↗(On Diff #13249)

In fact I had a version lying around that just calls xargs, but I don't know how to make it generate a random file name for each clang-tidy invocation. We use it to write out the fixits yaml.

bkramer updated this revision to Diff 13262.Sep 4 2014, 7:18 AM

Move import to the top.
Make it more obvious that we're refering to the compilation db.

klimek added a subscriber: klimek.Sep 8 2014, 3:53 AM
klimek added inline comments.
clang-tidy/tool/run-clang-tidy.py
11 ↗(On Diff #13262)

FIXME

13 ↗(On Diff #13262)

Is that for the \ at the end of the run-clang-tidy invocation?

63–66 ↗(On Diff #13262)

The second branch might need a comment.

71–72 ↗(On Diff #13262)

Unsafe how? Also, why is it ok for us?

117 ↗(On Diff #13262)

Call that file_name_re or something? Then you can remove the comment on bigre.search...

127–130 ↗(On Diff #13262)

Since we have to break anyway, call it invocation. Otherwise I'll always read it as invitation. Which is kinda funny, but in a sad way.

144–145 ↗(On Diff #13262)

Uuuh. Don't we have some way to pool() multiple processes?

147–153 ↗(On Diff #13262)

This reads like one of Shakespeare's tragicomedies. Just less entertaining. Oh, Python.

156–162 ↗(On Diff #13262)

Pull into a function.

158–159 ↗(On Diff #13262)

Indent.

bkramer updated this revision to Diff 13384.Sep 8 2014, 4:42 AM
  • Update & add comments.
  • Pulled apply_fixes into a function.
bkramer added inline comments.Sep 8 2014, 4:49 AM
clang-tidy/tool/run-clang-tidy.py
144–145 ↗(On Diff #13262)

There is multiprocessing.Pool which I couldn't get to work properly (it's part of the thread-with-processes emulation and adds all kinds of weird behavior to child processes). Then there's select which doesn't work on windows. Just sleeping seemed like the best option.

bkramer updated this revision to Diff 13385.Sep 8 2014, 5:03 AM

Remove experiment that broke everything, sorry.

bkramer updated this revision to Diff 13391.Sep 8 2014, 6:06 AM

Replace Popen + sleep hack with thread hack.

klimek added inline comments.Sep 8 2014, 6:49 AM
clang-tidy/tool/run-clang-tidy.py
76 ↗(On Diff #13391)

Use mkstemp!

bkramer updated this revision to Diff 13395.Sep 8 2014, 6:56 AM

Use mkstemp!

klimek accepted this revision.Sep 8 2014, 7:05 AM
klimek edited edge metadata.

LG

This revision is now accepted and ready to land.Sep 8 2014, 7:05 AM
Diffusion closed this revision.Sep 8 2014, 7:11 AM
Diffusion updated this revision to Diff 13399.

Closed by commit rL217368 (authored by d0k).

alexfh added inline comments.Sep 8 2014, 7:20 AM
clang-tidy/tool/run-clang-tidy.py
120 ↗(On Diff #13249)

Actually, if we want to support Windows properly, we can't rely on xargs and bash. So it may be a reasonable sacrifice to the gods of portability.