<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jun 26, 2014 at 1:24 AM, NAKAMURA Takumi <span dir="ltr"><<a href="mailto:geek4civic@gmail.com" target="_blank">geek4civic@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">2014-06-25 23:09 GMT+09:00 Alexander Kornienko <<a href="mailto:alexfh@google.com">alexfh@google.com</a>>:<br>

<div><div class="h5">> Author: alexfh<br>
> Date: Wed Jun 25 09:09:52 2014<br>
> New Revision: 211698<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=211698&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=211698&view=rev</a><br>
> Log:<br>
> Add clang-tidy-diff.py script to run clang-tidy and display warnings on changed lines only.<br>
><br>
> Reviewers: djasper<br>
><br>
> Reviewed By: djasper<br>
><br>
> Subscribers: cfe-commits<br>
><br>
> Differential Revision: <a href="http://reviews.llvm.org/D4288" target="_blank">http://reviews.llvm.org/D4288</a><br>
><br>
> Added:<br>
>     clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py   (with props)<br>
>     clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp<br>
><br>
> Added: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py?rev=211698&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py?rev=211698&view=auto</a><br>

> ==============================================================================<br>
> --- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py (added)<br>
> +++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py Wed Jun 25 09:09:52 2014<br>
> @@ -0,0 +1,115 @@<br>
> +#!/usr/bin/python<br>
> +#<br>
> +#===- clang-tidy-diff.py - ClangTidy Diff Checker ------------*- python -*--===#<br>
> +#<br>
> +#                     The LLVM Compiler Infrastructure<br>
> +#<br>
> +# This file is distributed under the University of Illinois Open Source<br>
> +# License. See LICENSE.TXT for details.<br>
> +#<br>
> +#===------------------------------------------------------------------------===#<br>
> +<br>
> +r"""<br>
> +ClangTidy Diff Checker<br>
> +======================<br>
> +<br>
> +This script reads input from a unified diff, runs clang-tidy on all changed<br>
> +files and outputs clang-tidy warnings in changed lines only. This is useful to<br>
> +detect clang-tidy regressions in the lines touched by a specific patch.<br>
> +Example usage for git/svn users:<br>
> +<br>
> +  git diff -U0 HEAD^ | clang-tidy-diff.py -p1<br>
> +  svn diff --diff-cmd=diff -x-U0 | \<br>
> +      clang-tidy-diff.py -fix -checks=-*,misc-use-override<br>
> +<br>
> +"""<br>
> +<br>
> +import argparse<br>
<br>
</div></div>argparse has been introduced since 2.7. I suppose we can still use 2.6.<br></blockquote><div><br></div><div>Python 2.7 has been available since 2010, and it should be available virtually everywhere. I've not seen any python-related buildbot failures after this patch, so I suppose they all use up-to-date python. Can you provide more details: where is python 2.6 still used and why do we need to support it?</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Suppressed in r211741. Could you rewrite to avoid argparse?<br></blockquote><div><br></div><div>Python 2.6 had optparse, which has been deprecated since 2.7. I don't think it's a good idea to rewrite the script using deprecated api.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class=""><div class="h5"><br>
> +import json<br>
> +import re<br>
> +import subprocess<br>
> +import sys<br>
> +<br>
> +<br>
> +def main():<br>
> +  parser = argparse.ArgumentParser(description=<br>
> +                                   'Reformat changed lines in diff. Without -i '<br>
> +                                   'option just output the diff that would be '<br>
> +                                   'introduced.')<br>
> +  parser.add_argument('-clang-tidy-binary', metavar='PATH',<br>
> +                      default='clang-tidy',<br>
> +                      help='path to clang-tidy binary')<br>
> +  parser.add_argument('-p', metavar='NUM', default=0,<br>
> +                      help='strip the smallest prefix containing P slashes')<br>
> +  parser.add_argument('-regex', metavar='PATTERN', default=None,<br>
> +                      help='custom pattern selecting file paths to reformat '<br>
> +                      '(case sensitive, overrides -iregex)')<br>
> +  parser.add_argument('-iregex', metavar='PATTERN', default=<br>
> +                      r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hpp|m|mm|inc)',<br>
> +                      help='custom pattern selecting file paths to reformat '<br>
> +                      '(case insensitive, overridden by -regex)')<br>
> +<br>
> +  parser.add_argument('-fix', action='store_true', default=False,<br>
> +                      help='apply suggested fixes')<br>
> +  parser.add_argument('-checks',<br>
> +                      help='checks filter, when not specified, use clang-tidy '<br>
> +                      'default',<br>
> +                      default='')<br>
> +  clang_tidy_args = []<br>
> +  argv = sys.argv[1:]<br>
> +  if '--' in argv:<br>
> +    clang_tidy_args.extend(argv[argv.index('--'):])<br>
> +    argv = argv[:argv.index('--')]<br>
> +<br>
> +  args = parser.parse_args(argv)<br>
> +<br>
> +  # Extract changed lines for each file.<br>
> +  filename = None<br>
> +  lines_by_file = {}<br>
> +  for line in sys.stdin:<br>
> +    match = re.search('^\+\+\+\ (.*?/){%s}(\S*)' % args.p, line)<br>
> +    if match:<br>
> +      filename = match.group(2)<br>
> +    if filename == None:<br>
> +      continue<br>
> +<br>
> +    if args.regex is not None:<br>
> +      if not re.match('^%s$' % args.regex, filename):<br>
> +        continue<br>
> +    else:<br>
> +      if not re.match('^%s$' % args.iregex, filename, re.IGNORECASE):<br>
> +        continue<br>
> +<br>
> +    match = re.search('^@@.*\+(\d+)(,(\d+))?', line)<br>
> +    if match:<br>
> +      start_line = int(match.group(1))<br>
> +      line_count = 1<br>
> +      if match.group(3):<br>
> +        line_count = int(match.group(3))<br>
> +      if line_count == 0:<br>
> +        continue<br>
> +      end_line = start_line + line_count - 1;<br>
> +      lines_by_file.setdefault(filename, []).append([start_line, end_line])<br>
> +<br>
> +  if len(lines_by_file) == 0:<br>
> +    print "No relevant changes found."<br>
> +    sys.exit(0)<br>
> +<br>
> +  line_filter_json = json.dumps(<br>
> +    [{"name" : name, "lines" : lines_by_file[name]} for name in lines_by_file],<br>
> +    separators = (',', ':'))<br>
> +<br>
> +  # Run clang-tidy on files containing changes.<br>
> +  command = [args.clang_tidy_binary]<br>
> +  command.append('-line-filter=\'' + line_filter_json + '\'')<br>
> +  if args.fix:<br>
> +    command.append('-fix')<br>
> +  if args.checks != '':<br>
> +    command.append('-checks=\'' + args.checks + '\'')<br>
> +  command.extend(lines_by_file.keys())<br>
> +  command.extend(clang_tidy_args)<br>
> +<br>
> +  sys.exit(subprocess.call(' '.join(command), shell=True))<br>
> +<br>
> +if __name__ == '__main__':<br>
> +  main()<br>
><br>
> Propchange: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py<br>
> ------------------------------------------------------------------------------<br>
>     svn:executable = *<br>
><br>
> Added: clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp?rev=211698&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp?rev=211698&view=auto</a><br>

