[PATCH] D90851: [clang-tidy] Extending bugprone-signal-handler with POSIX functions.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

In D90851#2385744 <https://reviews.llvm.org/D90851#2385744>, @aaron.ballman wrote:

> One thing that's not clear to me in the patch is what the behavior should be when the user specifies that they want the POSIX set but they're compiling for a non-POSIX system like Windows. Do you have thoughts on that situation?

I think that really it does not matter on what system the checker runs, at least from the check's point of view. The check itself is not dependent on the compilation target. If the compile target is not POSIX compliant but the flag is still set to POSIX it may indicate a problem but how to detect this? If we could detect this automatically the new option would not be needed, POSIX mode could be enabled automatically.
Because this issue maybe it is safer to have the **minimal** as the default option?



================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h:25
 public:
+  enum class AsyncSafeFunctionSetType { Minimal, POSIX };
+
----------------
aaron.ballman wrote:
> I don't think this needs to be public?
The `getEnumMapping` function uses it that is external to the class.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/system-header-posix-api.h:1
-//===--- signal.h - Stub header for tests -----------------------*- C++ -*-===//
+//===--- system-header-posix-api.h - Stub header for tests ------*- C++ -*-===//
 //
----------------
aaron.ballman wrote:
> I think we should strive to replicate the system headers rather than fake up a system header (these headers can be used by more than one check). So I think we want to keep signal.h and stdlib.h, and should include the POSIX-specific functionality with a macro. This also helps us test the behavior on systems like Windows which are not POSIX systems.
My concern was to add these many small header files with just some functions in them. For `accept` more than one header is needed if we want to exactly replicate the system files. And data types like `size_t` should have a common header too. So I decided to have one header that contains all system functions and data types. This can be used by multiple tests and extended as needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90851