[clang-tools-extra] [clang-tidy] Added bugprone-exception-rethrow check (PR #86448)

[via cfe-commits]

================
@@ -0,0 +1,60 @@
+// RUN: %check_clang_tidy %s bugprone-exception-rethrow %t -- -- -fexceptions
+
+struct exception {};
+
+void correct() {
+  try {
+      throw exception();
+  } catch(const exception &) {
+      throw;
+  }
+}
+
+void correct2() {
+  try {
+      throw exception();
+  } catch(const exception &e) {
+      try {
+        throw exception();
+      } catch(...) {}
+  }
+}
+
+void not_correct() {
+  try {
+      throw exception();
+  } catch(const exception &e) {
+      throw e;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throwing a copy of the caught 'exception' exception, remove the argument to throw the original exception object [bugprone-exception-rethrow]
+  }
+}
+
+void not_correct2() {
+  try {
+      throw 5;
+  } catch(const int &e) {
+      throw e;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throwing a copy of the caught 'int' exception, remove the argument to throw the original exception object [bugprone-exception-rethrow]
+  }
+}
+
+void rethrow_not_correct() {
+  throw;
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: empty 'throw' outside a catch block without an exception can trigger 'std::terminate' [bugprone-exception-rethrow]
+}
+
+void rethrow_not_correct2() {
+  try {
+    throw;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty 'throw' outside a catch block without an exception can trigger 'std::terminate' [bugprone-exception-rethrow]
+  } catch(...) {
+  }
+}
+
+void rethrow_correct() {
+  try {
+    throw 5;
+  } catch(...) {
+    throw;
+  }
+}
----------------
isuckatcs wrote:

Please add a testcase, where the exception object is moved before it's rethrowed.
```c++
void foo() {
  try {
    throw std::unique_ptr<int>();
  } catch (std::unique_ptr<int> &uptr) {
    throw std::move(uptr);
  }
}
```
I think this is ignored because of the `unless(has(expr()))` matcher, but this might not be correct in every case. In the following example the caught exception is still copied.
```c++
void foo() {
  try {
    throw S();
  } catch (const S &s) {
    throw(1, s);
  }
}
```
I don't mind if this latter case is not handled, because it might be difficult to do so properly (imagine a constant expression here with branches, etc.), but this should be documented as a tradeoff.

There is also the following case, that will be ignored by the check, which is once again difficult to handle.
```c++
void foo() {
  try {
    throw S();
  } catch (const S &s) {
    const auto &ref = s;
    throw ref;
  }
}
```
I don't know if it worths mentioning in the docs that we don't perform any flow-sensitive analysis here, because clang-tidy in general doesn't do that, but it also integrates some Clang Static Analyzer checks, which do. It might make sense though, so that users are fully aware of the limitations and won't think the missing warning for the above snippet is a bug.

https://github.com/llvm/llvm-project/pull/86448