<div dir="ltr">It seems like we can't require Python 2.7 for running tests, but I'm also against rewriting the script using deprecated APIs. I'd better make this test conditional on the availability of Python 2.7. What do you think?<div>
<br></div><div>Here's the patch which implements this:<br><div><br></div><div><pre style="color:rgb(0,0,0)">Index: test/clang-tidy/clang-tidy-diff.cpp
===================================================================
--- test/clang-tidy/clang-tidy-diff.cpp (revision 211769)
+++ test/clang-tidy/clang-tidy-diff.cpp (working copy)
@@ -1,6 +1,7 @@
+// REQUIRES: python27
// RUN: sed 's/placeholder_for_f/f/' %s > %t.cpp
// RUN: clang-tidy -checks=-*,misc-use-override %t.cpp -- -std=c++11 | FileCheck -check-prefix=CHECK-SANITY %s
-// R_U_N: 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
+// 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
struct A {
virtual void f() {}
virtual void g() {}
Index: test/lit.cfg
===================================================================
--- test/lit.cfg (revision 211769)
+++ test/lit.cfg (working copy)
@@ -174,3 +174,8 @@
# ANSI escape sequences in non-dumb terminal
if platform.system() not in ['Windows']:
config.available_features.add('ansi-escape-sequences')
+
+import sys
+# Scripts using argparse need Python 2.7.
+if sys.version_info >= (2, 7):
+ config.available_features.add('python27')</pre><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jun 26, 2014 at 1:45 AM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.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"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div><div class="h5">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" target="_blank">alexfh@google.com</a>>:<br>
<div><div>> 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></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 class="">
<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><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><div class="h5">
<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><div><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>
</div></div>
</blockquote></div><br>
</div></div></div></div>