This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Do not trigger unnecessary-value-param check on methods marked as final
ClosedPublic

Authored by flx on Nov 29 2016, 8:34 PM.

Details

Diff Detail

Event Timeline

flx updated this revision to Diff 79701.Nov 29 2016, 8:34 PM
flx retitled this revision from to [clang-tidy] Do not trigger unnecessary-value-param check on methods marked as final.
flx updated this object.
flx added reviewers: sbenza, alexfh, hokein.
flx set the repository for this revision to rL LLVM.
flx added a project: Restricted Project.
flx added a subscriber: cfe-commits.
hokein accepted this revision.Nov 30 2016, 1:33 AM
hokein edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Nov 30 2016, 1:33 AM
malcolm.parsons added inline comments.
test/clang-tidy/performance-unnecessary-value-param.cpp
276

Do we want to warn about methods in templated classes at all?

284

Typo Negativ -> Negative.

flx updated this revision to Diff 79732.Nov 30 2016, 6:44 AM
flx edited edge metadata.
flx removed rL LLVM as the repository for this revision.
flx marked an inline comment as done.
flx added inline comments.
test/clang-tidy/performance-unnecessary-value-param.cpp
276

I think we should still trigger on methods where we can determine that the parameter type is expensive to copy. Note that this is not the case when the type is dependent.

test/clang-tidy/performance-unnecessary-value-param.cpp
276

We still have issues when the overriding method is not marked override or final.
Is there another way to handle this?

alexfh accepted this revision.Dec 1 2016, 6:51 AM
alexfh edited edge metadata.

LG. Looks like a strict improvement.

flx added inline comments.Dec 2 2016, 6:42 AM
test/clang-tidy/performance-unnecessary-value-param.cpp
276

I filed https://llvm.org/bugs/show_bug.cgi?id=31236 to keep track of fixing the check for this case as well.

malcolm.parsons added inline comments.Dec 2 2016, 6:50 AM
test/clang-tidy/performance-unnecessary-value-param.cpp
276

Thanks.

This revision was automatically updated to reflect the committed changes.