[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

aaron.ballman added a comment. In D33825#2246369 , @jfb wrote: > MSC54-CPP refers to POF, which as I pointed out above isn't relevant anymore. > I'd much rather have a diagnostic which honors the state of things after > http://wg21.link/p0270. I agree,

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

jfb requested changes to this revision. jfb added a comment. This revision now requires changes to proceed. MSC54-CPP refers to POF, which as I pointed out above isn't relevant anymore. I'd much rather have a diagnostic which honors the state of things after http://wg21.link/p0270. Additionally

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

bruntib updated this revision to Diff 288777. bruntib added a comment. Note texts are now describing what kind of C++ constructs were used. The warning message also explains better the reason of the issue: "signal handlers must be plain old functions, C++-only constructs are not allowed" Repos

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

bruntib added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cert-signal-handler-must-be-plain-old-function.cpp:22 + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not use C++ constructs in signal handlers [cert-msc54-cpp] + // CHECK-MESSAGES: :[[@LI

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

bruntib updated this revision to Diff 288774. bruntib added a comment. Test file has been extended with the second line of the "note" diagnostic message which designates the location of the suspicious "C++ construct". The full clang-tidy output looks like this: llvm-project/clang-tools-extra/te

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

jfb requested changes to this revision. jfb added a comment. This revision now requires changes to proceed. Herald added a subscriber: dexonsmith. Please consider these changes, and whether this is relevant as implemented: http://wg21.link/p0270 Comment at: clang-tools-extra/

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

bruntib updated this revision to Diff 288617. bruntib added a comment. Update lincense. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D33825/new/ https://reviews.llvm.org/D33825 Files: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp clang-t

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

bruntib updated this revision to Diff 288606. bruntib added a comment. Herald added a subscriber: mgehre. I rebased the patch so it compiles with master version of LLVM/Clang. I did no other change, so I would like if this patch would be committed on behalf of @NorenaLeonetti if the patch is acc

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

bruntib commandeered this revision. bruntib added a reviewer: NorenaLeonetti. bruntib added a comment. Herald added subscribers: martong, steakhal, jfb. I'll rebase this patch and continue its development. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D33825/new/ h

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

aaron.ballman added a comment. Aside from a few small nits, this looks reasonable. Have you run it over any large code bases that use signals to test the quality of the check? Comment at: docs/clang-tidy/checks/cert-msc54-cpp.rst:23 +extern "C" void cpp_signal_handler(int

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

NorenaLeonetti updated this revision to Diff 113652. Repository: rL LLVM https://reviews.llvm.org/D33825 Files: clang-tidy/cert/CERTTidyModule.cpp clang-tidy/cert/CMakeLists.txt clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.cpp clang-tidy/cert/SignalHandlerMustBePlainOldFunc

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

jklaehn added inline comments. Comment at: docs/ReleaseNotes.rst:62 +<<< 2f301f50187ede4b9b8c7456ac4a67b9f7418004 - Renamed checks to use correct term "implicit conversion" instead of "implicit You missed a conflict marker here (and below in line 149). R

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

NorenaLeonetti updated this revision to Diff 113651. Herald added a subscriber: JDevlieghere. Repository: rL LLVM https://reviews.llvm.org/D33825 Files: clang-tidy/cert/CERTTidyModule.cpp clang-tidy/cert/CMakeLists.txt clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.cpp clang-

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

NorenaLeonetti marked 16 inline comments as done. NorenaLeonetti added a comment. In https://reviews.llvm.org/D33825#772688, @aaron.ballman wrote: > This check is an interesting one. The rules around what is signal safe are > changing for C++17 to be a bit more lenient than what the rules are fo

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

aaron.ballman added a comment. This check is an interesting one. The rules around what is signal safe are changing for C++17 to be a bit more lenient than what the rules are for C++14. CERT's rule is written against C++14, and so the current behavior matches the rule wording. However, the *inte

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

Eugene.Zelenko added inline comments. Comment at: clang-tidy/cert/CERTTidyModule.cpp:64 +"cert-msc54-cpp"); // C checkers // DCL checker -> check. Comment at: docs/ReleaseNotes.rst:73 + + Checks if a signal handler is not a p

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

NorenaLeonetti created this revision. NorenaLeonetti added a project: clang-tools-extra. Herald added subscribers: xazax.hun, mgorny. Based on CERT-MSC54-CPP Repository: rL LLVM https://reviews.llvm.org/D33825 Files: clang-tidy/cert/CERTTidyModule.cpp clang-tidy/cert/CMakeLists.txt cla