balazske added inline comments.
================ 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: > balazske wrote: > > 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. > I don't think we're too worried about having a bunch of small test headers > around -- I think it's more important the headers used to check system header > behavior be understandable as to what you're getting from them. For instance, > there are clang-tidy checks for llvm-libc that may have very different needs > from what you're doing here. > > What's more, these changes break existing tests -- I don't see any companion > changes to fix those up. On my system (Ubuntu) I do not see failing tests, and these changes do not touch files that were created before D87449, no new problems should happen. I am not against "mirroring" the POSIX API header structure into the test stub header structure, only do not like the overhead of adding these many files with just 1-2 lines (that is needed for this test) in them. It is not done this way in the clang tests either (there are multiple "system-header-simulator" files that contain every declaration usable for specific purposes at one or more tests). Also I want to have the opinion of another reviewer for this question. 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
