[clang-tools-extra] 556b917 - [clang-tidy] Merge common code between llvmlibc-restrict-system-libc-headers and portability-restrict-system-includes
Paula Toth via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 20 16:02:21 PDT 2020
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.
More information about the cfe-commits
mailing list