[PATCH] D21223: [clang-tidy] misc-move-const-arg: Detect if result of std::move() is being passed as a const ref argument

[Martin Böhme via cfe-commits]

mboehme added inline comments.

================
Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:37
@@ +36,3 @@
+  if (BeforeArgumentsRange.isValid() && AfterArgumentsRange.isValid()) {
+    (*Diag) << FixItHint::CreateRemoval(BeforeArgumentsRange)
+            << FixItHint::CreateRemoval(AfterArgumentsRange);
----------------
alexfh wrote:
> Alternatively, this function could return a `llvm::SmallVector<FixItHint, 2>` (either empty or with the fixes) and the caller could just feed it to the result of `diag()`. Not much difference though, just pointing to this possibility.
Thanks for the suggestion! If it's OK with you, though, I'd like to keep this as-is -- I feel it makes the call-site clearer.

================
Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:42
@@ +41,3 @@
+
+CharSourceRange FileCharRangeForExpr(const Expr *TheExpr,
+                                     const SourceManager &SM,
----------------
alexfh wrote:
> With two lines of implementation and just one caller, this function doesn't seem to add any value.
I've inlined the function. (I think I originally had two places where it got called, so I factored it out, but then one of those places went away.)

================
Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:116
@@ +115,3 @@
+    // and that appears to take precendence.
+    if (SM.isMacroBodyExpansion(CallMove->getLocStart()) ||
+        SM.isMacroArgExpansion(CallMove->getLocStart())) {
----------------
alexfh wrote:
> Is this equivalent to just ignoring all cases where `CallMove->getLocStart().isMacroID()`? If not, what's the subset of the cases that would be ignored by `isMacroID()`, but not with the current condition?
Actually, having looked into this some more, it seems that the macro cases are already handled by the check "if (!FileMoveRange.isValid())" above -- so I'm deleting this entire block of code.

(I think I added the isMacroBodyExpansion() and isMacroArgExpansion() checks first before I moved the FileMoveRange.isValid() check, which then made them unnecessary.)


http://reviews.llvm.org/D21223