[PATCH] D12031: Const std::move() argument ClangTidy check

[Alexander Kornienko via cfe-commits]

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