diff --git a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h --- a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h @@ -15,6 +15,22 @@ namespace tidy { namespace modernize { +namespace detail { +class IncludeModernizePPCallbacks; +class ExternCRefutationVisitor; +struct IncludeMarker { + std::string Replacement; + StringRef FileName; + SourceRange ReplacementRange; + std::pair DecomposedDiagLoc; +}; +bool operator<(const IncludeMarker &LHS, const IncludeMarker &RHS); +bool operator<(const IncludeMarker &LHS, + const std::pair &RHS); +bool operator<(const std::pair &LHS, + const IncludeMarker &RHS); +} // namespace detail + /// This check replaces deprecated C library headers with their C++ STL /// alternatives. /// @@ -41,6 +57,13 @@ } void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + friend class detail::IncludeModernizePPCallbacks; + friend class detail::ExternCRefutationVisitor; + std::vector IncludesToBeProcessed; }; } // namespace modernize diff --git a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp --- a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp @@ -7,23 +7,37 @@ //===----------------------------------------------------------------------===// #include "DeprecatedHeadersCheck.h" +#include "clang/AST/RecursiveASTVisitor.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Lex/PPCallbacks.h" #include "clang/Lex/Preprocessor.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringSet.h" +#include #include namespace clang { namespace tidy { namespace modernize { +namespace detail { +bool operator<(const IncludeMarker &LHS, const IncludeMarker &RHS) { + return LHS.DecomposedDiagLoc < RHS.DecomposedDiagLoc; +} +bool operator<(const IncludeMarker &LHS, + const std::pair &RHS) { + return LHS.DecomposedDiagLoc < RHS; +} +bool operator<(const std::pair &LHS, + const IncludeMarker &RHS) { + return LHS < RHS.DecomposedDiagLoc; +} -namespace { class IncludeModernizePPCallbacks : public PPCallbacks { public: - explicit IncludeModernizePPCallbacks(ClangTidyCheck &Check, - LangOptions LangOpts); + explicit IncludeModernizePPCallbacks(DeprecatedHeadersCheck &Check, + LangOptions LangOpts, + const SourceManager &SM); void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName, bool IsAngled, @@ -33,22 +47,84 @@ SrcMgr::CharacteristicKind FileType) override; private: - ClangTidyCheck &Check; + DeprecatedHeadersCheck &Check; LangOptions LangOpts; llvm::StringMap CStyledHeaderToCxx; llvm::StringSet<> DeleteHeaders; + const SourceManager &SM; +}; + +class ExternCRefutationVisitor + : public RecursiveASTVisitor { + std::vector &IncludesToBeProcessed; + const SourceManager &SM; + +public: + ExternCRefutationVisitor(std::vector &IncludesToBeProcessed, + SourceManager &SM) + : IncludesToBeProcessed(IncludesToBeProcessed), SM(SM) {} + bool shouldWalkTypesOfTypeLocs() const { return false; } + bool shouldVisitLambdaBody() const { return false; } + + bool VisitLinkageSpecDecl(LinkageSpecDecl *LinkSpecDecl) const { + auto ExternCBlockBegin = + SM.getDecomposedExpansionLoc(LinkSpecDecl->getBeginLoc()); + auto ExternCBlockEnd = + SM.getDecomposedExpansionLoc(LinkSpecDecl->getEndLoc()); + + auto Begin = IncludesToBeProcessed.begin(); + auto End = IncludesToBeProcessed.end(); + auto Low = std::lower_bound(Begin, End, ExternCBlockBegin); + auto Up = std::upper_bound(Low, End, ExternCBlockEnd); + IncludesToBeProcessed.erase(Low, Up); + return true; + } }; -} // namespace +} // namespace detail void DeprecatedHeadersCheck::registerPPCallbacks( const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { - PP->addPPCallbacks( - ::std::make_unique(*this, getLangOpts())); + PP->addPPCallbacks(::std::make_unique( + *this, getLangOpts(), PP->getSourceManager())); +} +void DeprecatedHeadersCheck::registerMatchers( + ast_matchers::MatchFinder *Finder) { + Finder->addMatcher(ast_matchers::translationUnitDecl().bind("TU"), this); +} + +void DeprecatedHeadersCheck::check( + const ast_matchers::MatchFinder::MatchResult &Result) { + SourceManager &SM = Result.Context->getSourceManager(); + using detail::IncludeMarker; + + llvm::sort(IncludesToBeProcessed); + + // Suppress includes wrapped by `extern "C" { ... }` blocks. + detail::ExternCRefutationVisitor Visitor(IncludesToBeProcessed, SM); + Visitor.TraverseAST(*Result.Context); + + // Emit all the remaining reports. + for (const IncludeMarker &Entry : IncludesToBeProcessed) { + SourceLocation DiagLoc = SM.getComposedLoc(Entry.DecomposedDiagLoc.first, + Entry.DecomposedDiagLoc.second); + if (Entry.Replacement.empty()) { + diag(DiagLoc, "including '%0' has no effect in C++; consider removing it") + << Entry.FileName << FixItHint::CreateRemoval(Entry.ReplacementRange); + } else { + diag(DiagLoc, "inclusion of deprecated C++ header " + "'%0'; consider using '%1' instead") + << Entry.FileName << Entry.Replacement + << FixItHint::CreateReplacement( + Entry.ReplacementRange, + (llvm::Twine("<") + Entry.Replacement + ">").str()); + } + } } -IncludeModernizePPCallbacks::IncludeModernizePPCallbacks(ClangTidyCheck &Check, - LangOptions LangOpts) - : Check(Check), LangOpts(LangOpts) { +detail::IncludeModernizePPCallbacks::IncludeModernizePPCallbacks( + DeprecatedHeadersCheck &Check, LangOptions LangOpts, + const SourceManager &SM) + : Check(Check), LangOpts(LangOpts), SM(SM) { for (const auto &KeyValue : std::vector>( {{"assert.h", "cassert"}, @@ -89,7 +165,7 @@ } } -void IncludeModernizePPCallbacks::InclusionDirective( +void detail::IncludeModernizePPCallbacks::InclusionDirective( SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName, bool IsAngled, CharSourceRange FilenameRange, Optional File, StringRef SearchPath, StringRef RelativePath, const Module *Imported, @@ -101,19 +177,16 @@ // 1. Insert std prefix for every such symbol occurrence. // 2. Insert `using namespace std;` to the beginning of TU. // 3. Do nothing and let the user deal with the migration himself. + std::pair DiagLoc = + SM.getDecomposedExpansionLoc(FilenameRange.getBegin()); if (CStyledHeaderToCxx.count(FileName) != 0) { - std::string Replacement = - (llvm::Twine("<") + CStyledHeaderToCxx[FileName] + ">").str(); - Check.diag(FilenameRange.getBegin(), "inclusion of deprecated C++ header " - "'%0'; consider using '%1' instead") - << FileName << CStyledHeaderToCxx[FileName] - << FixItHint::CreateReplacement(FilenameRange.getAsRange(), - Replacement); + Check.IncludesToBeProcessed.push_back( + IncludeMarker{CStyledHeaderToCxx[FileName], FileName, + FilenameRange.getAsRange(), DiagLoc}); } else if (DeleteHeaders.count(FileName) != 0) { - Check.diag(FilenameRange.getBegin(), - "including '%0' has no effect in C++; consider removing it") - << FileName << FixItHint::CreateRemoval( - SourceRange(HashLoc, FilenameRange.getEnd())); + Check.IncludesToBeProcessed.push_back( + IncludeMarker{std::string{}, FileName, + SourceRange{HashLoc, FilenameRange.getEnd()}, DiagLoc}); } } diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mylib.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mylib.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mylib.h @@ -0,0 +1 @@ +#include "assert.h" 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 new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp @@ -0,0 +1,47 @@ +// RUN: %check_clang_tidy -std=c++11-or-later %s modernize-deprecated-headers %t \ +// RUN: -- -header-filter='.*' -system-headers \ +// RUN: -extra-arg-before=-isystem%S/Inputs/modernize-deprecated-headers + +#define EXTERN_C extern "C" + +#include +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers] +// CHECK-FIXES: {{^}}#include {{$}} + +#include +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: including 'stdbool.h' has no effect in C++; consider removing it [modernize-deprecated-headers] + +#include +// CHECK-MESSAGES: mylib.h:1:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers] + +namespace wrapping { +extern "C" { +#include // no-warning +#include // no-warning +#include // no-warning +} +} + +extern "C" { +namespace wrapped { +#include // no-warning +#include // no-warning +#include // no-warning +} +} + +namespace wrapping { +extern "C" { +namespace wrapped { +#include // no-warning +#include // no-warning +#include // no-warning +} +} +} + +EXTERN_C { +#include // no-warning +#include // no-warning +#include // no-warning +}