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" { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits