steakhal created this revision.
steakhal added reviewers: aaron.ballman, whisperity, LegalizeAdulthood, alexfh.
Herald added subscribers: carlosgalvezp, martong, rnkovacs, xazax.hun.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
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/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,WARN-INTO-HEADERS \
+// RUN: -config="{CheckOptions: [{key: modernize-deprecated-headers.WarnIntoHeaders, 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-WARN-INTO-HEADERS: 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-WARN-INTO-HEADERS: 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/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 ``WarnIntoHeaders=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
@@ -49,6 +49,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;
@@ -59,6 +60,7 @@
friend class detail::IncludeModernizePPCallbacks;
friend class detail::ExternCRefutationVisitor;
std::vector<detail::IncludeMarker> IncludesToBeProcessed;
+ bool WarnIntoHeaders;
};
} // 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
@@ -24,7 +24,8 @@
class IncludeModernizePPCallbacks : public PPCallbacks {
public:
explicit IncludeModernizePPCallbacks(DeprecatedHeadersCheck &Check,
- LangOptions LangOpts);
+ LangOptions LangOpts,
+ const SourceManager &SM);
void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
StringRef FileName, bool IsAngled,
@@ -38,6 +39,7 @@
LangOptions LangOpts;
llvm::StringMap<std::string> CStyledHeaderToCxx;
llvm::StringSet<> DeleteHeaders;
+ const SourceManager &SM;
};
class ExternCRefutationVisitor
@@ -72,12 +74,17 @@
DeprecatedHeadersCheck::DeprecatedHeadersCheck(StringRef Name,
ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ : ClangTidyCheck(Name, Context),
+ WarnIntoHeaders(Options.get("WarnIntoHeaders", false)) {}
+
+void DeprecatedHeadersCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "WarnIntoHeaders", WarnIntoHeaders);
+}
void DeprecatedHeadersCheck::registerPPCallbacks(
const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
PP->addPPCallbacks(::std::make_unique<detail::IncludeModernizePPCallbacks>(
- *this, getLangOpts()));
+ *this, getLangOpts(), PP->getSourceManager()));
}
void DeprecatedHeadersCheck::registerMatchers(
ast_matchers::MatchFinder *Finder) {
@@ -121,8 +128,9 @@
}
detail::IncludeModernizePPCallbacks::IncludeModernizePPCallbacks(
- DeprecatedHeadersCheck &Check, LangOptions LangOpts)
- : Check(Check), LangOpts(LangOpts) {
+ DeprecatedHeadersCheck &Check, LangOptions LangOpts,
+ const SourceManager &SM)
+ : Check(Check), LangOpts(LangOpts), SM(SM) {
for (const auto &KeyValue :
std::vector<std::pair<llvm::StringRef, std::string>>(
{{"assert.h", "cassert"},
@@ -168,6 +176,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 (!Check.WarnIntoHeaders && !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
