This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by flx on May 2 2016, 6:57 PM.

Diff Detail

Event Timeline

flx updated this revision to Diff 55935.May 2 2016, 6:57 PM
flx retitled this revision from to [clang-tidy] MoveConstructorInitCheck - Add parameter name to check message..
flx updated this object.
flx added a reviewer: alexfh.
flx set the repository for this revision to rL LLVM.
flx added a subscriber: cfe-commits.
alexfh added inline comments.May 2 2016, 7:15 PM
clang-tidy/misc/MoveConstructorInitCheck.cpp
114

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

alexfh requested changes to this revision.May 3 2016, 1:04 AM
alexfh edited edge metadata.
This revision now requires changes to proceed.May 3 2016, 1:04 AM
flx updated this revision to Diff 55975.May 3 2016, 5:21 AM
flx edited edge metadata.
flx removed rL LLVM as the repository for this revision.
flx marked an inline comment as done.

Done.

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 added a comment.May 3 2016, 5:28 AM

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.

In D19849#419752, @flx 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

flx edited edge metadata.May 3 2016, 5:56 AM
flx set the repository for this revision to rL LLVM.
alexfh accepted this revision.May 3 2016, 6:53 AM
alexfh edited edge metadata.

LG. Thanks!

This revision is now accepted and ready to land.May 3 2016, 6:53 AM
This revision was automatically updated to reflect the committed changes.