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

[Paula Toth via Phabricator via cfe-commits]

PaulkaToast added a comment.

In D75332#1897487 <https://reviews.llvm.org/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, this will be done in a separate patch. Thanks for pointing it out!

In D75332#1897868 <https://reviews.llvm.org/D75332#1897868>, @Eugene.Zelenko wrote:

> 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).


Done.

In D75332#1899201 <https://reviews.llvm.org/D75332#1899201>, @MaskRay wrote:

> 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).


Please see my reply to aaron.



================
Comment at: clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp:40
+    SrcMgr::CharacteristicKind FileType) {
+  if (SrcMgr::isSystem(FileType)) {
+    if (!SM.isInMainFile(HashLoc)) {
----------------
aaron.ballman wrote:
> njames93 wrote:
> > abrachet wrote:
> > > Could you whitelist the freestanding/compiler provided headers like stddef, stdatomic...
> > Or have a user configurable whitelist 
> It should be configurable and named something other than whitelist. I'd recommend `AllowedHeaders` or something along those lines.
Maintaining a list of acceptable and blacklisted headers would produce a fair bit of maintenance burden. We have a specific use-case in mind here for llvm-libc which is to avoid use of system libc headers that aren't provided by the compiler. I've added a check to allow for compiler provided headers without necessitating a black/white list. 

If a general check is desirable then my suggestion is to pull out [[ https://clang.llvm.org/extra/clang-tidy/checks/fuchsia-restrict-system-includes.html | fuchsia-restrict-system-includes ]] which already implements the features mentioned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75332