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

[• ehsan]

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.

[alexfh]

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).

[• ehsan]

In D29806#673329, @alexfh wrote:

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.

[• ehsan]

Gentle ping. :-)

[alexfh]

In D29806#673489, @ehsan wrote:

In D29806#673329, @alexfh wrote:

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

35–45 ↗

(On Diff #87938)

Let's remove this.

90–97 ↗

(On Diff #87938)

Let's remove this.

149 ↗

(On Diff #87938)

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

[• ehsan]

Addressed the review comments.

[alexfh]

LG

clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py

parser.add_argument('-iregex', metavar='PATTERN', default=
                       '(case insensitive, overridden by -regex)')
 
   parser.add_argument('-fix', action='store_true', default=False,
                       help='apply suggested fixes')
   parser.add_argument('-checks',
                       help='checks filter, when not specified, use clang-tidy '
                       'default',
                       default='')
+  parser.add_argument('-path', dest='build_path',
+                      help='Path used to read a compile command database.')
   parser.add_argument('-extra-arg', dest='extra_arg',
                       action='append', default=[],
                       help='Additional argument to append to the compiler '
                       'command line.')
   parser.add_argument('-extra-arg-before', dest='extra_arg_before',
                       action='append', default=[],
                       help='Additional argument to prepend to the compiler '
                       'command line.')
def main():
   command = [args.clang_tidy_binary]
   command.append('-line-filter=' + quote + line_filter_json + quote)
   if args.fix:
     command.append('-fix')
   if args.checks != '':
     command.append('-checks=' + quote + args.checks + quote)
   if args.quiet:
     command.append('-quiet')
+  if args.build_path is not None:
+    command.append('-p=%s' % args.build_path)
   command.extend(lines_by_file.keys())
   for arg in args.extra_arg:
       command.append('-extra-arg=%s' % arg)
   for arg in args.extra_arg_before:
       command.append('-extra-arg-before=%s' % arg)
   command.extend(clang_tidy_args)
 
   sys.exit(subprocess.call(' '.join(command), shell=True))
 
 if __name__ == '__main__':
   main()

clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp

 // RUN: sed 's/placeholder_for_f/f/' %s > %t.cpp
 // RUN: clang-tidy -checks=-*,modernize-use-override %t.cpp -- -std=c++11 | FileCheck -check-prefix=CHECK-SANITY %s
 // RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -- -std=c++11 2>&1 | FileCheck %s
 // RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -quiet -- -std=c++11 2>&1 | FileCheck -check-prefix=CHECK-QUIET %s
+// RUN: mkdir -p %T/compilation-database-test/
+// RUN: echo '[{"directory": "%T", "command": "clang++ -o test.o -std=c++11 %t.cpp", "file": "%t.cpp"}]' > %T/compilation-database-test/compile_commands.json
+// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -path %T/compilation-database-test 2>&1 | FileCheck -check-prefix=CHECK %s
 struct A {
   virtual void f() {}
   virtual void g() {}
 };
 // CHECK-NOT: warning:
 // CHECK-QUIET-NOT: warning:
 struct B : public A {
   void placeholder_for_f() {}