> ==============================================================================<br>
> --- clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp (added)<br>
> +++ clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp Wed Jun 25 09:09:52 2014<br>
> @@ -0,0 +1,18 @@<br>
> +// RUN: sed 's/placeholder_for_f/f/' %s > %t.cpp<br>
> +// RUN: clang-tidy -checks=-*,misc-use-override %t.cpp -- -std=c++11 | FileCheck -check-prefix=CHECK-SANITY %s<br>
> +// RUN: not diff -u0 %s %t.cpp | python %S/../../clang-tidy/tool/clang-tidy-diff.py -checks=-*,misc-use-override -- -std=c++11 2>&1 | FileCheck %s<br>
> +struct A {<br>
> +  virtual void f() {}<br>
> +  virtual void g() {}<br>
> +};<br>
> +// CHECK-NOT: warning<br>
> +struct B : public A {<br>
> +  void placeholder_for_f() {}<br>
> +// CHECK-SANITY: [[@LINE-1]]:8: warning: Use exactly<br>
> +// CHECK: [[@LINE-2]]:8: warning: Use exactly<br>
> +  void g() {}<br>
> +// CHECK-SANITY: [[@LINE-1]]:8: warning: Use exactly<br>
> +// CHECK-NOT: warning:<br>
> +};<br>
> +// CHECK-SANITY-NOT: Suppressed<br>
> +// CHECK: Suppressed 1 warnings (1 due to line filter).<br></div></div></blockquote></div>
</div></div>