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

via cfe-commits cfe-commits at lists.llvm.org
Sun Apr 28 14:35:17 PDT 2024


================
@@ -0,0 +1,103 @@
+// RUN: %check_clang_tidy %s bugprone-exception-rethrow %t -- -- -fexceptions
+
+struct exception {};
+
+namespace std {
+  template <class T>
+  T&& move(T &x) {
+    return static_cast<T&&>(x);
+  }
+}
+
+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 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_correct3() {
+  try {
+      throw exception();
+  } catch(const exception &e) {
+      throw exception(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_correct4() {
+  try {
+      throw exception();
+  } catch(exception &e) {
+      throw std::move(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_correct5() {
+  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 with no operand triggers 'std::terminate()' [bugprone-exception-rethrow]
+}
+
+void rethrow_not_correct2() {
+  try {
+    throw;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty 'throw' outside a catch block with no operand triggers 'std::terminate()' [bugprone-exception-rethrow]
+  } catch(...) {
+  }
+}
+
+void rethrow_correct() {
+  try {
+    throw 5;
+  } catch(...) {
+    throw;
+  }
+}
+
+void rethrow_in_lambda() {
+  try {
+    throw 5;
+  } catch(...) {
+    auto lambda = [] { throw; };
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: empty 'throw' outside a catch block with no operand triggers 'std::terminate()' [bugprone-exception-rethrow]
----------------
isuckatcs wrote:

This is actually correct if the lambda is called inside the catch block, or not called at all.

See the snippet below on [godbolt](https://godbolt.org/z/xPhx4Pfa4).
```c++
void rethrow_in_lambda() {
  try {
    throw 5;
  } catch(...) {
    auto lambda = [] { throw; };
    lambda();
  }
}
```

The issue only happens, if the lambda escapes the `catch` block.

See my original example on [godbolt](https://godbolt.org/z/Yxoxso3ra):
```c++
void foo() {
  std::function<void()> fn;

  try {
    throw 1;
  } catch (...) {
    fn = [] { throw; }; // the lambda and the `throw` escape the catch block
  }

  fn();
}
```
I think besides using a lambda, the only other way this can happen is the one below.
```c++
void rethrow_in_lambda() {
    std::function<void()> fn;

    try {
        throw 1;
    } catch (...) {
        struct wrapper {
            static void foo() { throw; };
        };

        fn = wrapper::foo;
    }

    fn();
}
```
I guess to be 100% accurate in detecting these cases, we need flow sensitive analysis*, so I think we should just ignore them and mention it in the docs too. Finding these `throw;` statements is something we already do with `ASTMatcher`s, so ignoring them will work.

*the `throw;` escaping can happen in any nested function. E.g.:
```c++
#include <functional>

std::function<void()> fn;

void escape(std::function<void()> &ref) {
    fn = ref;
}

void rethrow_in_lambda() {
    try {
        throw 1;
    } catch (...) {
        std::function<void()> lambda = [] { throw; };

        escape(lambda);
    }
}

int main() {
    rethrow_in_lambda();
    fn();

    return 0;
}
```

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


More information about the cfe-commits mailing list