[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp:279-282 + llvm::DenseSet result; + result.insert(std::begin(posixFunctions), std::end(posixFunctions)); + result.insert(std::begin(glibcFunctions), std::end(glibcFunctions)); + r

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

njames93 added a comment. Also can this be ran through clang-tidy, feeling a few naming violations are in here. If you use arc to upload your patches you'll get lint warnings about clang-tidy and clang-format. Comment at: clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp:27

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

segoon added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp:295 + case MtUnsafeCheck::LibcType::Any: +return hasAnyName(anyFunctions); + } lebedev.ri wrote: > return anyOf(hasAnyName(posixFunctions), hasAnyName(glibcFunctio

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

lebedev.ri added a comment. `Libc` option name doesn't really make sense to me, maybe `FunctionSet` would fit better. Comment at: clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp:32-33 + +// Initial list was extracted from gcc documentation +static const StringRef glibcFunc

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

njames93 added a comment. Just a general drive by point, is it wise to add a new tidy module to clang-tidy called `threads` (or similar). We have got a few other checks related to multi-threaded code in the pipeline (D77493 , D75229

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

segoon added a comment. In D90944#2380910 , @lebedev.ri wrote: > What i would however like to be improved, is better docs. I hope I'll addressed your questions in documentation. Please tell me whether you still have any unanswered questions. CHANGES SI

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

segoon updated this revision to Diff 305460. segoon added a comment. - minor changes to docs CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90944/new/ https://reviews.llvm.org/D90944 Files: clang-tools-extra/clang-tidy/misc/CMakeLists.txt clang-tools-extra/clang-tidy/misc/MiscTidyMo

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

segoon updated this revision to Diff 304189. segoon edited the summary of this revision. segoon added a comment. - add `Libc` option - improve docs CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90944/new/ https://reviews.llvm.org/D90944 Files: clang-tools-extra/clang-tidy/misc/CMakeL

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

lebedev.ri added a comment. While i share concern about false-positives, it is literally impossible to avoid them here, and this should be viewed more as an enforcement tool (don't use thread-unsafe fns), not bug-detection check. What i would however like to be improved, is better docs. I'm gue

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

segoon updated this revision to Diff 303532. segoon added a comment. - use static instead of namespace {} - don't use SourceRange() - revert unrelated changes to .rst CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90944/new/ https://reviews.llvm.org/D90944 Files: clang-tools-extra/cla

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

segoon added a comment. In D90944#2379688 , @njames93 wrote: > It appears to not check for signs that the code is running in a multi > threaded manner, This will result in many false positives in code that is > known to be single threaded. I'm not sure

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

njames93 added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:33 `abseil-upgrade-duration-conversions `_, "Yes" - `altera-struct-pack-align `_, + `altera-struct-pack-align `_, "Yes" `android-cloexec-accept `_, "Yes" se

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

segoon marked an inline comment as done. segoon added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:33 `abseil-upgrade-duration-conversions `_, "Yes" - `altera-struct-pack-align `_, + `altera-struct-pack-align `_, "Yes" `android-cloex

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

njames93 added a comment. It appears to not check for signs that the code is running in a multi threaded manner, This will result in many false positives in code that is known to be single threaded. Comment at: clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp:19 + +namespa

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

segoon marked an inline comment as done. segoon added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe.cpp:3-4 + +#include +#include + lebedev.ri wrote: > Tests should be hermetic, they can not use headers from system fixed

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

segoon updated this revision to Diff 303499. segoon added a comment. don't include any system headers in tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90944/new/ https://reviews.llvm.org/D90944 Files: clang-tools-extra/clang-tidy/misc/CMakeLists.txt clang-tools-extra/clang-ti

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

lebedev.ri added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe.cpp:3-4 + +#include +#include + Tests should be hermetic, they can not use headers from system Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

segoon created this revision. segoon added a reviewer: alexfh. Herald added subscribers: cfe-commits, lxfind, modocache, xazax.hun, mgorny. Herald added a project: clang. segoon requested review of this revision. Checks for some thread-unsafe functions against a black list of known-to-be-unsafe f