[PATCH] D61851: [clang-tidy] New option for misc-throw-by-value-catch-by-reference

[Balogh, Ádám via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL361225: [clang-tidy] New option for misc-throw-by-value-catch-by-reference (authored by baloghadamsoftware, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61851?vs=200266&id=200414#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61851/new/

https://reviews.llvm.org/D61851

Files:
  clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
  clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst


Index: clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
===================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
+++ clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
@@ -41,6 +41,8 @@
   bool isCatchVariable(const DeclRefExpr *declRefExpr);
   bool isFunctionOrCatchVar(const DeclRefExpr *declRefExpr);
   const bool CheckAnonymousTemporaries;
+  const bool WarnOnLargeObject;
+  uint64_t MaxSize; // No `const` because we have to set it in two steps.
 };
 
 } // namespace misc
Index: clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
===================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
@@ -20,7 +20,10 @@
 ThrowByValueCatchByReferenceCheck::ThrowByValueCatchByReferenceCheck(
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      CheckAnonymousTemporaries(Options.get("CheckThrowTemporaries", true)) {}
+      CheckAnonymousTemporaries(Options.get("CheckThrowTemporaries", true)),
+      WarnOnLargeObject(Options.get("WarnOnLargeObject", false)),
+      // Cannot access `ASTContext` from here so set it to an extremal value.
+      MaxSize(Options.get("MaxSize", std::numeric_limits<uint64_t>::max())) {}
 
 void ThrowByValueCatchByReferenceCheck::registerMatchers(MatchFinder *Finder) {
   // This is a C++ only check thus we register the matchers only for C++
@@ -150,8 +153,19 @@
     // If it's not a pointer and not a reference then it must be caught "by
     // value". In this case we should emit a diagnosis message unless the type
     // is trivial.
-    if (!caughtType.isTrivialType(context))
+    if (!caughtType.isTrivialType(context)) {
       diag(varDecl->getBeginLoc(), diagMsgCatchReference);
+    } else if (WarnOnLargeObject) {
+      // If the type is trivial, then catching it by reference is not dangerous.
+      // However, catching large objects by value decreases the performance.
+
+      // We can now access `ASTContext` so if `MaxSize` is an extremal value
+      // then set it to the size of `size_t`.
+      if (MaxSize == std::numeric_limits<uint64_t>::max())
+        MaxSize = context.getTypeSize(context.getSizeType());
+      if (context.getTypeSize(caughtType) > MaxSize)
+        diag(varDecl->getBeginLoc(), diagMsgCatchReference);
+    }
   }
 }
 
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
===================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
@@ -32,3 +32,18 @@
    <https://www.securecoding.cert.org/confluence/display/cplusplus/ERR09-CPP.+Throw+anonymous+temporaries>`_.
    Default is `1`.
 
+.. option:: WarnOnLargeObject
+
+   Also warns for any large, trivial object caught by value. Catching a large
+   object by value is not dangerous but affects the performance negatively. The
+   maximum size of an object allowed to be caught without warning can be set
+   using the `MaxSize` option.
+   Default is `0`.
+
+.. option:: MaxSize
+
+   Determines the maximum size of an object allowed to be caught without
+   warning. Only applicable if `WarnOnLargeObject` is set to `1`. If option is
+   set by the user to `std::numeric_limits<uint64_t>::max()` then it reverts to
+   the default value.
+   Default is the size of `size_t`.
Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -170,6 +170,11 @@
 
   Rewrites function signatures to use a trailing return type.
 
+- The :doc:`misc-throw-by-value-catch-by-reference
+  <clang-tidy/misc-throw-by-value-catch-by-reference.rst>` now supports
+  `WarnOnLargeObject` and `MaxSize` options to warn on any large trivial
+  object caught by value.
+
 Improvements to include-fixer
 -----------------------------
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61851.200414.patch
Type: text/x-patch
Size: 4337 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190521/fcea7bba/attachment.bin>