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 DiagnosticBuilder object, you can stream into the
Check.diag() call.
================
Comment at:
clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp:57
+ } else {
+ DiagnosticBuilder D =
+ Check.diag(HashLoc, "system libc header %0 not allowed");
----------------
No need for the local `DiagnosticBuilder` object, you can stream into the
`Check.diag()` call.
================
Comment at:
clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp:66-67
+ const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP)
{
+ SmallString<128> CompilerIncudeDir =
+ StringRef(PP->getHeaderSearchInfo().getHeaderSearchOpts().ResourceDir);
+ llvm::sys::path::append(CompilerIncudeDir, "include");
----------------
The user can control this path -- is that an issue? You're using it to
determine what a compiler-provided header file is, and this seems like an
escape hatch for users to get around that. If that's reasonable to you, then
I'm okay with it, but you had mentioned you want to remove human error as a
factor and this seems like it could be a subtle human error situation.
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/llvmlibc-restrict-system-libc-headers.rst:16
+
+This check is necesary because accidentally including sytem libc headers can
+lead to subtle and hard to detect bugs. For example consider a system libc
----------------
necesary -> necessary
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-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
