[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

PaulkaToast created this revision. PaulkaToast added a reviewer: sivachandra. PaulkaToast added projects: clang-tools-extra, libc-project. Herald added subscribers: cfe-commits, MaskRay, xazax.hun, mgorny. Herald added a project: clang. This adds a new module to enforce standards specific to the l

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

njames93 added a comment. The test cases need fixing as they are causing the build to fail. Also would it be wise to add a .clang-tidy file to libc/ to enable this module for that subdirectory? Comment at: clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt:17 + \ No newli

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

Eugene.Zelenko added a comment. Please mention new module in docs/clang-tidy/index.rst and Release Notes (new modules section is above new checks one and please add subsubsection). Comment at: clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp:21 + void addCheckFact

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

abrachet added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp:40 +SrcMgr::CharacteristicKind FileType) { + if (SrcMgr::isSystem(FileType)) { +if (!SM.isInMainFile(HashLoc)) { Could you whitelist the

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

MaskRay added a comment. I am of the feeling that this check should not be llvm-libc specific. It is a general need that certain system headers are not desired. This patch can provide a general mechanism (some whitelists and blacklists). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp:40 +SrcMgr::CharacteristicKind FileType) { + if (SrcMgr::isSystem(FileType)) { +if (!SM.isInMainFile(HashLoc)) { abrachet wrote: > Could

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp:40 +SrcMgr::CharacteristicKind FileType) { + if (SrcMgr::isSystem(FileType)) { +if (!SM.isInMainFile(HashLoc)) { njames93 wrote: >

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

PaulkaToast added a comment. In D75332#1897487 , @njames93 wrote: > The test cases need fixing as they are causing the build to fail. Done. > Also would it be wise to add a .clang-tidy file to libc/ to enable this > module for that subdirectory? Yes,

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

PaulkaToast updated this revision to Diff 247794. PaulkaToast marked 11 inline comments as done. Herald added subscribers: jfb, arphaman. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75332/new/ https://reviews.llvm.org/D75332 Files: clang-tools-e

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

abrachet added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp:40 +SrcMgr::CharacteristicKind FileType) { + if (SrcMgr::isSystem(FileType)) { +if (!SM.isInMainFile(HashLoc)) { PaulkaToast wrote: > aa

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

PaulkaToast updated this revision to Diff 247960. PaulkaToast marked 2 inline comments as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75332/new/ https://reviews.llvm.org/D75332 Files: clang-tools-extra/clang-tidy/CMakeLists.txt clang-too

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

MaskRay added a comment. > `RestrictSystemLibcHeadersCheck` As I commented previously, I think the checker name should be generalized, as it does not need to be coupled with llvm-libc. Other projects may have similar needs. For example, they don't want to accidentally include a system zlib.h ->

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

MaskRay added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/llvmlibc-restrict-system-libc-headers.rst:16-20 +This check is necesary because accidentally including sytem libc headers can +lead to subtle and hard to detect bugs. For example consider a syste

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

PaulkaToast updated this revision to Diff 248044. PaulkaToast marked 2 inline comments as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75332/new/ https://reviews.llvm.org/D75332 Files: clang-tools-extra/clang-tidy/CMakeLists.txt clang-too

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

PaulkaToast added a comment. In D75332#1903570 , @MaskRay wrote: > > `RestrictSystemLibcHeadersCheck` > > As I commented previously, I think the checker name should be generalized, as > it does not need to be coupled with llvm-libc. Other projects may hav

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/llvmlibc-restrict-system-libc-headers.rst:18 +lead to subtle and hard to detect bugs. For example consider a system libc +whose `dirent` struct has slightly different field ordering than l

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

MaskRay added a subscriber: juliehockett. MaskRay added a comment. In D75332#1904264 , @PaulkaToast wrote: > In D75332#1903570 , @MaskRay wrote: > > > > `RestrictSystemLibcHeadersCheck` > > > > As I commented previo

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

PaulkaToast updated this revision to Diff 248059. PaulkaToast marked an inline comment as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75332/new/ https://reviews.llvm.org/D75332 Files: clang-tools-extra/clang-tidy/CMakeLists.txt clang-too

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

aaron.ballman added a comment. In D75332#1904317 , @MaskRay wrote: > In D75332#1904264 , @PaulkaToast > wrote: > > > In D75332#1903570 , @MaskRay wrote: > > > > > > `Restric

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

PaulkaToast updated this revision to Diff 248851. PaulkaToast added a comment. Thanks for the suggestions, the general check sounds like a great idea. I can see the use case for this as it can be used by anyone. I took the time to port out fuchsia's check and flesh out the user facing documentat

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp:52 +if (!SM.isInMainFile(HashLoc)) { + DiagnosticBuilder D = Check.diag( + HashLoc, No need for the local DiagnosticBuilde

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

PaulkaToast added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp:66-67 +const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + SmallString<128> CompilerIncudeDir = + StringRef(PP->getHeader

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

PaulkaToast updated this revision to Diff 249530. PaulkaToast marked 4 inline comments as done. PaulkaToast added a comment. Addressed @aaron.ballman comments (: Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75332/new/ https://reviews.llvm.org/D753

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:100 + + Finds includes of sytem libc headers in llvm-libc implementation. + Please synchronize with first statement in documentation. Repository: rG LLVM Github Monorep

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

PaulkaToast updated this revision to Diff 249544. PaulkaToast added a comment. Addressed @Eugene.Zelenko comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75332/new/ https://reviews.llvm.org/D75332 Files: clang-tools-extra/clang-tidy/CMake

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp:66-67 +const SourceManager &SM, Preprocessor *PP, Preprocessor *

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

PaulkaToast updated this revision to Diff 249802. PaulkaToast added a comment. Mocked the header files so that we don't experience failures due to differences in systems. Mind taking a quick look @aaron.ballman? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75332/new/ https://reviews.llvm.org/D75332 ___ cfe-commits mailing list cfe-comm

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

This revision was automatically updated to reflect the committed changes. Closed by commit rGeb41cc619866: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check. (authored by PaulkaToast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv