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

2020-03-12 Thread Paula Toth via Phabricator via cfe-commits
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

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

2020-03-12 Thread Aaron Ballman via Phabricator via cfe-commits
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

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

2020-03-11 Thread Paula Toth via Phabricator via cfe-commits
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

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

2020-03-11 Thread Aaron Ballman via Phabricator via cfe-commits
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 , Preprocessor *PP, Preprocessor

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

2020-03-10 Thread Paula Toth via Phabricator via cfe-commits
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:

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

2020-03-10 Thread Eugene Zelenko via Phabricator via cfe-commits
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

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

2020-03-10 Thread Paula Toth via Phabricator via cfe-commits
PaulkaToast added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp:66-67 +const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + SmallString<128> CompilerIncudeDir = +

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

2020-03-10 Thread Paula Toth via Phabricator via cfe-commits
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/

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

2020-03-10 Thread Aaron Ballman via Phabricator via cfe-commits
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

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

2020-03-06 Thread Paula Toth via Phabricator via cfe-commits
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

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

2020-03-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D75332#1904317 , @MaskRay wrote: > In D75332#1904264 , @PaulkaToast > wrote: > > > In D75332#1903570 , @MaskRay wrote: > > > > > >

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

2020-03-03 Thread Paula Toth via Phabricator via cfe-commits
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

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

2020-03-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a subscriber: juliehockett. MaskRay added a comment. In D75332#1904264 , @PaulkaToast wrote: > In D75332#1903570 , @MaskRay wrote: > > > > `RestrictSystemLibcHeadersCheck` > > > > As I commented

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

2020-03-03 Thread Eugene Zelenko via Phabricator via cfe-commits
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

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

2020-03-03 Thread Paula Toth via Phabricator via cfe-commits
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

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

2020-03-03 Thread Paula Toth via Phabricator via cfe-commits
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

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

2020-03-03 Thread Fangrui Song via Phabricator via cfe-commits
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

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

2020-03-03 Thread Paula Toth via Phabricator via cfe-commits
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

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

2020-03-03 Thread Fangrui Song via Phabricator via cfe-commits
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.

2020-03-03 Thread Alex Brachet via Phabricator via cfe-commits
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: >

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

2020-03-02 Thread Paula Toth via Phabricator via cfe-commits
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:

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

2020-03-02 Thread Paula Toth via Phabricator via cfe-commits
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.

2020-03-02 Thread Aaron Ballman via Phabricator via cfe-commits
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.

2020-03-01 Thread Nathan James via Phabricator via cfe-commits
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: >

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

2020-02-28 Thread Fangrui Song via Phabricator via cfe-commits
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

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

2020-02-28 Thread Alex Brachet via Phabricator via cfe-commits
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

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

2020-02-28 Thread Eugene Zelenko via Phabricator via cfe-commits
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

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

2020-02-28 Thread Nathan James via Phabricator via cfe-commits
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

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

2020-02-28 Thread Paula Toth via Phabricator via cfe-commits
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