[libc-commits] [PATCH] D76559: [libc] Enable llvmlibc clang-tidy checks

Alex Brachet via Phabricator via libc-commits libc-commits at lists.llvm.org
Sat Mar 21 20:18:41 PDT 2020


abrachet accepted this revision.
abrachet marked 2 inline comments as done.
abrachet added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libc/src/.clang-tidy:6
+  - key:             llvmlibc-restrict-system-libc-headers.Includes
+    value:           '-*, linux/*, asm/unistd.h'
----------------
PaulkaToast wrote:
> abrachet wrote:
> > Everything in `asm/*` should be safe, I imagine.
> That's what I thought but it has stuff like https://elixir.bootlin.com/linux/latest/source/arch/x86/include/asm/string_32.h
> 
> which might be unsafe to include?
That's not in `/usr/include/asm` on my machine, and is protected by an `#ifdef __KERNEL__` it looks like.

I have no strong preference, we're only using `asm/unistd.h` I suppose so it makes no big difference either way.


================
Comment at: libc/src/math/round_redirector.cpp:10
+// Include okay for this redirector.
+// NOLINTNEXTLINE(llvmlibc-restrict-system-libc-headers)
 #include <math.h>
----------------
PaulkaToast wrote:
> abrachet wrote:
> > Is it possible for value to be something like `*redirector*` so we don't need to manually add the nolint manually like this? Not that its a big deal as this is the only one.
> I think it might be better to be on the safe side here and avoid a blanket "allow all includes in redirectors" sort of mechanism. Just to avoid accidentally making a mistake, thoughts?
That's probably better, you're right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76559





More information about the libc-commits mailing list