Page MenuHomePhabricator

[PATCH] clang-tidy: Bug 27731 - modernize-pass-by-value suggest using std::move for types that perform copies on move
ClosedPublic

Authored by madsravn on May 18 2016, 6:20 AM.

Details

Reviewers
alexfh
Summary

This is a patch for bug: https://llvm.org/bugs/show_bug.cgi?id=27731

I have excluded types which are trivially copyable from being called with std::move in the modernizer. In the current state another modernizer will catch them and change them back, thus creating a cyclic behaviour.

Diff Detail

Event Timeline

madsravn updated this revision to Diff 57607.May 18 2016, 6:20 AM
madsravn retitled this revision from to [PATCH] clang-tidy: Bug 27731 - modernize-pass-by-value suggest using std::move for types that perform copies on move.
madsravn updated this object.
madsravn added reviewers: alexfh, vsk, djasper, klimek.
madsravn added a subscriber: cfe-commits.
alexfh accepted this revision.May 18 2016, 6:53 AM
alexfh removed reviewers: klimek, djasper, vsk.
alexfh edited edge metadata.
alexfh added a subscriber: flx.

LG. Do you need me to submit the patch for you?

Next time please create diff with the full context (http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface).

Thank you!

This revision is now accepted and ready to land.May 18 2016, 6:55 AM

Just curious, as I'm sort of new to this. How long will it take before its merged in?

Prazek added a subscriber: Prazek.May 21 2016, 6:44 AM

You have to push it by yourself. It's ain't fun if someone do it for you :D
You have to obtain commit access http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access and then push it (you can also find in docs how to do it)

Thanks for fixing this bug, it was pissing me off.

@Prazek thanks. I will look into it :)

madsravn closed this revision.May 23 2016, 11:22 AM

Code committed.

Just curious, as I'm sort of new to this. How long will it take before its merged in?

I was waiting for an answer to the "Do you need me to submit the patch for you?" question. Apparently, the answer is "no" ;)

Did you revert the commit? I see that it is commieted, but after it I see revert.
Also please stick to convention of commit messages http://llvm.org/docs/DeveloperPolicy.html#commit-messages
Commit message like "[clang-tidy] modernize-pass-by-value bugfix" would be much better.

@alexfh I don't know how I could miss that. But I got my commit access and committed the code myself. Thanks though.

@Prazek Yes I reverted. The code made the build server (as seen on IRC) fail. So I quickly reverted. I'm gonna fix the code tonight - I had to "make clean" my entire llvm project yesterday, so that took a pretty long compiling time before I could start testing again.