Author: Paula Toth Date: 2020-03-20T15:53:05-07:00 New Revision: 556b917fffcfaa15ea11a277e1b0d87b8d13e0f1
URL: https://github.com/llvm/llvm-project/commit/556b917fffcfaa15ea11a277e1b0d87b8d13e0f1 DIFF: https://github.com/llvm/llvm-project/commit/556b917fffcfaa15ea11a277e1b0d87b8d13e0f1.diff LOG: [clang-tidy] Merge common code between llvmlibc-restrict-system-libc-headers and portability-restrict-system-includes Summary: Made llvmlibc::RestrictSystemLibcHeadersCheck a subclass of protability::RestrictSystemIncludesCheck to re-use common code between the two. This also adds the ability to white list linux development headers. Reviewers: aaron.ballman Reviewed By: aaron.ballman Subscribers: mgorny, xazax.hun, MaskRay, cfe-commits, sivachandra Tags: #clang-tools-extra, #clang Differential Revision: https://reviews.llvm.org/D76395 Added: Modified: clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.h clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.cpp clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.h clang-tools-extra/docs/clang-tidy/checks/list.rst clang-tools-extra/docs/clang-tidy/checks/llvmlibc-restrict-system-libc-headers.rst clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers.cpp Removed: clang-tools-extra/test/clang-tidy/checkers/Inputs/llvmlibc/system/math.h clang-tools-extra/test/clang-tidy/checkers/Inputs/llvmlibc/transitive.h clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers-transitive.cpp ################################################################################ diff --git a/clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt b/clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt index c03d8b677f26..cc213d35a572 100644 --- a/clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt @@ -10,6 +10,7 @@ add_clang_library(clangTidyLLVMLibcModule clangBasic clangLex clangTidy + clangTidyPortabilityModule clangTidyUtils clangTooling ) diff --git a/clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp index 8a597c0b2a24..4a19b5359d4f 100644 --- a/clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp @@ -18,12 +18,14 @@ namespace llvm_libc { namespace { -class RestrictedIncludesPPCallbacks : public PPCallbacks { +class RestrictedIncludesPPCallbacks + : public portability::RestrictedIncludesPPCallbacks { public: explicit RestrictedIncludesPPCallbacks( RestrictSystemLibcHeadersCheck &Check, const SourceManager &SM, const SmallString<128> CompilerIncudeDir) - : Check(Check), SM(SM), CompilerIncudeDir(CompilerIncudeDir) {} + : portability::RestrictedIncludesPPCallbacks(Check, SM), + CompilerIncudeDir(CompilerIncudeDir) {} void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName, bool IsAngled, @@ -33,8 +35,6 @@ class RestrictedIncludesPPCallbacks : public PPCallbacks { SrcMgr::CharacteristicKind FileType) override; private: - RestrictSystemLibcHeadersCheck &Check; - const SourceManager &SM; const SmallString<128> CompilerIncudeDir; }; @@ -45,18 +45,12 @@ void RestrictedIncludesPPCallbacks::InclusionDirective( bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File, StringRef SearchPath, StringRef RelativePath, const Module *Imported, SrcMgr::CharacteristicKind FileType) { - if (SrcMgr::isSystem(FileType)) { - // Compiler provided headers are allowed (e.g stddef.h). - if (SearchPath == CompilerIncudeDir) return; - if (!SM.isInMainFile(HashLoc)) { - Check.diag( - HashLoc, - "system libc header %0 not allowed, transitively included from %1") - << FileName << SM.getFilename(HashLoc); - } else { - Check.diag(HashLoc, "system libc header %0 not allowed") << FileName; - } - } + // Compiler provided headers are allowed (e.g stddef.h). + if (SrcMgr::isSystem(FileType) && SearchPath == CompilerIncudeDir) + return; + portability::RestrictedIncludesPPCallbacks::InclusionDirective( + HashLoc, IncludeTok, FileName, IsAngled, FilenameRange, File, SearchPath, + RelativePath, Imported, FileType); } void RestrictSystemLibcHeadersCheck::registerPPCallbacks( diff --git a/clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.h b/clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.h index 3910a29a28e4..9eead7a22882 100644 --- a/clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.h +++ b/clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.h @@ -10,6 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVMLIBC_RESTRICTSYSTEMLIBCHEADERSCHECK_H #include "../ClangTidyCheck.h" +#include "../portability/RestrictSystemIncludesCheck.h" namespace clang { namespace tidy { @@ -20,10 +21,11 @@ namespace llvm_libc { /// /// For the user-facing documentation see: /// http://clang.llvm.org/extra/clang-tidy/checks/llvmlibc-restrict-system-libc-headers.html -class RestrictSystemLibcHeadersCheck : public ClangTidyCheck { +class RestrictSystemLibcHeadersCheck + : public portability::RestrictSystemIncludesCheck { public: RestrictSystemLibcHeadersCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + : portability::RestrictSystemIncludesCheck(Name, Context, "-*") {} void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) override; }; diff --git a/clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.cpp b/clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.cpp index 15076d01a771..f6163989a461 100644 --- a/clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.cpp +++ b/clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.cpp @@ -20,42 +20,6 @@ namespace clang { namespace tidy { namespace portability { -class RestrictedIncludesPPCallbacks : public PPCallbacks { -public: - explicit RestrictedIncludesPPCallbacks(RestrictSystemIncludesCheck &Check, - const SourceManager &SM) - : Check(Check), SM(SM) {} - - void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok, - StringRef FileName, bool IsAngled, - CharSourceRange FilenameRange, const FileEntry *File, - StringRef SearchPath, StringRef RelativePath, - const Module *Imported, - SrcMgr::CharacteristicKind FileType) override; - void EndOfMainFile() override; - -private: - struct IncludeDirective { - IncludeDirective() = default; - IncludeDirective(SourceLocation Loc, CharSourceRange Range, - StringRef Filename, StringRef FullPath, bool IsInMainFile) - : Loc(Loc), Range(Range), IncludeFile(Filename), IncludePath(FullPath), - IsInMainFile(IsInMainFile) {} - - SourceLocation Loc; // '#' location in the include directive - CharSourceRange Range; // SourceRange for the file name - std::string IncludeFile; // Filename as a string - std::string IncludePath; // Full file path as a string - bool IsInMainFile; // Whether or not the include is in the main file - }; - - using FileIncludes = llvm::SmallVector<IncludeDirective, 8>; - llvm::SmallDenseMap<FileID, FileIncludes> IncludeDirectives; - - RestrictSystemIncludesCheck &Check; - const SourceManager &SM; -}; - void RestrictedIncludesPPCallbacks::InclusionDirective( SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName, bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File, diff --git a/clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.h b/clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.h index db2f9935534b..c34f054fba2e 100644 --- a/clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.h +++ b/clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.h @@ -23,9 +23,10 @@ namespace portability { /// http://clang.llvm.org/extra/clang-tidy/checks/portability-restrict-system-includes.html class RestrictSystemIncludesCheck : public ClangTidyCheck { public: - RestrictSystemIncludesCheck(StringRef Name, ClangTidyContext *Context) + RestrictSystemIncludesCheck(StringRef Name, ClangTidyContext *Context, + std::string DefaultAllowedIncludes = "*") : ClangTidyCheck(Name, Context), - AllowedIncludes(Options.get("Includes", "*")), + AllowedIncludes(Options.get("Includes", DefaultAllowedIncludes)), AllowedIncludesGlobList(AllowedIncludes) {} void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, @@ -40,8 +41,44 @@ class RestrictSystemIncludesCheck : public ClangTidyCheck { GlobList AllowedIncludesGlobList; }; +class RestrictedIncludesPPCallbacks : public PPCallbacks { +public: + explicit RestrictedIncludesPPCallbacks(RestrictSystemIncludesCheck &Check, + const SourceManager &SM) + : Check(Check), SM(SM) {} + + void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok, + StringRef FileName, bool IsAngled, + CharSourceRange FilenameRange, const FileEntry *File, + StringRef SearchPath, StringRef RelativePath, + const Module *Imported, + SrcMgr::CharacteristicKind FileType) override; + void EndOfMainFile() override; + +private: + struct IncludeDirective { + IncludeDirective() = default; + IncludeDirective(SourceLocation Loc, CharSourceRange Range, + StringRef Filename, StringRef FullPath, bool IsInMainFile) + : Loc(Loc), Range(Range), IncludeFile(Filename), IncludePath(FullPath), + IsInMainFile(IsInMainFile) {} + + SourceLocation Loc; // '#' location in the include directive + CharSourceRange Range; // SourceRange for the file name + std::string IncludeFile; // Filename as a string + std::string IncludePath; // Full file path as a string + bool IsInMainFile; // Whether or not the include is in the main file + }; + + using FileIncludes = llvm::SmallVector<IncludeDirective, 8>; + llvm::SmallDenseMap<FileID, FileIncludes> IncludeDirectives; + + RestrictSystemIncludesCheck &Check; + const SourceManager &SM; +}; + } // namespace portability } // namespace tidy } // namespace clang -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PORTABILITY_RESTRICTINCLUDESSCHECK_H \ No newline at end of file +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PORTABILITY_RESTRICTINCLUDESSCHECK_H diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 5e943c5003f0..37f020d6018f 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -187,7 +187,7 @@ Clang-Tidy Checks `llvm-prefer-isa-or-dyn-cast-in-conditionals <llvm-prefer-isa-or-dyn-cast-in-conditionals.html>`_, "Yes" `llvm-prefer-register-over-unsigned <llvm-prefer-register-over-unsigned.html>`_, "Yes" `llvm-twine-local <llvm-twine-local.html>`_, "Yes" - `llvmlibc-restrict-system-libc-headers <llvmlibc-restrict-system-libc-headers.html>`_, + `llvmlibc-restrict-system-libc-headers <llvmlibc-restrict-system-libc-headers.html>`_, "Yes" `misc-definitions-in-headers <misc-definitions-in-headers.html>`_, "Yes" `misc-misplaced-const <misc-misplaced-const.html>`_, `misc-new-delete-overloads <misc-new-delete-overloads.html>`_, diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvmlibc-restrict-system-libc-headers.rst b/clang-tools-extra/docs/clang-tidy/checks/llvmlibc-restrict-system-libc-headers.rst index 0ec092584895..bf39dd62ba95 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/llvmlibc-restrict-system-libc-headers.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvmlibc-restrict-system-libc-headers.rst @@ -18,3 +18,18 @@ lead to subtle and hard to detect bugs. For example consider a system libc whose ``dirent`` struct has slightly diff erent field ordering than llvm-libc. While this will compile successfully, this can cause issues during runtime because they are ABI incompatible. + +Options +------- + +.. option:: Includes + + A string containing a comma separated glob list of allowed include + filenames. Similar to the -checks glob list for running clang-tidy itself, + the two wildcard characters are `*` and `-`, to include and exclude globs, + respectively. The default is `-*`, which disallows all includes. + + This can be used to allow known safe includes such as Linux development + headers. See :doc:`portability-restrict-system-includes + <portability-restrict-system-includes>` for more + details. diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/llvmlibc/system/math.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/llvmlibc/system/math.h deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/llvmlibc/transitive.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/llvmlibc/transitive.h deleted file mode 100644 index a84546e5bc83..000000000000 --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/llvmlibc/transitive.h +++ /dev/null @@ -1 +0,0 @@ -#include <math.h> diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers-transitive.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers-transitive.cpp deleted file mode 100644 index 745aa0bb3401..000000000000 --- a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers-transitive.cpp +++ /dev/null @@ -1,8 +0,0 @@ -// RUN: %check_clang_tidy %s llvmlibc-restrict-system-libc-headers %t \ -// RUN: -- -header-filter=.* \ -// RUN: -- -I %S/Inputs/llvmlibc \ -// RUN: -isystem %S/Inputs/llvmlibc/system \ -// RUN: -resource-dir %S/Inputs/llvmlibc/resource - -#include "transitive.h" -// CHECK-MESSAGES: :1:1: warning: system libc header math.h not allowed, transitively included from {{.*}} diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers.cpp index 43f5b1e94279..52e25faf190f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers.cpp @@ -3,11 +3,11 @@ // RUN: -resource-dir %S/Inputs/llvmlibc/resource #include <stdio.h> -// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system libc header stdio.h not allowed +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include stdio.h not allowed #include <stdlib.h> -// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system libc header stdlib.h not allowed +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include stdlib.h not allowed #include "string.h" -// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system libc header string.h not allowed +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include string.h not allowed #include "stdatomic.h" #include <stddef.h> // Compiler provided headers should not throw warnings. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
