alexfh added a comment.

A few more comments.


================
Comment at: test/clang-tidy/move-const-arg.cpp:1
@@ +1,2 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s move-const-arg %t 
+// REQUIRES: shell
----------------
Please use check_clang_tidy.py instead:

  // RUN: %python %S/check_clang_tidy.py %s move-const-arg %t

and remove `// REQUIRES: shell`, as it's not needed any more.

================
Comment at: test/clang-tidy/move-const-arg.cpp:41
@@ +40,3 @@
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: move of const variable 
[move-const-arg]
+  // CHECK-FIXES: return x;
+}
----------------
Please make the checked code lines unique to avoid matching in a wrong place. 
FileCheck (the utility that is called by the check_clang_tidy.py script to 
verify the `// CHECK-...` lines, 
http://llvm.org/docs/CommandGuide/FileCheck.html) doesn't take the location of 
the `// CHECK-FIXES:` line in the test file into consideration, it just 
verifies that the fixed file has a subsequence of lines that matches all `// 
CHECK-FIXES` lines in that order. Here, for example, `return x;` would equally 
match, if the check would fix line 34 instead of line 39. We could solve this 
by numbering lines so that CHECK-FIXES patterns could refer to the line 
numbers, but until we do that (if we decide so), making the checked lines 
unique is the way to go (e.g. by using different variable names in each test 
case instead of just `x`).


http://reviews.llvm.org/D12031



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to