[clang-tools-extra] 0606467 - [clang-tidy] Introduce the WarnIntoHeaders option to modernize-deprecated-headers
Balazs Benics via cfe-commits
cfe-commits at lists.llvm.org
Fri May 20 12:44:33 PDT 2022
Author: Balazs Benics
Date: 2022-05-20T21:41:25+02:00
New Revision: 0606467ea122da5cb23a588e1222a4140445734a
URL: https://github.com/llvm/llvm-project/commit/0606467ea122da5cb23a588e1222a4140445734a
DIFF: https://github.com/llvm/llvm-project/commit/0606467ea122da5cb23a588e1222a4140445734a.diff
LOG: [clang-tidy] Introduce the WarnIntoHeaders option to modernize-deprecated-headers
Unfortunately, we must restrict the checker to warn for deprecated headers
only if the header is included directly from a c++ source file.
For header files, we cannot know if the project has a C source file
that also directly/indirectly includes the offending header file
otherwise. Thus, it's better to be on the safe side and suppress those
reports.
One can opt-in the old behavior, emitting diagnostics into header files,
if one explicitly sets the WarnIntoHeaders=true, in which case nothing
will be changed.
Reviewed By: LegalizeAdulthood
Differential Revision: https://reviews.llvm.org/D125769
Added:
Modified:
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-deprecated-headers.rst
clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
index ee78747d397ff..72508a99e4a1a 100644
--- a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
@@ -27,7 +27,8 @@ namespace {
class IncludeModernizePPCallbacks : public PPCallbacks {
public:
explicit IncludeModernizePPCallbacks(
- std::vector<IncludeMarker> &IncludesToBeProcessed, LangOptions LangOpts);
+ std::vector<IncludeMarker> &IncludesToBeProcessed, LangOptions LangOpts,
+ const SourceManager &SM, bool CheckHeaderFile);
void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
StringRef FileName, bool IsAngled,
@@ -41,6 +42,8 @@ class IncludeModernizePPCallbacks : public PPCallbacks {
LangOptions LangOpts;
llvm::StringMap<std::string> CStyledHeaderToCxx;
llvm::StringSet<> DeleteHeaders;
+ const SourceManager &SM;
+ bool CheckHeaderFile;
};
class ExternCRefutationVisitor
@@ -75,12 +78,18 @@ class ExternCRefutationVisitor
DeprecatedHeadersCheck::DeprecatedHeadersCheck(StringRef Name,
ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ : ClangTidyCheck(Name, Context),
+ CheckHeaderFile(Options.get("CheckHeaderFile", false)) {}
+
+void DeprecatedHeadersCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "CheckHeaderFile", CheckHeaderFile);
+}
void DeprecatedHeadersCheck::registerPPCallbacks(
const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
PP->addPPCallbacks(std::make_unique<IncludeModernizePPCallbacks>(
- IncludesToBeProcessed, getLangOpts()));
+ IncludesToBeProcessed, getLangOpts(), PP->getSourceManager(),
+ CheckHeaderFile));
}
void DeprecatedHeadersCheck::registerMatchers(
ast_matchers::MatchFinder *Finder) {
@@ -124,8 +133,10 @@ void DeprecatedHeadersCheck::check(
}
IncludeModernizePPCallbacks::IncludeModernizePPCallbacks(
- std::vector<IncludeMarker> &IncludesToBeProcessed, LangOptions LangOpts)
- : IncludesToBeProcessed(IncludesToBeProcessed), LangOpts(LangOpts) {
+ std::vector<IncludeMarker> &IncludesToBeProcessed, LangOptions LangOpts,
+ const SourceManager &SM, bool CheckHeaderFile)
+ : IncludesToBeProcessed(IncludesToBeProcessed), LangOpts(LangOpts), SM(SM),
+ CheckHeaderFile(CheckHeaderFile) {
for (const auto &KeyValue :
std::vector<std::pair<llvm::StringRef, std::string>>(
{{"assert.h", "cassert"},
@@ -171,6 +182,12 @@ void IncludeModernizePPCallbacks::InclusionDirective(
bool IsAngled, CharSourceRange FilenameRange, Optional<FileEntryRef> File,
StringRef SearchPath, StringRef RelativePath, const Module *Imported,
SrcMgr::CharacteristicKind FileType) {
+
+ // If we don't want to warn for non-main file reports and this is one, skip
+ // it.
+ if (!CheckHeaderFile && !SM.isInMainFile(HashLoc))
+ return;
+
// FIXME: Take care of library symbols from the global namespace.
//
// Reasonable options for the check:
diff --git a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
index 2af626d42129b..e5023c3f47ed5 100644
--- a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
@@ -38,6 +38,7 @@ class DeprecatedHeadersCheck : public ClangTidyCheck {
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.CPlusPlus;
}
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
@@ -53,6 +54,7 @@ class DeprecatedHeadersCheck : public ClangTidyCheck {
private:
std::vector<IncludeMarker> IncludesToBeProcessed;
+ bool CheckHeaderFile;
};
} // namespace modernize
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 806e5d10a5fdc..41a9723ffeac8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -174,6 +174,10 @@ Changes in existing checks
<clang-tidy/checks/modernize-deprecated-headers>` involving including
C header files from C++ files wrapped by ``extern "C" { ... }`` blocks.
Such includes will be ignored by now.
+ By default now it doesn't warn for including deprecated headers from header
+ files, since that header file might be used from C source files. By passing
+ the ``CheckHeaderFile=true`` option if header files of the project only
+ included by C++ source files.
- Improved :doc:`performance-inefficient-vector-operation
<clang-tidy/checks/performance-inefficient-vector-operation>` to work when
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize-deprecated-headers.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize-deprecated-headers.rst
index 9cdd1d44f6012..974a56abd97dd 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize-deprecated-headers.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize-deprecated-headers.rst
@@ -10,6 +10,17 @@ Standard [depr.c.headers] section.
This check replaces C standard library headers with their C++ alternatives and
removes redundant ones.
+.. code-block:: c++
+
+ // C++ source file...
+ #include <assert.h>
+ #include <stdbool.h>
+
+ // becomes
+
+ #include <cassert>
+ // No 'stdbool.h' here.
+
Important note: the Standard doesn't guarantee that the C++ headers declare all
the same functions in the global namespace. The check in its current form can
break the code that uses library symbols from the global namespace.
@@ -47,3 +58,26 @@ These headers don't have effect in C++:
* `<iso646.h>`
* `<stdalign.h>`
* `<stdbool.h>`
+
+The checker ignores `include` directives within `extern "C" { ... }` blocks,
+since a library might want to expose some API for C and C++ libraries.
+
+.. code-block:: c++
+
+ // C++ source file...
+ extern "C" {
+ #include <assert.h> // Left intact.
+ #include <stdbool.h> // Left intact.
+ }
+
+Options
+-------
+
+.. option:: CheckHeaderFile
+
+ `clang-tidy` cannot know if the header file included by the currently
+ analyzed C++ source file is not included by any other C source files.
+ Hence, to omit false-positives and wrong fixit-hints, we ignore emitting
+ reports into header files. One can set this option to `true` if they know
+ that the header files in the project are only used by C++ source file.
+ Default is `false`.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp
index 458c329de7743..98d66ce00ceb2 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp
@@ -8,6 +8,13 @@
// RUN: && cp %S/Inputs/modernize-deprecated-headers/mylib.h %t/usr/mylib.h
// RUN: %check_clang_tidy -std=c++11 %s modernize-deprecated-headers %t \
+// RUN: -check-suffixes=DEFAULT \
+// RUN: --header-filter='.*' --system-headers \
+// RUN: -- -I %t/usr -isystem %t/sys -isystem %S/Inputs/modernize-deprecated-headers
+
+// RUN: %check_clang_tidy -std=c++11 %s modernize-deprecated-headers %t \
+// RUN: -check-suffixes=DEFAULT,CHECK-HEADER-FILE \
+// RUN: -config="{CheckOptions: [{key: modernize-deprecated-headers.CheckHeaderFile, value: 'true'}]}" \
// RUN: --header-filter='.*' --system-headers \
// RUN: -- -I %t/usr -isystem %t/sys -isystem %S/Inputs/modernize-deprecated-headers
@@ -18,20 +25,20 @@
extern "C++" {
// We should still have the warnings here.
#include <stdbool.h>
-// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: including 'stdbool.h' has no effect in C++; consider removing it [modernize-deprecated-headers]
+// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:10: warning: including 'stdbool.h' has no effect in C++; consider removing it [modernize-deprecated-headers]
}
#include <assert.h>
-// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
+// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
#include <stdbool.h>
-// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: including 'stdbool.h' has no effect in C++; consider removing it [modernize-deprecated-headers]
+// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:10: warning: including 'stdbool.h' has no effect in C++; consider removing it [modernize-deprecated-headers]
#include <mysystemlib.h> // FIXME: We should have no warning into system headers.
-// CHECK-MESSAGES: mysystemlib.h:1:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
+// CHECK-MESSAGES-CHECK-HEADER-FILE: mysystemlib.h:1:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
#include <mylib.h>
-// CHECK-MESSAGES: mylib.h:1:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
+// CHECK-MESSAGES-CHECK-HEADER-FILE: mylib.h:1:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
namespace wrapping {
extern "C" {
More information about the cfe-commits
mailing list