This is an archive of the discontinued LLVM Phabricator instance.

Extend UnnecessaryCopyInitialization check to trigger on non-const copies that can be safely converted to const references.
ClosedPublic

Authored by flx on Feb 20 2016, 8:36 PM.

Details

Summary

Move code shared between UnnecessaryCopyInitialization and ForRangeCopyCheck into utilities files.
Add more test cases for UnnecessaryCopyInitialization and disable fixes inside of macros.

Diff Detail

Event Timeline

flx updated this revision to Diff 48607.Feb 20 2016, 8:36 PM
flx retitled this revision from to Extend UnnecessaryCopyInitialization check to trigger on non-const copies that can be safely converted to const references..
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.Feb 22 2016, 7:03 AM
clang-tidy/performance/UnnecessaryCopyInitialization.cpp
69

nit: Clang-format doesn't reflow string literals
One way to fix this is to merge all the lines of this string literal and clang-format again.

clang-tidy/performance/UnnecessaryCopyInitialization.h
18–19

Maybe just "The check detects local variable declarations ...."?

Also, should the .rst file be updated as well?

clang-tidy/utils/DeclRefExprUtils.h
22

nit: There should be an empty line after the brief summary, IIRC. (But you can look at the doxygen output and see how this looks like.)

clang-tidy/utils/FixItHintUtils.cpp
24

Lexer::getLocForEndOfToken seems to be more suitable for this purpose.

clang-tidy/utils/FixItHintUtils.h
22

Since this is now a library, we should make names clear and unambiguous. I'd pull a part of the name's meaning to the namespace and expand the name with the needed details. Maybe something like this:

namespace clang {
namespace tidy {
namespace utils {
namespace create_fixit {

FixItHint changeVarDeclToReference(...);
FixItHint changeVarDeclToConst(...);
alexfh requested changes to this revision.Feb 24 2016, 5:44 AM
alexfh edited edge metadata.
This revision now requires changes to proceed.Feb 24 2016, 5:44 AM
flx updated this revision to Diff 49306.Feb 27 2016, 8:04 PM
flx edited edge metadata.
flx removed rL LLVM as the repository for this revision.
flx marked 5 inline comments as done.
flx added inline comments.
clang-tidy/performance/UnnecessaryCopyInitialization.h
18–19

.rst file didn't exist. Added one now.

clang-tidy/utils/FixItHintUtils.h
22

Done, but put an extra underscore in the create_fix_it namespace to be consistent with the camelcase of the class.

alexfh accepted this revision.Mar 1 2016, 5:38 PM
alexfh edited edge metadata.

Looks good. Thanks!

This revision is now accepted and ready to land.Mar 1 2016, 5:38 PM
alexfh added inline comments.Mar 1 2016, 5:39 PM
clang-tidy/utils/FixItHintUtils.cpp
2

Please fix the line break.

flx updated this revision to Diff 49884.Mar 5 2016, 1:10 PM
flx edited edge metadata.
flx marked an inline comment as done.
This revision was automatically updated to reflect the committed changes.