[clang-tidy] MoveConstructorInitCheck - Add parameter name to check message.

[alexfh]

clang-tidy/misc/MoveConstructorInitCheck.cpp

114

I guess, it should work without ->getName(). Could you try?

[flx]

Done.

[aaron.ballman]

I'm not opposed to showing the name, but I'm not certain I understand under what circumstances the name would be useful. Since this is triggering on move constructors, and move constructors can only have one parameter, the name seems wholly redundant, isn't it?

[flx]

In D19849#419751, @aaron.ballman wrote:

I'm not opposed to showing the name, but I'm not certain I understand under what circumstances the name would be useful. Since this is triggering on move constructors, and move constructors can only have one parameter, the name seems wholly redundant, isn't it?

The case this is covering is when a value parameter can be moved, of which there can be many.

[aaron.ballman]

In D19849#419752, @flx wrote:

In D19849#419751, @aaron.ballman wrote:

I'm not opposed to showing the name, but I'm not certain I understand under what circumstances the name would be useful. Since this is triggering on move constructors, and move constructors can only have one parameter, the name seems wholly redundant, isn't it?

The case this is covering is when a value parameter can be moved, of which there can be many.

Ah, I think I was thrown by the fact that the only instance of the diagnostic was with a move constructor in a check called misc-move-constructor-init. :-P

[alexfh]

LG. Thanks!

clang-tidy/misc/MoveConstructorInitCheck.cpp

const auto *MovableParam =
       Result.Nodes.getNodeAs<ParmVarDecl>("movable-param");
   const auto *ConstructorDecl =
       Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor-decl");
   const auto *InitArg = Result.Nodes.getNodeAs<DeclRefExpr>("init-arg");
   // If the parameter is referenced more than once it is not safe to move it.
   if (parmVarDeclRefExprOccurences(*MovableParam, *ConstructorDecl,
                                    *Result.Context) > 1)
     return;
-  auto DiagOut =
-      diag(InitArg->getLocStart(), "value argument can be moved to avoid copy");
+  auto DiagOut = diag(InitArg->getLocStart(),
+                      "value argument %0 can be moved to avoid copy")
+                 << MovableParam;
   DiagOut << FixItHint::CreateReplacement(
       InitArg->getSourceRange(),
       (Twine("std::move(") + MovableParam->getName() + ")").str());
   if (auto IncludeFixit = Inserter->CreateIncludeInsertion(
           Result.SourceManager->getFileID(InitArg->getLocStart()), "utility",
           /*IsAngled=*/true)) {
     DiagOut << *IncludeFixit;
   }

test/clang-tidy/misc-move-constructor-init.cpp

 struct TriviallyCopyable {
   TriviallyCopyable() = default;
   TriviallyCopyable(TriviallyCopyable &&) = default;
   TriviallyCopyable(const TriviallyCopyable &) = default;
 };
 
 struct Positive {
   Positive(Movable M) : M_(M) {}
-  // CHECK-MESSAGES: [[@LINE-1]]:28: warning: value argument can be moved to avoid copy [misc-move-constructor-init]
+  // CHECK-MESSAGES: [[@LINE-1]]:28: warning: value argument 'M' can be moved to avoid copy [misc-move-constructor-init]
   // CHECK-FIXES: Positive(Movable M) : M_(std::move(M)) {}
   Movable M_;
 };
 
 struct NegativeMultipleInitializerReferences {
   NegativeMultipleInitializerReferences(Movable M) : M_(M), n_(M) {}
   Movable M_;
   Movable n_;