This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Add -path option to clang-tidy-diff.py
ClosedPublic

Authored by ehsan on Feb 9 2017, 6:47 PM.

Details

Summary

This flag allows specifying a custom path for the compilation
database. Unfortunately we can't use the -p flag like other
clang-tidy tools because it's already taken.

Event Timeline

ehsan created this revision.Feb 9 2017, 6:47 PM
alexfh requested changes to this revision.Feb 10 2017, 3:20 AM

What's your use case? Can it be addressed by just forwarding the -p flag to clang-tidy?

The script shouldn't know anything about implementation details of the compilation database being used (since it can be something other than JSON compilation database).

This revision now requires changes to proceed.Feb 10 2017, 3:20 AM

What's your use case? Can it be addressed by just forwarding the -p flag to clang-tidy?

I just need to pass the full path to the compilation DB to clang-tidy. The problem is that invoking clang-tidy-diff.py -- -p PATH will run clang-tidy -- -p PATH, in order words adding -p to the compiler command line, not clang-tidy's.

The script shouldn't know anything about implementation details of the compilation database being used (since it can be something other than JSON compilation database).

I copied the code to look for the DB verbatim from run-clang-tidy.py. I personally don't need the search logic, and just tried to keep this consistent with run-clang-tidy.py. I'd be happy to remove the search logic if you prefer that.

Gentle ping. :-)

What's your use case? Can it be addressed by just forwarding the -p flag to clang-tidy?

I just need to pass the full path to the compilation DB to clang-tidy. The problem is that invoking clang-tidy-diff.py -- -p PATH will run clang-tidy -- -p PATH, in order words adding -p to the compiler command line, not clang-tidy's.

I meant adding -p flag to the script and forwarding it to clang-tidy the same way as -quiet, -checks, etc. But there's already -p flag with a different meaning, so calling the new flag -path makes sense. I would still just forward it, if it's present.

The script shouldn't know anything about implementation details of the compilation database being used (since it can be something other than JSON compilation database).

I copied the code to look for the DB verbatim from run-clang-tidy.py. I personally don't need the search logic, and just tried to keep this consistent with run-clang-tidy.py. I'd be happy to remove the search logic if you prefer that.

I see. I guess, this is needed as an optimization or something like that. Not useful for clang-tidy-diff.py.

clang-tidy/tool/clang-tidy-diff.py
34–44

Let's remove this.

78–85

Let's remove this.

129

Let's only add this flag when args.build_path is not None.

ehsan updated this revision to Diff 88879.Feb 17 2017, 6:24 AM
ehsan edited edge metadata.

Addressed the review comments.

ehsan marked 3 inline comments as done.Feb 17 2017, 6:27 AM
This revision is now accepted and ready to land.Feb 17 2017, 11:22 AM
This revision was automatically updated to reflect the committed changes.