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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
