This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
steakhal marked an inline comment as done.
Closed by commit rG0606467ea122: [clang-tidy] Introduce the WarnIntoHeaders
option to modernize-deprecated… (authored by steakhal).
Changed prior to commit:
https://reviews.llvm.org/D125769?vs=430303&id=431029#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125769/new/
https://reviews.llvm.org/D125769
Files:
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
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp
+++ 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" {
Index: clang-tools-extra/docs/clang-tidy/checks/modernize-deprecated-headers.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/modernize-deprecated-headers.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-deprecated-headers.rst
@@ -10,6 +10,17 @@
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 @@
* `<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`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -174,6 +174,10 @@
<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
Index: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
+++ clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
@@ -38,6 +38,7 @@
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 @@
private:
std::vector<IncludeMarker> IncludesToBeProcessed;
+ bool CheckHeaderFile;
};
} // namespace modernize
Index: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
@@ -27,7 +27,8 @@
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 @@
LangOptions LangOpts;
llvm::StringMap<std::string> CStyledHeaderToCxx;
llvm::StringSet<> DeleteHeaders;
+ const SourceManager &SM;
+ bool CheckHeaderFile;
};
class ExternCRefutationVisitor
@@ -75,12 +78,18 @@
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 @@
}
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 @@
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:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
