This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] New option for misc-throw-by-value-catch-by-reference
ClosedPublic

Authored by baloghadamsoftware on May 13 2019, 4:17 AM.

Details

Summary

Catching trivial objects by value is not dangerous but may be inefficient if they are too large. This patch adds an option WarnOnLargeObject to the checker to also warn if such an object is caught by value. An object is considered as "large" if its size is greater than MaxSize which is another option. Default value is the machine word of the architecture (size of the type size_t).

Diff Detail

Repository
rL LLVM

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMay 13 2019, 4:17 AM
torbjoernk added inline comments.
docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
35–46 ↗(On Diff #199245)

I think it might be worth adding a note on the fallback to sizeof(size_t) if MaxSize is not set or manually set to std::numeric_limits<uint64_t>::max() by the user.

aaron.ballman added inline comments.May 13 2019, 12:41 PM
clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
25 ↗(On Diff #199245)

Missing a full stop at the end of the comment.

docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
37–40 ↗(On Diff #199245)

large trivial object -> large, trivial object
affects the perofrmance -> affects the performance
using option MaxSize -> using the MaxSize option.

lebedev.ri added inline comments.May 14 2019, 2:19 AM
docs/ReleaseNotes.rst
182–184 ↗(On Diff #199245)

for check :doc:..<..> check

(repeated 'check' word)

Updated according to the comments.

baloghadamsoftware marked 4 inline comments as done.May 20 2019, 6:28 AM

Looks good

clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
45 ↗(On Diff #200266)

because

aaron.ballman accepted this revision.May 20 2019, 7:05 AM

LGTM (with the typo fix).

This revision is now accepted and ready to land.May 20 2019, 7:05 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 21 2019, 12:23 AM