[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

Whisperity via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 9 01:13:08 PDT 2020


whisperity added a comment.

In general, the test files should be cleaned up.



================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-std::thread.cpp:1
+// RUN: %check_clang_tidy %s bugprone-signal-in-multithreaded-program %t \
+// RUN: -config='{CheckOptions: \
----------------
Please rename this file to something that doesn't conatain `:` in the name. Windows peoples' computers will explode if it's merged like this...


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-std::thread.cpp:5-6
+
+typedef unsigned long int thrd_t;
+typedef int (*thrd_start_t)(void *);
+typedef int sig_atomic_t;
----------------
Are these definitions needed for C++ test?


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-std::thread.cpp:32
+
+int main(void) {
+  signal(SIGUSR1, handler);
----------------
As far as I know, `int main(void)` is **not** valid C++ code. Only `int main()` and `int main(int, char**)` are valid, see `[basic.start.main]`/2.

If we are writing C++ test code, let's make it appear as if it was C++ code.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-std::thread.cpp:1
+// RUN: %check_clang_tidy %s bugprone-signal-in-multithreaded-program %t
+
----------------
Same rename here.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-thrd_create.cpp:1
+// RUN: %check_clang_tidy %s bugprone-signal-in-multithreaded-program %t
+
----------------
thrd_create is a C standard function, and this file looks like a C file to me, so why is the extension `.cpp`?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D75229





More information about the cfe-commits mailing list