[clang-tidy] Merge common code between llvmlibc-restrict-system-libc-headers and...
authorPaula Toth <paulatoth@google.com>
Fri, 20 Mar 2020 22:52:39 +0000 (15:52 -0700)
committerPaula Toth <paulatoth@google.com>
Fri, 20 Mar 2020 22:53:05 +0000 (15:53 -0700)
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

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/Inputs/llvmlibc/system/math.h [deleted file]
clang-tools-extra/test/clang-tidy/checkers/Inputs/llvmlibc/transitive.h [deleted file]
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers-transitive.cpp [deleted file]
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers.cpp

index c03d8b6..cc213d3 100644 (file)
@@ -10,6 +10,7 @@ add_clang_library(clangTidyLLVMLibcModule
   clangBasic
   clangLex
   clangTidy
+  clangTidyPortabilityModule
   clangTidyUtils
   clangTooling
   )
index 8a597c0..4a19b53 100644 (file)
@@ -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 @@ public:
                           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(
index 3910a29..9eead7a 100644 (file)
@@ -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;
 };
index 15076d0..f616398 100644 (file)
@@ -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,
index db2f993..c34f054 100644 (file)
@@ -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 @@ private:
   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
index 5e943c5..37f020d 100644 (file)
@@ -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>`_,
index 0ec0925..bf39dd6 100644 (file)
@@ -18,3 +18,18 @@ lead to subtle and hard to detect bugs. For example consider a system libc
 whose ``dirent`` struct has slightly different 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 (file)
index e69de29..0000000
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 (file)
index a84546e..0000000
+++ /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 (file)
index 745aa0b..0000000
+++ /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 {{.*}}
index 43f5b1e..52e25fa 100644 (file)
@@ -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.